Skip to content

Commit 1e7fc96

Browse files
marcphilippmarcphilipp
committed
Report intermediate containers finished lazily
Since JUnit 4 does report events for intermediate containers, the Vintage engine has to synthesize them. Until now this was done lazily for start events, i.e. when the first child was started, but eagerly for finish events, i.e. when the last static child of a container was finished. However, the latter approach is problematic for containers that contain both static and dynamic children, for example a Spock specification with a regular and an unrolled feature method. Instead, intermediate containers are now reported as finished as soon as a test is started for which the container is not an ancestor. Fixes #1819.
1 parent 7312743 commit 1e7fc96

File tree

5 files changed

+184
-23
lines changed

5 files changed

+184
-23
lines changed

‎junit-vintage-engine/src/main/java/org/junit/vintage/engine/execution/EventType.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/*
2+
* Copyright 2015-2019 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+
* http://www.eclipse.org/legal/epl-v20.html
9+
*/
10+
11+
package org.junit.vintage.engine.execution;
12+
13+
/**
14+
* @since 5.4.1
15+
*/
16+
enum EventType {
17+
REPORTED, SYNTHETIC
18+
}

‎junit-vintage-engine/src/main/java/org/junit/vintage/engine/execution/RunListenerAdapter.java‎

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class RunListenerAdapter extends RunListener {
4747
@Override
4848
public void testRunStarted(Description description) {
4949
if (description.isSuite() && description.getAnnotation(Ignore.class) == null) {
50-
fireExecutionStarted(testRun.getRunnerTestDescriptor());
50+
fireExecutionStarted(testRun.getRunnerTestDescriptor(), EventType.REPORTED);
5151
}
5252
}
5353

@@ -58,7 +58,7 @@ public void testIgnored(Description description) {
5858

5959
@Override
6060
public void testStarted(Description description) {
61-
testStarted(lookupOrRegisterTestDescriptor(description));
61+
testStarted(lookupOrRegisterTestDescriptor(description), EventType.REPORTED);
6262
}
6363

6464
@Override
@@ -81,8 +81,11 @@ public void testRunFinished(Result result) {
8181
RunnerTestDescriptor runnerTestDescriptor = testRun.getRunnerTestDescriptor();
8282
if (testRun.isNotSkipped(runnerTestDescriptor)) {
8383
if (testRun.isNotStarted(runnerTestDescriptor)) {
84-
fireExecutionStarted(runnerTestDescriptor);
84+
fireExecutionStarted(runnerTestDescriptor, EventType.SYNTHETIC);
8585
}
86+
testRun.getInProgressTestDescriptorsWithSyntheticStartEvents().stream() //
87+
.filter(this::canFinish) //
88+
.forEach(this::fireExecutionFinished);
8689
if (testRun.isNotFinished(runnerTestDescriptor)) {
8790
fireExecutionFinished(runnerTestDescriptor);
8891
}
@@ -128,17 +131,17 @@ private void handleFailure(Failure failure, Function<Throwable, TestExecutionRes
128131

129132
private void fireMissingContainerEvents(TestDescriptor testDescriptor) {
130133
if (testRun.isNotStarted(testDescriptor)) {
131-
testStarted(testDescriptor);
134+
testStarted(testDescriptor, EventType.SYNTHETIC);
132135
}
133136
if (testRun.isNotFinished(testDescriptor)) {
134137
testFinished(testDescriptor);
135138
}
136139
}
137140

138141
private void testIgnored(TestDescriptor testDescriptor, String reason) {
142+
fireExecutionFinishedForInProgressNonAncestorTestDescriptorsWithSyntheticStartEvents(testDescriptor);
139143
fireExecutionStartedIncludingUnstartedAncestors(testDescriptor.getParent());
140144
fireExecutionSkipped(testDescriptor, reason);
141-
fireExecutionFinishedIncludingAncestorsWithoutPendingChildren(testDescriptor.getParent());
142145
}
143146

144147
private String determineReasonForIgnoredTest(Description description) {
@@ -151,27 +154,38 @@ private void dynamicTestRegistered(TestDescriptor testDescriptor) {
151154
listener.dynamicTestRegistered(testDescriptor);
152155
}
153156

154-
private void testStarted(TestDescriptor testDescriptor) {
157+
private void testStarted(TestDescriptor testDescriptor, EventType eventType) {
158+
fireExecutionFinishedForInProgressNonAncestorTestDescriptorsWithSyntheticStartEvents(testDescriptor);
155159
fireExecutionStartedIncludingUnstartedAncestors(testDescriptor.getParent());
156-
fireExecutionStarted(testDescriptor);
160+
fireExecutionStarted(testDescriptor, eventType);
161+
}
162+
163+
private void fireExecutionFinishedForInProgressNonAncestorTestDescriptorsWithSyntheticStartEvents(
164+
TestDescriptor testDescriptor) {
165+
testRun.getInProgressTestDescriptorsWithSyntheticStartEvents().stream() //
166+
.filter(it -> !isAncestor(it, testDescriptor) && canFinish(it)) //
167+
.forEach(this::fireExecutionFinished);
168+
}
169+
170+
private boolean isAncestor(TestDescriptor candidate, TestDescriptor testDescriptor) {
171+
Optional<TestDescriptor> parent = testDescriptor.getParent();
172+
if (!parent.isPresent()) {
173+
return false;
174+
}
175+
if (parent.get().equals(candidate)) {
176+
return true;
177+
}
178+
return isAncestor(candidate, parent.get());
157179
}
158180

159181
private void testFinished(TestDescriptor descriptor) {
160182
fireExecutionFinished(descriptor);
161-
fireExecutionFinishedIncludingAncestorsWithoutPendingChildren(descriptor.getParent());
162183
}
163184

164185
private void fireExecutionStartedIncludingUnstartedAncestors(Optional<TestDescriptor> parent) {
165186
if (parent.isPresent() && canStart(parent.get())) {
166187
fireExecutionStartedIncludingUnstartedAncestors(parent.get().getParent());
167-
fireExecutionStarted(parent.get());
168-
}
169-
}
170-
171-
private void fireExecutionFinishedIncludingAncestorsWithoutPendingChildren(Optional<TestDescriptor> parent) {
172-
if (parent.isPresent() && canFinish(parent.get())) {
173-
fireExecutionFinished(parent.get());
174-
fireExecutionFinishedIncludingAncestorsWithoutPendingChildren(parent.get().getParent());
188+
fireExecutionStarted(parent.get(), EventType.SYNTHETIC);
175189
}
176190
}
177191

@@ -192,8 +206,8 @@ private void fireExecutionSkipped(TestDescriptor testDescriptor, String reason)
192206
listener.executionSkipped(testDescriptor, reason);
193207
}
194208

195-
private void fireExecutionStarted(TestDescriptor testDescriptor) {
196-
testRun.markStarted(testDescriptor);
209+
private void fireExecutionStarted(TestDescriptor testDescriptor, EventType eventType) {
210+
testRun.markStarted(testDescriptor, eventType);
197211
listener.executionStarted(testDescriptor);
198212
}
199213

‎junit-vintage-engine/src/main/java/org/junit/vintage/engine/execution/TestRun.java‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,14 @@
1919
import static org.junit.platform.engine.TestExecutionResult.successful;
2020

2121
import java.util.ArrayList;
22+
import java.util.Collection;
23+
import java.util.Collections;
2224
import java.util.HashMap;
2325
import java.util.LinkedHashMap;
2426
import java.util.LinkedHashSet;
2527
import java.util.List;
2628
import java.util.Map;
29+
import java.util.Map.Entry;
2730
import java.util.Optional;
2831
import java.util.Set;
2932
import java.util.concurrent.CopyOnWriteArrayList;
@@ -46,7 +49,7 @@ class TestRun {
4649
private final Map<Description, List<VintageTestDescriptor>> descriptionToDescriptors;
4750
private final Map<TestDescriptor, List<TestExecutionResult>> executionResults = new LinkedHashMap<>();
4851
private final Set<TestDescriptor> skippedDescriptors = new LinkedHashSet<>();
49-
private final Set<TestDescriptor> startedDescriptors = new LinkedHashSet<>();
52+
private final Map<TestDescriptor, EventType> startedDescriptors = new LinkedHashMap<>();
5053
private final Set<TestDescriptor> finishedDescriptors = new LinkedHashSet<>();
5154

5255
TestRun(RunnerTestDescriptor runnerTestDescriptor) {
@@ -70,6 +73,16 @@ RunnerTestDescriptor getRunnerTestDescriptor() {
7073
return runnerTestDescriptor;
7174
}
7275

76+
Collection<TestDescriptor> getInProgressTestDescriptorsWithSyntheticStartEvents() {
77+
List<TestDescriptor> result = startedDescriptors.entrySet().stream() //
78+
.filter(entry -> entry.getValue().equals(EventType.SYNTHETIC)) //
79+
.map(Entry::getKey) //
80+
.filter(descriptor -> !isFinished(descriptor)) //
81+
.collect(toCollection(ArrayList::new));
82+
Collections.reverse(result);
83+
return result;
84+
}
85+
7386
boolean isDescendantOfRunnerTestDescriptor(TestDescriptor testDescriptor) {
7487
return runnerDescendants.contains(testDescriptor);
7588
}
@@ -113,12 +126,12 @@ boolean isSkipped(TestDescriptor testDescriptor) {
113126
return skippedDescriptors.contains(testDescriptor);
114127
}
115128

116-
void markStarted(TestDescriptor testDescriptor) {
117-
startedDescriptors.add(testDescriptor);
129+
void markStarted(TestDescriptor testDescriptor, EventType eventType) {
130+
startedDescriptors.put(testDescriptor, eventType);
118131
}
119132

120133
boolean isNotStarted(TestDescriptor testDescriptor) {
121-
return !startedDescriptors.contains(testDescriptor);
134+
return !startedDescriptors.containsKey(testDescriptor);
122135
}
123136

124137
void markFinished(TestDescriptor testDescriptor) {

‎junit-vintage-engine/src/test/java/org/junit/vintage/engine/VintageTestEngineExecutionTests.java‎

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,13 @@
4646
import org.junit.runner.RunWith;
4747
import org.junit.runner.Runner;
4848
import org.junit.runner.notification.RunNotifier;
49+
import org.junit.runners.Suite;
50+
import org.junit.runners.Suite.SuiteClasses;
4951
import org.junit.vintage.engine.samples.junit3.PlainJUnit3TestCaseWithSingleTestWhichFails;
5052
import org.junit.vintage.engine.samples.junit4.EmptyIgnoredTestCase;
5153
import org.junit.vintage.engine.samples.junit4.EnclosedJUnit4TestCase;
5254
import org.junit.vintage.engine.samples.junit4.IgnoredJUnit4TestCase;
55+
import org.junit.vintage.engine.samples.junit4.IgnoredParameterizedTestCase;
5356
import org.junit.vintage.engine.samples.junit4.JUnit4SuiteOfSuiteWithIgnoredJUnit4TestCase;
5457
import org.junit.vintage.engine.samples.junit4.JUnit4SuiteOfSuiteWithJUnit4TestCaseWithAssumptionFailureInBeforeClass;
5558
import org.junit.vintage.engine.samples.junit4.JUnit4SuiteOfSuiteWithJUnit4TestCaseWithErrorInBeforeClass;
@@ -420,6 +423,23 @@ void executesParameterizedTestCase() {
420423
event(engine(), finishedSuccessfully()));
421424
}
422425

426+
@Test
427+
void executesIgnoredParameterizedTestCase() {
428+
Class<?> testClass = IgnoredParameterizedTestCase.class;
429+
430+
execute(testClass).assertEventsMatchExactly( //
431+
event(engine(), started()), //
432+
event(container(testClass), started()), //
433+
event(container("[foo]"), started()), //
434+
event(test("test[foo]"), skippedWithReason("")), //
435+
event(container("[foo]"), finishedSuccessfully()), //
436+
event(container("[bar]"), started()), //
437+
event(test("test[bar]"), skippedWithReason("")), //
438+
event(container("[bar]"), finishedSuccessfully()), //
439+
event(container(testClass), finishedSuccessfully()), //
440+
event(engine(), finishedSuccessfully()));
441+
}
442+
423443
@Test
424444
void executesJUnit4TestCaseWithExceptionThrowingRunner() {
425445
Class<?> testClass = JUnit4TestCaseWithExceptionThrowingRunner.class;
@@ -466,7 +486,6 @@ public void run(RunNotifier notifier) {
466486

467487
@RunWith(DynamicSuiteRunner.class)
468488
public static class DynamicTestClass {
469-
470489
}
471490

472491
@Test
@@ -483,6 +502,61 @@ void reportsDynamicTestsForUnknownDescriptions() {
483502
event(engine(), finishedSuccessfully()));
484503
}
485504

505+
public static class DynamicAndStaticChildrenRunner extends Runner {
506+
507+
private final Class<?> testClass;
508+
509+
public DynamicAndStaticChildrenRunner(Class<?> testClass) {
510+
this.testClass = testClass;
511+
}
512+
513+
@Override
514+
public Description getDescription() {
515+
Description suiteDescription = createSuiteDescription(testClass);
516+
suiteDescription.addChild(createTestDescription(testClass, "staticTest"));
517+
return suiteDescription;
518+
}
519+
520+
@Override
521+
public void run(RunNotifier notifier) {
522+
Description staticDescription = getDescription().getChildren().get(0);
523+
notifier.fireTestStarted(staticDescription);
524+
notifier.fireTestFinished(staticDescription);
525+
Description dynamicDescription = createTestDescription(testClass, "dynamicTest");
526+
notifier.fireTestStarted(dynamicDescription);
527+
notifier.fireTestFinished(dynamicDescription);
528+
}
529+
530+
}
531+
532+
@RunWith(DynamicAndStaticChildrenRunner.class)
533+
public static class DynamicAndStaticTestClass {
534+
}
535+
536+
@RunWith(Suite.class)
537+
@SuiteClasses(DynamicAndStaticTestClass.class)
538+
public static class SuiteWithDynamicAndStaticTestClass {
539+
}
540+
541+
@Test
542+
void reportsIntermediateContainersFinishedAfterTheirDynamicChildren() {
543+
Class<?> suiteClass = SuiteWithDynamicAndStaticTestClass.class;
544+
Class<?> testClass = DynamicAndStaticTestClass.class;
545+
546+
execute(suiteClass).assertEventsMatchExactly( //
547+
event(engine(), started()), //
548+
event(container(suiteClass.getName()), started()), //
549+
event(container(testClass.getName()), started()), //
550+
event(test("staticTest"), started()), //
551+
event(test("staticTest"), finishedSuccessfully()), //
552+
event(dynamicTestRegistered("dynamicTest")), //
553+
event(test("dynamicTest"), started()), //
554+
event(test("dynamicTest"), finishedSuccessfully()), //
555+
event(container(testClass.getName()), finishedSuccessfully()), //
556+
event(container(suiteClass.getName()), finishedSuccessfully()), //
557+
event(engine(), finishedSuccessfully()));
558+
}
559+
486560
public static class MisbehavingChildlessRunner extends Runner {
487561

488562
private final Class<?> testClass;

‎junit-vintage-engine/src/test/java/org/junit/vintage/engine/samples/junit4/IgnoredParameterizedTestCase.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright 2015-2019 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+
* http://www.eclipse.org/legal/epl-v20.html
9+
*/
10+
11+
package org.junit.vintage.engine.samples.junit4;
12+
13+
import static java.util.Arrays.asList;
14+
15+
import org.junit.Ignore;
16+
import org.junit.Test;
17+
import org.junit.runner.RunWith;
18+
import org.junit.runners.Parameterized;
19+
import org.junit.runners.Parameterized.Parameter;
20+
import org.junit.runners.Parameterized.Parameters;
21+
22+
/**
23+
* @since 5.4.1
24+
*/
25+
@RunWith(Parameterized.class)
26+
public class IgnoredParameterizedTestCase {
27+
28+
@Parameters(name = "{0}")
29+
public static Iterable<String> parameters() {
30+
return asList("foo", "bar");
31+
}
32+
33+
@Parameter
34+
public String value;
35+
36+
@Test
37+
@Ignore
38+
public void test() {
39+
// never called
40+
}
41+
42+
}

0 commit comments

Comments
 (0)