You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/09/13 18:56:47 UTC
[GitHub] [maven-surefire] papegaaij opened a new pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
papegaaij opened a new pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389
A LauncherSession (JUnit Platform 1.8 feature) is started when the Launcher is first used and closed when all tests have run. This allows integrations to run pre and post fixtures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#issuecomment-1016344148
@papegaaij
We are in hurry with many PRs.
Pls let us review again against Jira...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] papegaaij commented on pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
papegaaij commented on pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#issuecomment-1016545576
@Tibor17 I've rebased the PR and squashed the commits.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] papegaaij commented on pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
papegaaij commented on pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#issuecomment-1016426984
The `LauncherSession` is explicitly meant to provide a state over all tests. It is completely separate from `Extension`. This patch will only effect projects that already use these `LauncherSessions`. For that, they have to implement `LauncherSessionListener` and put the classname in a service file for the `ServiceLoader` to pick it up. If a project makes use of `LauncherSessionListener`, they will, without this patch, see a new session being started for every test, which actually is incorrect behavior. The javadoc on `LauncherSession` explains this: `The LauncherSession API is the main entry point for client code that wishes to repeatedly discover and execute tests using one or more test engines.` This is exactly what Surefire does, hence it should use `LauncherSession` to do this.
What a user does with a `LauncherSessionListener` implementation is up to the user. The javadoc does not give any guidelines. The Gradle documentation actually points to this interface for once-per-vm setup and teardown: https://docs.gradle.com/enterprise/test-distribution-gradle-plugin/#junit_5_8_and_later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 merged pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
Tibor17 merged pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#discussion_r787793027
##########
File path: surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java
##########
@@ -231,6 +225,21 @@ private void execute( TestsToRun testsToRun, RunListenerAdapter adapter )
} );
}
}
+
+ private void closeLauncher()
+ {
+ if ( launcher instanceof AutoCloseable )
+ {
+ try
+ {
+ ( (AutoCloseable) launcher ).close();
+ }
+ catch ( Exception e )
+ {
+ throw new SurefireReflectionException( e );
Review comment:
pls use `throw new SurefireReflectionException( e.grtLocalMessage(), e );`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#issuecomment-918996965
You should not take everything into one session. There are reruns which
have to be isolated trom the main run. And I am convinced that the reruns
also have to be isolated in each.
Dňa po 13. 9. 2021, 20:56 Emond Papegaaij ***@***.***>
napísal(a):
> A LauncherSession (JUnit Platform 1.8 feature) is started when the
> Launcher is first used and closed when all tests have run. This allows
> integrations to run pre and post fixtures.
> ------------------------------
> You can view, comment on, or merge this pull request online at:
>
> https://github.com/apache/maven-surefire/pull/389
> Commit Summary
>
> - [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via
> LauncherSession
>
> File Changes
>
> - *M*
> surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java
> <https://github.com/apache/maven-surefire/pull/389/files#diff-80115dd9c11ea216f73791da899b0ea3dc424f154dd7c8c65173de7071cc38ee>
> (37)
> - *M*
> surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/LazyLauncher.java
> <https://github.com/apache/maven-surefire/pull/389/files#diff-84b65bdf8ce1e6e82794216416cc3d8c7ea2f769e0a5e3e3f3425b68c685c617>
> (24)
>
> Patch Links:
>
> - https://github.com/apache/maven-surefire/pull/389.patch
> - https://github.com/apache/maven-surefire/pull/389.diff
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/maven-surefire/pull/389>, or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAH7ER674FANSMJGXS43EPLUBZCPFANCNFSM5D6PDG2Q>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#issuecomment-1016377341
I understand that you are aiming for loading heavy resources only once.
On the other hand the unit tests have a rule where each test/method has isolated context.
How can we ensure such rule in this patch?
The interface `Extension` is marker interface and has many implementations. We only need to guarantee that the session you have implemented would not make e.g. `BeforeEachCallback` stateful across all tests (before-each represents stateless resource).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] papegaaij commented on pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
papegaaij commented on pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#issuecomment-1016353322
We have been running a patched build for several months now with this change and it works very well. The downside to this solution in relation to the jira issue is that it only helps when the framework actually makes use of the launcher sessions. This is still very rare as it is a new feature. It is however the recommended way (by JUnit) of doing this and they seem reluctant to make other changes in this regard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#issuecomment-1016536386
@papegaaij
Pls squash both commit. I have made the review but I prefer one commit in the final review. Thx.
Looks fine so far.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] slawekjaranowski commented on pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#issuecomment-998781866
please rebase with current code
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] papegaaij commented on pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
papegaaij commented on pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#issuecomment-920633143
The PR is updated to start a new LauncherSession for every iteration during the failure reruns.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#issuecomment-1017233013
Thx for contributing!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
Tibor17 edited a comment on pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#issuecomment-1016536386
@papegaaij
Pls squash both commits. I have made the review but I prefer one commit in the final review. Thx.
Looks fine so far.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#issuecomment-1016650449
LGTM
Some test has timed out in the `surefire-api` module but I am convinced it has nothing to do with this change.
Let's wait for the CI to complete.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] papegaaij commented on a change in pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
papegaaij commented on a change in pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#discussion_r788098364
##########
File path: surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java
##########
@@ -231,6 +225,21 @@ private void execute( TestsToRun testsToRun, RunListenerAdapter adapter )
} );
}
}
+
+ private void closeLauncher()
+ {
+ if ( launcher instanceof AutoCloseable )
+ {
+ try
+ {
+ ( (AutoCloseable) launcher ).close();
+ }
+ catch ( Exception e )
+ {
+ throw new SurefireReflectionException( e );
Review comment:
`SurefireReflectionException` only has one constructor, the one taking a `Throwable cause`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#discussion_r788253466
##########
File path: surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java
##########
@@ -231,6 +225,21 @@ private void execute( TestsToRun testsToRun, RunListenerAdapter adapter )
} );
}
}
+
+ private void closeLauncher()
+ {
+ if ( launcher instanceof AutoCloseable )
+ {
+ try
+ {
+ ( (AutoCloseable) launcher ).close();
+ }
+ catch ( Exception e )
+ {
+ throw new SurefireReflectionException( e );
Review comment:
@papegaaij
ok, no problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] papegaaij commented on pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
papegaaij commented on pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#issuecomment-919258184
> You should not take everything into one session. There are reruns which have to be isolated trom the main run. And I am convinced that the reruns also have to be isolated in each.
I did think about this, but as the contract of LauncherSession is very unclear about this, I decided to put it all in one session. The documentation of LauncherSession states `The LauncherSession API is the main entry point for client code that wishes to repeatedly discover and execute tests using one or more test engines.` IMHO, running the same test multiple times also falls under 'repeatedly discover and execute tests'. I guess this really depends on the way it is actually used which one is correct.
It will require some work to open a new session for each failure-iteration, but I can probably do that later this week.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] papegaaij commented on pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
papegaaij commented on pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#issuecomment-998812535
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] papegaaij commented on pull request #389: [SUREFIRE-1935] Upgrade to JUnit Platform 1.8, start Launcher via LauncherSession
Posted by GitBox <gi...@apache.org>.
papegaaij commented on pull request #389:
URL: https://github.com/apache/maven-surefire/pull/389#issuecomment-1015397834
Any progress on this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org