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.

Sign up for GitHub

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

Jump to bottom

Document concurrency considerations for TestExecutionListener implementations #2539

Closed
jaikiran opened this issue Jan 24, 2021 · 6 comments · Fixed by #3053
Closed

Document concurrency considerations for TestExecutionListener implementations #2539

jaikiran opened this issue Jan 24, 2021 · 6 comments · Fixed by #3053
Assignees
@edysli
Labels
component: Platform theme: concurrency theme: documentation
Milestone
5.10 M1

Comments

@jaikiran
Copy link
Contributor

jaikiran commented Jan 24, 2021 •
edited

I've been looking at some documentations to see if there is any thread safety semantics to consider when implementing a org.junit.platform.launcher.TestExecutionListener. I haven't found any, so asking here.

More specifically, is there ever a case where the same instance of TestExecutionListener could get invoked by multiple threads (simultaneously), such that these implementations need to handle any thread safety issues? If so, is it only for certain methods or do all methods on that interface's implementation need to take into account this aspect? For example, in theory, I wouldn't expect more than one thread invoking the testPlanExecutionFinished method.
@marcphilipp
Copy link
Member

Good point, we should document that. testPlanExecutionStarted and testPlanExecutionFinished are always called from the same thread and it's safe to assume there's at most one TestPlan at a time (in theory, someone could call the Launcher concurrently but AFAIK no tool does that). All other method could be called from different threads concurrently in case one or multiple test engines execute tests in parallel.

@marcphilipp marcphilipp added this to the 5.x Backlog milestone Jan 24, 2021
@sbrannen sbrannen changed the title Should implementations of TestExecutionListener worry about thread safety issues? Document concurrency considerations for TestExecutionListener implementations Jan 24, 2021
@jaikiran
Copy link
Contributor Author

@marcphilipp, thank you. That answer helps me decide how to go about one of the implementations in Apache Ant.

@marcphilipp marcphilipp removed this from the 5.x Backlog milestone Jun 19, 2021
simkam added a commit to simkam/xtf that referenced this issue Aug 6, 2021
@simkam
make TestExecutionLogger thread safe
c82cc85 
to avoid races like
```
Aug 06, 2021 1:23:55 PM org.junit.platform.launcher.core.TestExecutionListenerRegistry lambda$notifyEach$1
WARNING: TestExecutionListener [cz.xtf.junit5.listeners.TestExecutionLogger] threw exception for method: executionFinished(TestIdentifier [uniqueId = '[engine:junit-jupiter]/[class:com.redhat.xpaas.eap.microprofile.health.MPHealthUpTest]/[method:healthCheckUpTest()]', parentId = '[engine:junit-jupiter]/[class:com.redhat.xpaas.eap.microprofile.health.MPHealthUpTest]', displayName = 'healthCheckUpTest()', legacyReportingName = 'healthCheckUpTest()', source = MethodSource [className = 'com.redhat.xpaas.eap.microprofile.health.MPHealthUpTest', methodName = 'healthCheckUpTest', methodParameterTypes = ''], tags = [level2, openshift-api], type = TEST], TestExecutionResult [status = SUCCESSFUL, throwable = null])
java.lang.NullPointerException
        at cz.xtf.junit5.listeners.TestExecutionLogger.executionFinished(TestExecutionLogger.java:39)
        at org.junit.platform.launcher.core.TestExecutionListenerRegistry$CompositeTestExecutionListener.lambda$executionFinished$10(TestExecutionListenerRegistry.java:109)
        at org.junit.platform.launcher.core.TestExecutionListenerRegistry.lambda$notifyEach$1(TestExecutionListenerRegistry.java:67)
        at java.util.ArrayList.forEach(ArrayList.java:1257)
        at org.junit.platform.launcher.core.TestExecutionListenerRegistry.notifyEach(TestExecutionListenerRegistry.java:65)
        at org.junit.platform.launcher.core.TestExecutionListenerRegistry.access$200(TestExecutionListenerRegistry.java:32)
        at org.junit.platform.launcher.core.TestExecutionListenerRegistry$CompositeTestExecutionListener.executionFinished(TestExecutionListenerRegistry.java:108)
        at org.junit.platform.launcher.core.ExecutionListenerAdapter.executionFinished(ExecutionListenerAdapter.java:56)
        at org.junit.platform.launcher.core.DelegatingEngineExecutionListener.executionFinished(DelegatingEngineExecutionListener.java:46)
        at org.junit.platform.launcher.core.OutcomeDelayingEngineExecutionListener.executionFinished(OutcomeDelayingEngineExecutionListener.java:63)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.reportCompletion(NodeTestTask.java:183)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:89)
        at org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:185)
        at java.util.concurrent.RecursiveAction.exec(RecursiveAction.java:189)
        at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
        at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
        at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
        at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)
```

According to junit-team/junit5#2539 (comment), only `testPlanExecutionStarted` and `testPlanExecutionFinished` are always called from the same thread. All other method could be called from different threads concurrently in case one or multiple test engines execute tests in parallel.
@simkam simkam mentioned this issue Aug 6, 2021
Merged
4 tasks
mnovak1 pushed a commit to xtf-cz/xtf that referenced this issue Aug 6, 2021
@simkam @mnovak1
make TestExecutionLogger thread safe
de248a4 
to avoid races like
```
Aug 06, 2021 1:23:55 PM org.junit.platform.launcher.core.TestExecutionListenerRegistry lambda$notifyEach$1
WARNING: TestExecutionListener [cz.xtf.junit5.listeners.TestExecutionLogger] threw exception for method: executionFinished(TestIdentifier [uniqueId = '[engine:junit-jupiter]/[class:com.redhat.xpaas.eap.microprofile.health.MPHealthUpTest]/[method:healthCheckUpTest()]', parentId = '[engine:junit-jupiter]/[class:com.redhat.xpaas.eap.microprofile.health.MPHealthUpTest]', displayName = 'healthCheckUpTest()', legacyReportingName = 'healthCheckUpTest()', source = MethodSource [className = 'com.redhat.xpaas.eap.microprofile.health.MPHealthUpTest', methodName = 'healthCheckUpTest', methodParameterTypes = ''], tags = [level2, openshift-api], type = TEST], TestExecutionResult [status = SUCCESSFUL, throwable = null])
java.lang.NullPointerException
        at cz.xtf.junit5.listeners.TestExecutionLogger.executionFinished(TestExecutionLogger.java:39)
        at org.junit.platform.launcher.core.TestExecutionListenerRegistry$CompositeTestExecutionListener.lambda$executionFinished$10(TestExecutionListenerRegistry.java:109)
        at org.junit.platform.launcher.core.TestExecutionListenerRegistry.lambda$notifyEach$1(TestExecutionListenerRegistry.java:67)
        at java.util.ArrayList.forEach(ArrayList.java:1257)
        at org.junit.platform.launcher.core.TestExecutionListenerRegistry.notifyEach(TestExecutionListenerRegistry.java:65)
        at org.junit.platform.launcher.core.TestExecutionListenerRegistry.access$200(TestExecutionListenerRegistry.java:32)
        at org.junit.platform.launcher.core.TestExecutionListenerRegistry$CompositeTestExecutionListener.executionFinished(TestExecutionListenerRegistry.java:108)
        at org.junit.platform.launcher.core.ExecutionListenerAdapter.executionFinished(ExecutionListenerAdapter.java:56)
        at org.junit.platform.launcher.core.DelegatingEngineExecutionListener.executionFinished(DelegatingEngineExecutionListener.java:46)
        at org.junit.platform.launcher.core.OutcomeDelayingEngineExecutionListener.executionFinished(OutcomeDelayingEngineExecutionListener.java:63)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.reportCompletion(NodeTestTask.java:183)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:89)
        at org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:185)
        at java.util.concurrent.RecursiveAction.exec(RecursiveAction.java:189)
        at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
        at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
        at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
        at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)
```

According to junit-team/junit5#2539 (comment), only `testPlanExecutionStarted` and `testPlanExecutionFinished` are always called from the same thread. All other method could be called from different threads concurrently in case one or multiple test engines execute tests in parallel.
@stale
Copy link

stale bot commented Jun 21, 2022

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the status: stale label Jun 21, 2022
@stale
Copy link

stale bot commented Jul 12, 2022

This issue has been automatically closed due to inactivity. If you have a good use case for this feature, please feel free to reopen the issue.

@stale stale bot closed this as completed Jul 12, 2022
@jbduncan
Copy link
Contributor

The proposed documentation change sounds useful to me, so I wonder if this issue should be re-opened?

@sbrannen sbrannen reopened this Jul 13, 2022
@edysli
Copy link
Contributor

edysli commented Oct 4, 2022

I'd like to give this one a try. :)
Is updating the Javadoc on org.junit.platform.launcher.TestExecutionListener enough, or is there another guide to update as well?

edysli added a commit to edysli/junit5 that referenced this issue Oct 6, 2022
@edysli @marcphilipp
Document concurrency for TestExecutionListener implementations
7e90169 
Add a note about concurrency considerations for
`TestExecutionListener` implementations to this interface's Javadoc.

Issue: junit-team#2539
Co-authored-by: Marc Philipp <mail@marcphilipp.de>
@edysli edysli mentioned this issue Oct 6, 2022
Merged
7 tasks
sormuras pushed a commit that referenced this issue Oct 10, 2022
@edysli @marcphilipp @sormuras
Document concurrency for TestExecutionListener implementations
4a87980 
Add a note about concurrency considerations for
`TestExecutionListener` implementations to this interface's Javadoc.

Issue: #2539
Co-authored-by: Marc Philipp <mail@marcphilipp.de>
@sbrannen sbrannen modified the milestones: 5.10, 5.10 M1 Oct 10, 2022
marcphilipp added a commit that referenced this issue Jan 6, 2023
@edysli @marcphilipp
Document concurrency for TestExecutionListener implementations
5ec6242 
Add a note about concurrency considerations for
`TestExecutionListener` implementations to this interface's Javadoc.

Issue: #2539
Co-authored-by: Marc Philipp <mail@marcphilipp.de>
@sbrannen sbrannen changed the title Document concurrency considerations for TestExecutionListener implementations Document concurrency considerations for TestExecutionListener implementations Apr 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@edysli edysli

Labels
component: Platform theme: concurrency theme: documentation
Projects
None yet
Milestone
5.10 M1
Development

Successfully merging a pull request may close this issue.

Document concurrency considerations for TestExecutionListener implementations edysli/junit5
5 participants
@sbrannen @jaikiran @marcphilipp @edysli @jbduncan