Skip to content

Commit

Permalink
Call EngineExecutionListener.executionFinished method in reverse order
Browse files Browse the repository at this point in the history 
Issue: #3082
  • Loading branch information
@marcphilipp
marcphilipp committed Jan 10, 2023
1 parent 90a0ccf commit 3365072
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 33 deletions.
21 changes: 12 additions & 9 deletions ...cher/src/main/java/org/junit/platform/launcher/core/CompositeEngineExecutionListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,38 +35,41 @@ class CompositeEngineExecutionListener implements EngineExecutionListener {

@Override
public void dynamicTestRegistered(TestDescriptor testDescriptor) {
notifyEach(engineExecutionListeners, listener -> listener.dynamicTestRegistered(testDescriptor),
notifyEach(engineExecutionListeners, IterationOrder.ORIGINAL,
listener -> listener.dynamicTestRegistered(testDescriptor),
() -> "dynamicTestRegistered(" + testDescriptor + ")");
}

@Override
public void executionSkipped(TestDescriptor testDescriptor, String reason) {
notifyEach(engineExecutionListeners, listener -> listener.executionSkipped(testDescriptor, reason),
notifyEach(engineExecutionListeners, IterationOrder.ORIGINAL,
listener -> listener.executionSkipped(testDescriptor, reason),
() -> "executionSkipped(" + testDescriptor + ", " + reason + ")");
}

@Override
public void executionStarted(TestDescriptor testDescriptor) {
notifyEach(engineExecutionListeners, listener -> listener.executionStarted(testDescriptor),
() -> "executionStarted(" + testDescriptor + ")");
notifyEach(engineExecutionListeners, IterationOrder.ORIGINAL,
listener -> listener.executionStarted(testDescriptor), () -> "executionStarted(" + testDescriptor + ")");
}

@Override
public void executionFinished(TestDescriptor testDescriptor, TestExecutionResult testExecutionResult) {
notifyEach(engineExecutionListeners,
notifyEach(engineExecutionListeners, IterationOrder.REVERSED,
listener -> listener.executionFinished(testDescriptor, testExecutionResult),
() -> "executionFinished(" + testDescriptor + ", " + testExecutionResult + ")");
}

@Override
public void reportingEntryPublished(TestDescriptor testDescriptor, ReportEntry entry) {
notifyEach(engineExecutionListeners, listener -> listener.reportingEntryPublished(testDescriptor, entry),
notifyEach(engineExecutionListeners, IterationOrder.ORIGINAL,
listener -> listener.reportingEntryPublished(testDescriptor, entry),
() -> "reportingEntryPublished(" + testDescriptor + ", " + entry + ")");
}

private static <T extends EngineExecutionListener> void notifyEach(List<T> listeners, Consumer<T> consumer,
Supplier<String> description) {
listeners.forEach(listener -> {
private static <T extends EngineExecutionListener> void notifyEach(List<T> listeners, IterationOrder iterationOrder,
Consumer<T> consumer, Supplier<String> description) {
iterationOrder.forEach(listeners, listener -> {
try {
consumer.accept(listener);
}
Expand Down
63 changes: 39 additions & 24 deletions ...src/test/java/org/junit/platform/launcher/core/CompositeEngineExecutionListenerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;

import java.util.ArrayList;
Expand All @@ -29,6 +30,7 @@
import org.junit.platform.engine.UniqueId;
import org.junit.platform.engine.reporting.ReportEntry;
import org.junit.platform.engine.support.descriptor.DemoMethodTestDescriptor;
import org.mockito.InOrder;

@TrackLogRecords
class CompositeEngineExecutionListenerTests {
Expand All @@ -38,37 +40,29 @@ class CompositeEngineExecutionListenerTests {

@Test
void shouldNotThrowExceptionButLogIfDynamicTestRegisteredListenerMethodFails(LogRecordListener logRecordListener) {
var testDescriptor = getSampleMethodTestDescriptor();

compositeTestExecutionListener().dynamicTestRegistered(testDescriptor);
compositeEngineExecutionListener().dynamicTestRegistered(anyTestDescriptor());

assertThatTestListenerErrorLogged(logRecordListener, ThrowingEngineExecutionListener.class,
"dynamicTestRegistered");
}

@Test
void shouldNotThrowExceptionButLogIfExecutionStartedListenerMethodFails(LogRecordListener logRecordListener) {
var testDescriptor = getSampleMethodTestDescriptor();

compositeTestExecutionListener().executionStarted(testDescriptor);
compositeEngineExecutionListener().executionStarted(anyTestDescriptor());

assertThatTestListenerErrorLogged(logRecordListener, ThrowingEngineExecutionListener.class, "executionStarted");
}

@Test
void shouldNotThrowExceptionButLogIfExecutionSkippedListenerMethodFails(LogRecordListener logRecordListener) {
var testDescriptor = getSampleMethodTestDescriptor();

compositeTestExecutionListener().executionSkipped(testDescriptor, "deliberately skipped container");
compositeEngineExecutionListener().executionSkipped(anyTestDescriptor(), "deliberately skipped container");

assertThatTestListenerErrorLogged(logRecordListener, ThrowingEngineExecutionListener.class, "executionSkipped");
}

@Test
void shouldNotThrowExceptionButLogIfExecutionFinishedListenerMethodFails(LogRecordListener logRecordListener) {
var testDescriptor = getSampleMethodTestDescriptor();

compositeTestExecutionListener().executionFinished(testDescriptor, mock(TestExecutionResult.class));
compositeEngineExecutionListener().executionFinished(anyTestDescriptor(), anyTestExecutionResult());

assertThatTestListenerErrorLogged(logRecordListener, ThrowingEngineExecutionListener.class,
"executionFinished");
Expand All @@ -77,9 +71,7 @@ void shouldNotThrowExceptionButLogIfExecutionFinishedListenerMethodFails(LogReco
@Test
void shouldNotThrowExceptionButLogIfReportingEntryPublishedListenerMethodFails(
LogRecordListener logRecordListener) {
var testDescriptor = getSampleMethodTestDescriptor();

compositeTestExecutionListener().reportingEntryPublished(testDescriptor, ReportEntry.from("one", "two"));
compositeEngineExecutionListener().reportingEntryPublished(anyTestDescriptor(), ReportEntry.from("one", "two"));

assertThatTestListenerErrorLogged(logRecordListener, ThrowingEngineExecutionListener.class,
"reportingEntryPublished");
Expand All @@ -94,14 +86,37 @@ public void executionStarted(TestDescriptor testDescriptor) {
throw new OutOfMemoryError();
}
});
var testDescriptor = getSampleMethodTestDescriptor();
assertThatThrownBy(() -> compositeTestExecutionListener().executionStarted(testDescriptor)).isInstanceOf(
var testDescriptor = anyTestDescriptor();

assertThatThrownBy(() -> compositeEngineExecutionListener().executionStarted(testDescriptor)).isInstanceOf(
OutOfMemoryError.class);

assertNotLogs(logRecordListener);
}

private EngineExecutionListener compositeTestExecutionListener() {
@Test
void callsListenersInReverseOrderForFinishedEvents() {
listeners.clear();
var firstListener = mock(EngineExecutionListener.class, "firstListener");
var secondListener = mock(EngineExecutionListener.class, "secondListener");
listeners.add(firstListener);
listeners.add(secondListener);

var testDescriptor = anyTestDescriptor();
var testExecutionResult = anyTestExecutionResult();

var composite = compositeEngineExecutionListener();
composite.executionStarted(testDescriptor);
composite.executionFinished(testDescriptor, testExecutionResult);

InOrder inOrder = inOrder(firstListener, secondListener);
inOrder.verify(firstListener).executionStarted(testDescriptor);
inOrder.verify(secondListener).executionStarted(testDescriptor);
inOrder.verify(secondListener).executionFinished(testDescriptor, testExecutionResult);
inOrder.verify(firstListener).executionFinished(testDescriptor, testExecutionResult);
}

private EngineExecutionListener compositeEngineExecutionListener() {
return new CompositeEngineExecutionListener(listeners);
}

Expand All @@ -114,8 +129,8 @@ private void assertNotLogs(LogRecordListener logRecordListener) throws Assertion
assertThat(logRecordListener.stream(CompositeEngineExecutionListener.class, Level.WARNING).count()).isZero();
}

private TestDescriptor getSampleMethodTestDescriptor() {
return getDemoMethodTestDescriptor();
private static TestExecutionResult anyTestExecutionResult() {
return mock(TestExecutionResult.class);
}

private void assertThatTestListenerErrorLogged(LogRecordListener logRecordListener, Class<?> listenerClass,
Expand All @@ -124,10 +139,10 @@ private void assertThatTestListenerErrorLogged(LogRecordListener logRecordListen
"EngineExecutionListener [" + listenerClass.getName() + "] threw exception for method: " + methodName);
}

private DemoMethodTestDescriptor getDemoMethodTestDescriptor() {
var method = ReflectionUtils.findMethod(this.getClass(), "getDemoMethodTestDescriptor",
new Class<?>[0]).orElseThrow();
return new DemoMethodTestDescriptor(UniqueId.root("method", "unique_id"), this.getClass(), method);
private static TestDescriptor anyTestDescriptor() {
var testClass = CompositeEngineExecutionListenerTests.class;
var method = ReflectionUtils.findMethod(testClass, "anyTestDescriptor", new Class<?>[0]).orElseThrow();
return new DemoMethodTestDescriptor(UniqueId.root("method", "unique_id"), testClass, method);
}

private static class ThrowingEngineExecutionListener implements EngineExecutionListener {
Expand Down

0 comments on commit 3365072

Please sign in to comment.