mps-test doesn't include ingestion-sink JAR

[ahounsel]

I'm trying to update the schema for a custom ping that we developed for a Normandy study (telemetry/dnssec-study-v1). I want to test my changes and ensure that they're not breaking the schema. However, I'm having trouble getting the tests to run in Docker.

I'm on a 2018 Macbook Pro running Catalina. When I run mps-build followed by mps-test, the tests hang on "Collecting 0 items." To investigate further, I modified mps-test to replace --entrypoint=pytest with --entrypoint=/bin/bash. This shelled me into a container, so I then ran pytest -s -v to see if any errors would get printed. This seems to be the problem causing the tests to hang:

jnius.JavaException: JVM exception occurred: com/mozilla/telemetry/ingestion/sink/config/SinkConfig java.lang.NoClassDefFoundError

Interestingly, if I simply remove the --volume "$(pwd)":/app flag from mps-test, the dependency seems to appear in the container in target/ingestion-sink-0.1-SNAPSHOT.jar and target/ingestion-sink-0.1-SNAPSHOT.lib. When I run pytest -s -v again, the tests work.

Anyone have insight into why the ingestion-sink JAR is not copied over to the container when I run mps-test?

[jklukas]

The Dockerfile includes RUN mvn dependency:copy-dependencies which is how target/ingestion-sink-0.1-SNAPSHOT.jar ends up in the built image at /app/target/ingestion-sink-0.1-SNAPSHOT.jar. In the mps-test script, I see now there a comment:

# NOTE: this will shadow java dependencies in `target/dependency`

So your steps to repro I think make sense to me. The --volume "$(pwd)":/app flag mounts the contents our your current directory at /app so the target/ directory becomes inaccessible.

The point of --volume "$(pwd)":/app is that you're able to rerun tests without having to rebuild the container. By taking that line out, you'll need to rebuild the image every time you make a change in the templates dir in order to have those changes reflected when you run the test.

I'm thinking about potential workarounds here.

[jklukas]

Thanks for writing this up, @ahounsel!