-
Notifications
You must be signed in to change notification settings - Fork 550
Implement fail-fast behavior for JUnit Platform provider #3155
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
Implement fail-fast behavior for JUnit Platform provider #3155
Conversation
Wow, cool! Thanks |
surefire-its/src/test/resources/fail-fast-junit-platform/pom.xml
Outdated
Show resolved
Hide resolved
I've changed this PR to "draft" because it requires at least a milestone release of JUnit 6.0.0 to be merged. I've opened it early so I can address any feedback you might have. |
Moreover, this PR supersedes #503 |
static final Class<?> LAUNCHER_EXECUTION_REQUEST_CLASS = loadLauncherExecutionRequestClass(); | ||
static final Class<?> CANCELLATION_TOKEN_CLASS = loadCancellationTokenClass(); | ||
static final Supplier<CancellationTokenAdapter> CANCELLATION_TOKEN_FACTORY = createCancellationTokenFactory(); | ||
static final Method EXECUTE_METHOD_WITH_LAUNCHER_EXECUTION_REQUEST_PARAMETER = | ||
findExecuteMethodWithLauncherExecutionRequestParameter(); |
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 class heavily relies on reflection to keep compatible with pre-6.x versions of JUnit
With JUnit 6 in play, testing with JDK 8 and 11 are out - as JDK 17 is the new minimum. |
Done in 4221aca |
@cstamas Could you please approve workflow execution? I think all tests should be green now. 🤞 |
One more time, please! 😬 |
The one remaining failure looks unrelated. Is that test known to be flaky? 🤔 |
Great! Thanks for this. |
I'll update this PR early next week after which it should be ready to be merged. |
5cb20ea
to
dd77018
Compare
dd77018
to
d834468
Compare
I've rebased this PR to include the latest changes from the default branch and updated the integration test to use JUnit 6.0.0-M2. From my perspective, it's ready to be reviewed and then merged now. |
I pushed another commit that should fix the problem on JDK 17 and later |
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.
LGTM Thanks
@olamy Can this PR be merged? |
Sorry for delay and thanks a lot for the contribution |
Resolves #2038.
Following this checklist to help us incorporate your
contribution quickly and easily:
mvn clean install
to make sure basic checks pass. A more thorough check willbe performed on your pull request automatically.
mvn -Prun-its clean install
).If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.