Update JUnit 5 dependency recommendations in POM

[smoyer64]

When I submitted PR #16 (Alters POM to minimize test-compile dependencies), I purposely included only the junit-jupiter-api artifact as a test-scoped dependency and included the other packages that were needed at test-execution time as a dependency of the maven-surefire-plugin. My intention was to only expose the classes in junit-jupiter-api to the test writer's test compiler.

In this comment, @Tibor17 has instructed that this is not the correct configuration and that only the junit-platform-surefire-provider should be a plugin dependency. I'll let @Tibor17 comment on why this is important if he chooses.

Related to https://github.com/junit-team/junit5/issues/848

[marcphilipp]

@Tibor17 Please comment on why you think our existing config should not work, even though it does! 😉

[Tibor17]

We can check AbstractSurefireMojo if really needed. This is to my memory that we pickup such artifact which has Surefire SPI. This artifact is provided in classpath when you run your test. Surefire documented adding provider in plugin's dependency section.

Is there any reason to not to use project dependencies?

[Tibor17]

Pls follow https://github.com/junit-team/junit5-samples/pull/26.

[smoyer64]

The reason to not use project dependencies is that the classes included via the plugin dependencies are not on the classpath for the test writer (but are available to the plugin itself). That's the original impetus for the current documentation and sample.

[Tibor17]

But everything must be on classpath; otherwise the forked jvm would not run. Consider you use forkCount=5 in Surefire configuration. If you start very long term running test class go to target/surefire/ and copy jar file from it to a safe folder. Open it and to go manifest. You will see Class-Path header. This is the classpath the ForkedBooter has, which includes provider, all transitive dependencies and project dependencies with scope=test.

[Tibor17]

Last to mention, the plugin (plugin's ClassLoader) does not need to have your dependencies. The point is that plugin starts forked JVM where all jar files reside. This means plugin != provider. Provider and plugin is isolated. Mostly on the level of jvm processes, or ClassLoader (if forkCount=0) within one process. The default and usual is process isolation.

[smoyer64]

So apparently I'm not running into issues because I'm hardly using the junit-vintage-engine and the junit-jupiter-engine doesn't support forking through the junit-platform-surefire-provider.

[Tibor17]

The plugin itself does forking. This means, simply the plugin creates jar file with manifest having Class-Path, the plugin is then executing java -jar target/surefire/1234567890.jar. The Class-Path is pointing to jars located in your local repository e.g. ~/.m2/repository.

[marcphilipp]

This is what MANIFEST.MF looks like for our sample project (cf. POM):

Manifest-Version: 1.0
Class-Path: file:/Users/marcp/.m2/repository/org/apache/maven/surefire
 /surefire-booter/2.19.1/surefire-booter-2.19.1.jar file:/Users/marcp/
 .m2/repository/org/apache/maven/surefire/surefire-api/2.19.1/surefire
 -api-2.19.1.jar file:/Users/marcp/Repositories/junit5-samples/junit5-
 maven-consumer/target/test-classes/ file:/Users/marcp/Repositories/ju
 nit5-samples/junit5-maven-consumer/target/classes/ file:/Users/marcp/
 .m2/repository/org/junit/jupiter/junit-jupiter-api/5.0.0-M4/junit-jup
 iter-api-5.0.0-M4.jar file:/Users/marcp/.m2/repository/org/opentest4j
 /opentest4j/1.0.0-M2/opentest4j-1.0.0-M2.jar file:/Users/marcp/.m2/re
 pository/org/junit/platform/junit-platform-commons/1.0.0-M4/junit-pla
 tform-commons-1.0.0-M4.jar file:/Users/marcp/.m2/repository/junit/jun
 it/4.12/junit-4.12.jar file:/Users/marcp/.m2/repository/org/hamcrest/
 hamcrest-core/1.3/hamcrest-core-1.3.jar file:/Users/marcp/.m2/reposit
 ory/org/junit/platform/junit-platform-surefire-provider/1.0.0-M4/juni
 t-platform-surefire-provider-1.0.0-M4.jar file:/Users/marcp/.m2/repos
 itory/org/junit/platform/junit-platform-launcher/1.0.0-M4/junit-platf
 orm-launcher-1.0.0-M4.jar file:/Users/marcp/.m2/repository/org/apache
 /maven/surefire/common-java5/2.19.1/common-java5-2.19.1.jar file:/Use
 rs/marcp/.m2/repository/org/junit/jupiter/junit-jupiter-engine/5.0.0-
 M4/junit-jupiter-engine-5.0.0-M4.jar file:/Users/marcp/.m2/repository
 /org/junit/platform/junit-platform-engine/1.0.0-M4/junit-platform-eng
 ine-1.0.0-M4.jar file:/Users/marcp/.m2/repository/org/junit/vintage/j
 unit-vintage-engine/4.12.0-M4/junit-vintage-engine-4.12.0-M4.jar file
 :/Users/marcp/.m2/repository/org/apache/maven/reporting/maven-reporti
 ng-api/3.0/maven-reporting-api-3.0.jar
Main-Class: org.apache.maven.surefire.booter.ForkedBooter

As you can see all the dependencies listed in the plugin configuration's dependencies section are on the classpath. Thus, I do not see a problem with our recommended configuration.

@Tibor17 Do you want to double-check AbstractSurefireMojo?

[smoyer64]

I had a project and tested this both ways when I originally was trying to minimize the classpath used in the test scope. Since the JUnit 5 provider is single-threaded, I've never seen an issue. I'm relying on @Tibor17 to describe the requirements for a threaded provider.

[marcphilipp]

The above MANIFEST.MF is from a run with <forkCount>4</forkCount> and it does indeed fork and run tests in parallel (in different VMs) without any issue.

[smoyer64]

@marcphilipp - Then I vote to keep the current configuration as well. I like the fact that the engine classes aren't available to the test writer.

[marcphilipp]

@Tibor17 Do you want to take another look?

[marcphilipp]

Haven't heard back from @Tibor17 but I'm going ahead and closing this and the related #26, junit-team/junit5#848, and junit-team/junit5#849.