Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alters POM to minimize test-compile dependencies #16

Merged

Conversation

smoyer64
Copy link
Contributor

@smoyer64 smoyer64 commented Sep 8, 2016

Overview

JUnit 5 has separated the public facing APIs from the runner, engines, launcher and console in an effort to keep test writers from using non-public classes when writing their tests. The existing Maven POM file adds dependencies these non-public classes to the test-compile scope which allows users to accidentally become dependent on these non-public classes. The test-compile dependency hierarchy in the existing POM is as follows:

junit5-maven-consumer-dependencies

This pull request alters the POM file so that the JUnit 5 classes exposed to the test writer are minimized. Since the runner and engine classes (and their dependencies) are required during test execution, these dependencies are instead added to the classpath when the maven-surefire-plugin is executed. After this change, the test-compile dependency hierarchy in the POM is as follows:

junit5-maven-consumer-minimal-dependencies

The results of running mvn clean test is unchanged, so the README file is still accurate.


I hereby agree to the terms of the JUnit Contributor License Agreement.

<dependency>
<groupId>org.junit.platform</groupId>
<artifactId>junit-platform-runner</artifactId>
<version>${junit.platform.version}</version>
Copy link
Member

@marcphilipp marcphilipp Sep 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why junit-platform-runner? If that were used it would need to be a test dependency, because it would be required by test-compile, wouldn't it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree: the JUnitPlatform runner is only useful if a user can actually reference it from test classes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right ... that dependency isn't needed. It seemed to be an artifact of some of the experiments I ran trying to use the JUnitPlatform runner without putting so many classes on the test-compile classpath. Those experiments ended up producing this issue in the JUnit 5 project: junit-team/junit5#498.

I'll alter this PR to match the users guide tomorrow when I get into work.

@marcphilipp
Copy link
Member

Thanks for changing this! I've been meaning to do so myself a few times! 👍

@smoyer64 smoyer64 force-pushed the feature/minimal-maven-dependencies branch from 9b4c8d7 to 0e3eac6 Compare September 12, 2016 13:17
@smoyer64
Copy link
Contributor Author

I've eliminated the extra plugin dependency from this PR (junit-platform-runner).

@marcphilipp marcphilipp merged commit 39d1d9c into junit-team:master Sep 12, 2016
@marcphilipp
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants