Skip to content

Commit 90a0ccf

Browse files
marcphilippmarcphilipp
committed
Call TestExecutionListener finish methods in reverse order
To allow for multiple listeners that depend on each other to be called in an order consistent with Jupiter's lifecycle callbacks. Issue: #3082
1 parent 87648fd commit 90a0ccf

File tree

5 files changed

+135
-51
lines changed

5 files changed

+135
-51
lines changed

‎junit-platform-commons/src/main/java/org/junit/platform/commons/util/CollectionUtils.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@
2424
import java.util.HashSet;
2525
import java.util.Iterator;
2626
import java.util.List;
27+
import java.util.ListIterator;
2728
import java.util.Set;
29+
import java.util.function.Consumer;
2830
import java.util.stream.Collector;
2931
import java.util.stream.DoubleStream;
3032
import java.util.stream.IntStream;
@@ -203,4 +205,14 @@ public static Stream<?> toStream(Object object) {
203205
"Cannot convert instance of " + object.getClass().getName() + " into a Stream: " + object);
204206
}
205207

208+
/**
209+
* Call the supplied action on each element of the supplied {@link List} from last to first element.
210+
*/
211+
@API(status = INTERNAL, since = "1.9.2")
212+
public static <T> void forEachInReverseOrder(List<T> list, Consumer<? super T> action) {
213+
for (ListIterator<T> iterator = list.listIterator(list.size()); iterator.hasPrevious();) {
214+
action.accept(iterator.previous());
215+
}
216+
}
217+
206218
}

‎junit-platform-launcher/src/main/java/org/junit/platform/launcher/core/CompositeTestExecutionListener.java

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,54 +43,61 @@ class CompositeTestExecutionListener implements TestExecutionListener {
4343

4444
@Override
4545
public void dynamicTestRegistered(TestIdentifier testIdentifier) {
46-
notifyEach(testExecutionListeners, listener -> listener.dynamicTestRegistered(testIdentifier),
46+
notifyEach(testExecutionListeners, IterationOrder.ORIGINAL,
47+
listener -> listener.dynamicTestRegistered(testIdentifier),
4748
() -> "dynamicTestRegistered(" + testIdentifier + ")");
4849
}
4950

5051
@Override
5152
public void executionSkipped(TestIdentifier testIdentifier, String reason) {
52-
notifyEach(testExecutionListeners, listener -> listener.executionSkipped(testIdentifier, reason),
53+
notifyEach(testExecutionListeners, IterationOrder.ORIGINAL,
54+
listener -> listener.executionSkipped(testIdentifier, reason),
5355
() -> "executionSkipped(" + testIdentifier + ", " + reason + ")");
5456
}
5557

5658
@Override
5759
public void executionStarted(TestIdentifier testIdentifier) {
58-
notifyEach(eagerTestExecutionListeners, listener -> listener.executionJustStarted(testIdentifier),
60+
notifyEach(eagerTestExecutionListeners, IterationOrder.ORIGINAL,
61+
listener -> listener.executionJustStarted(testIdentifier),
5962
() -> "executionJustStarted(" + testIdentifier + ")");
60-
notifyEach(testExecutionListeners, listener -> listener.executionStarted(testIdentifier),
61-
() -> "executionStarted(" + testIdentifier + ")");
63+
notifyEach(testExecutionListeners, IterationOrder.ORIGINAL,
64+
listener -> listener.executionStarted(testIdentifier), () -> "executionStarted(" + testIdentifier + ")");
6265
}
6366

6467
@Override
6568
public void executionFinished(TestIdentifier testIdentifier, TestExecutionResult testExecutionResult) {
66-
notifyEach(eagerTestExecutionListeners,
69+
notifyEach(eagerTestExecutionListeners, IterationOrder.REVERSED,
6770
listener -> listener.executionJustFinished(testIdentifier, testExecutionResult),
6871
() -> "executionJustFinished(" + testIdentifier + ", " + testExecutionResult + ")");
69-
notifyEach(testExecutionListeners, listener -> listener.executionFinished(testIdentifier, testExecutionResult),
72+
notifyEach(testExecutionListeners, IterationOrder.REVERSED,
73+
listener -> listener.executionFinished(testIdentifier, testExecutionResult),
7074
() -> "executionFinished(" + testIdentifier + ", " + testExecutionResult + ")");
7175
}
7276

7377
@Override
7478
public void testPlanExecutionStarted(TestPlan testPlan) {
75-
notifyEach(testExecutionListeners, listener -> listener.testPlanExecutionStarted(testPlan),
79+
notifyEach(testExecutionListeners, IterationOrder.ORIGINAL,
80+
listener -> listener.testPlanExecutionStarted(testPlan),
7681
() -> "testPlanExecutionStarted(" + testPlan + ")");
7782
}
7883

7984
@Override
8085
public void testPlanExecutionFinished(TestPlan testPlan) {
81-
notifyEach(testExecutionListeners, listener -> listener.testPlanExecutionFinished(testPlan),
86+
notifyEach(testExecutionListeners, IterationOrder.REVERSED,
87+
listener -> listener.testPlanExecutionFinished(testPlan),
8288
() -> "testPlanExecutionFinished(" + testPlan + ")");
8389
}
8490

8591
@Override
8692
public void reportingEntryPublished(TestIdentifier testIdentifier, ReportEntry entry) {
87-
notifyEach(testExecutionListeners, listener -> listener.reportingEntryPublished(testIdentifier, entry),
93+
notifyEach(testExecutionListeners, IterationOrder.ORIGINAL,
94+
listener -> listener.reportingEntryPublished(testIdentifier, entry),
8895
() -> "reportingEntryPublished(" + testIdentifier + ", " + entry + ")");
8996
}
9097

91-
private static <T extends TestExecutionListener> void notifyEach(List<T> listeners, Consumer<T> consumer,
92-
Supplier<String> description) {
93-
listeners.forEach(listener -> {
98+
private static <T extends TestExecutionListener> void notifyEach(List<T> listeners, IterationOrder iterationOrder,
99+
Consumer<T> consumer, Supplier<String> description) {
100+
iterationOrder.forEach(listeners, listener -> {
94101
try {
95102
consumer.accept(listener);
96103
}
@@ -109,4 +116,5 @@ default void executionJustStarted(TestIdentifier testIdentifier) {
109116
default void executionJustFinished(TestIdentifier testIdentifier, TestExecutionResult testExecutionResult) {
110117
}
111118
}
119+
112120
}

‎junit-platform-launcher/src/main/java/org/junit/platform/launcher/core/IterationOrder.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Copyright 2015-2023 the original author or authors.
3+
*
4+
* All rights reserved. This program and the accompanying materials are
5+
* made available under the terms of the Eclipse Public License v2.0 which
6+
* accompanies this distribution and is available at
7+
*
8+
* https://www.eclipse.org/legal/epl-v20.html
9+
*/
10+
11+
package org.junit.platform.launcher.core;
12+
13+
import static org.junit.platform.commons.util.CollectionUtils.forEachInReverseOrder;
14+
15+
import java.util.List;
16+
import java.util.function.Consumer;
17+
18+
enum IterationOrder {
19+
20+
ORIGINAL {
21+
@Override
22+
<T> void forEach(List<T> listeners, Consumer<T> action) {
23+
listeners.forEach(action);
24+
}
25+
},
26+
27+
REVERSED {
28+
@Override
29+
<T> void forEach(List<T> listeners, Consumer<T> action) {
30+
forEachInReverseOrder(listeners, action);
31+
}
32+
};
33+
34+
abstract <T> void forEach(List<T> listeners, Consumer<T> action);
35+
}

‎platform-tests/src/test/java/org/junit/platform/commons/util/CollectionUtilsTests.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,4 +277,13 @@ private void toStreamWithPrimitiveArray(Object primitiveArray) {
277277
}
278278
}
279279

280+
@Test
281+
void iteratesListElementsInReverseOrder() {
282+
var items = List.of("foo", "bar", "baz");
283+
var result = new ArrayList<>();
284+
285+
CollectionUtils.forEachInReverseOrder(items, result::add);
286+
287+
assertThat(result).containsExactly("baz", "bar", "foo");
288+
}
280289
}

‎platform-tests/src/test/java/org/junit/platform/launcher/core/CompositeTestExecutionListenerTests.java

Lines changed: 58 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import static org.assertj.core.api.Assertions.assertThat;
1414
import static org.assertj.core.api.Assertions.assertThatThrownBy;
15+
import static org.mockito.Mockito.inOrder;
1516
import static org.mockito.Mockito.mock;
1617

1718
import java.util.ArrayList;
@@ -33,6 +34,7 @@
3334
import org.junit.platform.launcher.TestIdentifier;
3435
import org.junit.platform.launcher.TestPlan;
3536
import org.junit.platform.launcher.core.CompositeTestExecutionListener.EagerTestExecutionListener;
37+
import org.mockito.InOrder;
3638

3739
@TrackLogRecords
3840
class CompositeTestExecutionListenerTests {
@@ -41,47 +43,37 @@ class CompositeTestExecutionListenerTests {
4143

4244
@Test
4345
void shouldNotThrowExceptionButLogIfDynamicTestRegisteredListenerMethodFails(LogRecordListener logRecordListener) {
44-
var testIdentifier = getSampleMethodTestIdentifier();
45-
46-
compositeTestExecutionListener().dynamicTestRegistered(testIdentifier);
46+
compositeTestExecutionListener().dynamicTestRegistered(anyTestIdentifier());
4747

4848
assertThatTestListenerErrorLogged(logRecordListener, ThrowingTestExecutionListener.class,
4949
"dynamicTestRegistered");
5050
}
5151

5252
@Test
5353
void shouldNotThrowExceptionButLogIfExecutionStartedListenerMethodFails(LogRecordListener logRecordListener) {
54-
var testIdentifier = getSampleMethodTestIdentifier();
55-
56-
compositeTestExecutionListener().executionStarted(testIdentifier);
54+
compositeTestExecutionListener().executionStarted(anyTestIdentifier());
5755

5856
assertThatTestListenerErrorLogged(logRecordListener, ThrowingTestExecutionListener.class, "executionStarted");
5957
}
6058

6159
@Test
6260
void shouldNotThrowExceptionButLogIfExecutionSkippedListenerMethodFails(LogRecordListener logRecordListener) {
63-
var testIdentifier = getSampleMethodTestIdentifier();
64-
65-
compositeTestExecutionListener().executionSkipped(testIdentifier, "deliberately skipped container");
61+
compositeTestExecutionListener().executionSkipped(anyTestIdentifier(), "deliberately skipped container");
6662

6763
assertThatTestListenerErrorLogged(logRecordListener, ThrowingTestExecutionListener.class, "executionSkipped");
6864
}
6965

7066
@Test
7167
void shouldNotThrowExceptionButLogIfExecutionFinishedListenerMethodFails(LogRecordListener logRecordListener) {
72-
var testIdentifier = getSampleMethodTestIdentifier();
73-
74-
compositeTestExecutionListener().executionFinished(testIdentifier, mock(TestExecutionResult.class));
68+
compositeTestExecutionListener().executionFinished(anyTestIdentifier(), anyTestExecutionResult());
7569

7670
assertThatTestListenerErrorLogged(logRecordListener, ThrowingTestExecutionListener.class, "executionFinished");
7771
}
7872

7973
@Test
8074
void shouldNotThrowExceptionButLogIfReportingEntryPublishedListenerMethodFails(
8175
LogRecordListener logRecordListener) {
82-
var testIdentifier = getSampleMethodTestIdentifier();
83-
84-
compositeTestExecutionListener().reportingEntryPublished(testIdentifier, ReportEntry.from("one", "two"));
76+
compositeTestExecutionListener().reportingEntryPublished(anyTestIdentifier(), ReportEntry.from("one", "two"));
8577

8678
assertThatTestListenerErrorLogged(logRecordListener, ThrowingTestExecutionListener.class,
8779
"reportingEntryPublished");
@@ -90,10 +82,7 @@ void shouldNotThrowExceptionButLogIfReportingEntryPublishedListenerMethodFails(
9082
@Test
9183
void shouldNotThrowExceptionButLogIfTesPlanExecutionStartedListenerMethodFails(
9284
LogRecordListener logRecordListener) {
93-
var testDescriptor = getDemoMethodTestDescriptor();
94-
95-
compositeTestExecutionListener().testPlanExecutionStarted(
96-
TestPlan.from(Set.of(testDescriptor), mock(ConfigurationParameters.class)));
85+
compositeTestExecutionListener().testPlanExecutionStarted(anyTestPlan());
9786

9887
assertThatTestListenerErrorLogged(logRecordListener, ThrowingTestExecutionListener.class,
9988
"testPlanExecutionStarted");
@@ -102,10 +91,7 @@ void shouldNotThrowExceptionButLogIfTesPlanExecutionStartedListenerMethodFails(
10291
@Test
10392
void shouldNotThrowExceptionButLogIfTesPlanExecutionFinishedListenerMethodFails(
10493
LogRecordListener logRecordListener) {
105-
var testDescriptor = getDemoMethodTestDescriptor();
106-
107-
compositeTestExecutionListener().testPlanExecutionFinished(
108-
TestPlan.from(Set.of(testDescriptor), mock(ConfigurationParameters.class)));
94+
compositeTestExecutionListener().testPlanExecutionFinished(anyTestPlan());
10995

11096
assertThatTestListenerErrorLogged(logRecordListener, ThrowingTestExecutionListener.class,
11197
"testPlanExecutionFinished");
@@ -116,8 +102,7 @@ void shouldNotThrowExceptionButLogIfExecutionJustStartedEagerTestListenerMethodF
116102
LogRecordListener logRecordListener) {
117103
listeners.add(new ThrowingEagerTestExecutionListener());
118104

119-
var testIdentifier = getSampleMethodTestIdentifier();
120-
compositeTestExecutionListener().executionStarted(testIdentifier);
105+
compositeTestExecutionListener().executionStarted(anyTestIdentifier());
121106

122107
assertThatTestListenerErrorLogged(logRecordListener, ThrowingEagerTestExecutionListener.class,
123108
"executionJustStarted");
@@ -128,8 +113,7 @@ void shouldNotThrowExceptionButLogIfExecutionJustFinishedEagerTestListenerMethod
128113
LogRecordListener logRecordListener) {
129114
listeners.add(new ThrowingEagerTestExecutionListener());
130115

131-
var testIdentifier = getSampleMethodTestIdentifier();
132-
compositeTestExecutionListener().executionFinished(testIdentifier, mock(TestExecutionResult.class));
116+
compositeTestExecutionListener().executionFinished(anyTestIdentifier(), anyTestExecutionResult());
133117

134118
assertThatTestListenerErrorLogged(logRecordListener, ThrowingEagerTestExecutionListener.class,
135119
"executionJustFinished");
@@ -144,8 +128,8 @@ public void executionStarted(TestIdentifier testIdentifier) {
144128
throw new OutOfMemoryError();
145129
}
146130
});
147-
var testIdentifier = getSampleMethodTestIdentifier();
148-
assertThatThrownBy(() -> compositeTestExecutionListener().executionStarted(testIdentifier)).isInstanceOf(
131+
132+
assertThatThrownBy(() -> compositeTestExecutionListener().executionStarted(anyTestIdentifier())).isInstanceOf(
149133
OutOfMemoryError.class);
150134

151135
assertNotLogs(logRecordListener);
@@ -159,13 +143,42 @@ public void executionJustStarted(TestIdentifier testIdentifier) {
159143
throw new OutOfMemoryError();
160144
}
161145
});
162-
var testIdentifier = getSampleMethodTestIdentifier();
163-
assertThatThrownBy(() -> compositeTestExecutionListener().executionStarted(testIdentifier)).isInstanceOf(
146+
147+
assertThatThrownBy(() -> compositeTestExecutionListener().executionStarted(anyTestIdentifier())).isInstanceOf(
164148
OutOfMemoryError.class);
165149

166150
assertNotLogs(logRecordListener);
167151
}
168152

153+
@Test
154+
void callsListenersInReverseOrderForFinishedEvents() {
155+
listeners.clear();
156+
var firstListener = mock(TestExecutionListener.class, "firstListener");
157+
var secondListener = mock(TestExecutionListener.class, "secondListener");
158+
listeners.add(firstListener);
159+
listeners.add(secondListener);
160+
161+
var testPlan = anyTestPlan();
162+
var testIdentifier = anyTestIdentifier();
163+
var testExecutionResult = anyTestExecutionResult();
164+
165+
var composite = compositeTestExecutionListener();
166+
composite.testPlanExecutionStarted(testPlan);
167+
composite.executionStarted(testIdentifier);
168+
composite.executionFinished(testIdentifier, testExecutionResult);
169+
composite.testPlanExecutionFinished(testPlan);
170+
171+
InOrder inOrder = inOrder(firstListener, secondListener);
172+
inOrder.verify(firstListener).testPlanExecutionStarted(testPlan);
173+
inOrder.verify(secondListener).testPlanExecutionStarted(testPlan);
174+
inOrder.verify(firstListener).executionStarted(testIdentifier);
175+
inOrder.verify(secondListener).executionStarted(testIdentifier);
176+
inOrder.verify(secondListener).executionFinished(testIdentifier, testExecutionResult);
177+
inOrder.verify(firstListener).executionFinished(testIdentifier, testExecutionResult);
178+
inOrder.verify(secondListener).testPlanExecutionFinished(testPlan);
179+
inOrder.verify(firstListener).testPlanExecutionFinished(testPlan);
180+
}
181+
169182
private TestExecutionListener compositeTestExecutionListener() {
170183
return new CompositeTestExecutionListener(listeners);
171184
}
@@ -179,9 +192,12 @@ private void assertNotLogs(LogRecordListener logRecordListener) throws Assertion
179192
assertThat(logRecordListener.stream(CompositeTestExecutionListener.class, Level.WARNING).count()).isZero();
180193
}
181194

182-
private TestIdentifier getSampleMethodTestIdentifier() {
183-
var demoMethodTestDescriptor = getDemoMethodTestDescriptor();
184-
return TestIdentifier.from(demoMethodTestDescriptor);
195+
private static TestExecutionResult anyTestExecutionResult() {
196+
return TestExecutionResult.successful();
197+
}
198+
199+
private static TestIdentifier anyTestIdentifier() {
200+
return TestIdentifier.from(anyTestDescriptor());
185201
}
186202

187203
private void assertThatTestListenerErrorLogged(LogRecordListener logRecordListener, Class<?> listenerClass,
@@ -190,10 +206,14 @@ private void assertThatTestListenerErrorLogged(LogRecordListener logRecordListen
190206
"TestExecutionListener [" + listenerClass.getName() + "] threw exception for method: " + methodName);
191207
}
192208

193-
private DemoMethodTestDescriptor getDemoMethodTestDescriptor() {
194-
var method = ReflectionUtils.findMethod(this.getClass(), "getDemoMethodTestDescriptor",
195-
new Class<?>[0]).orElseThrow();
196-
return new DemoMethodTestDescriptor(UniqueId.root("method", "unique_id"), this.getClass(), method);
209+
private static TestPlan anyTestPlan() {
210+
return TestPlan.from(Set.of(anyTestDescriptor()), mock(ConfigurationParameters.class));
211+
}
212+
213+
private static DemoMethodTestDescriptor anyTestDescriptor() {
214+
var testClass = CompositeTestExecutionListenerTests.class;
215+
var method = ReflectionUtils.findMethod(testClass, "anyTestDescriptor", new Class<?>[0]).orElseThrow();
216+
return new DemoMethodTestDescriptor(UniqueId.root("method", "unique_id"), testClass, method);
197217
}
198218

199219
private static class ThrowingEagerTestExecutionListener extends ThrowingTestExecutionListener

0 commit comments

Comments
 (0)