-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add forking support in surefire provider #382
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
Add forking support in surefire provider #382
Conversation
} | ||
|
||
@Test | ||
@Disabled("Methods annotated with @TestFactory are not accepted, they are containers, not tests") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Classes which contain only @TestFactory
annotated methods do not seem to be discovered by surefire provider. This condition https://github.com/junit-team/junit5/blob/master/junit-platform-surefire-provider/src/main/java/org/junit/platform/surefire/provider/TestPlanScannerFilter.java#L38 accepts only test methods and @TestFactory
methods are containers.
Please let me know if it is valuable to keep these disabled tests here or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was fixed recently, see #384 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, I'll rebase this PR today.
Forking requires two methods to be implemented in JUnitPlatformProvider: 1) `#getSuites()` - defines set of test classes for the future forks to execute 2) `#invoke(Object)` - called inside a fork with a `Class` argument from (1) This commit adds implementation of `#getSuits()` and alters `#invoke(Object)` to support forking. It also adds unit tests for all classes in junit-platform-surefire-provider module.
7ad1c31
to
af9aba2
Compare
Looks good to me! @junit-team/junit-lambda Any objections on merging this? |
Based on principle, I am all for having this support, but I haven't reviewed it personally. So, if you think it's good to go, then... merge it! |
@marcphilipp yes, I can work on such tests. they should be part of |
Yep. 👍 |
Thanks to the newly added tests, our test coverage went from 93.4% to 95.3%. 👍 |
Overview
Forking requires two methods to be implemented in JUnitPlatformProvider:
#getSuites()
- defines set of test classes for the future forks to execute#invoke(Object)
- called inside a fork with aClass
argument from (1)This PR adds implementation of
#getSuits()
and alters#invoke(Object)
to support forking. It also adds unit tests for all classes in junit-platform-surefire-provider module.Fixes #362 #363
I hereby agree to the terms of the JUnit Contributor License Agreement.