Skip to content

Commit 755cfd4

Browse files
krmahadevankrmahadevan
committed
Segregate native & user listeners before ordering
Closes #2771
1 parent 8634720 commit 755cfd4

File tree

13 files changed

+156
-15
lines changed

13 files changed

+156
-15
lines changed

‎CHANGES.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
Current
22
Fixed: GITHUB-2796: Option for onAfterClass to run after @AfterClass
3+
Fixed: GITHUB-2771: After upgrading to TestNG 7.5.0, setting ITestResult.status to FAILURE doesn't fail the test anymore (Krishnan Mahadevan)
34
Fixed: GITHUB-2857: XmlTest index is not set for test suites invoked with YAML
45

56
7.7.1

‎testng-core-api/src/main/java/org/testng/internal/RuntimeBehavior.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,14 @@ public final class RuntimeBehavior {
1616
public static final String TESTNG_DEFAULT_VERBOSE = "testng.default.verbose";
1717
public static final String IGNORE_CALLBACK_INVOCATION_SKIPS = "testng.ignore.callback.skip";
1818
public static final String SYMMETRIC_LISTENER_EXECUTION = "testng.listener.execution.symmetric";
19+
public static final String INTELLIJ_PACKAGE_NAME = "testng.intellij.package.name";
1920

2021
private RuntimeBehavior() {}
2122

23+
public static String getIntellijPackageName() {
24+
return System.getProperty(INTELLIJ_PACKAGE_NAME, "com.intellij.rt.testng");
25+
}
26+
2227
public static boolean ignoreCallbackInvocationSkips() {
2328
return Boolean.getBoolean(IGNORE_CALLBACK_INVOCATION_SKIPS);
2429
}

‎testng-core/src/main/java/org/testng/SuiteRunner.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,11 +260,12 @@ public void setReportResults(boolean reportResults) {
260260

261261
private void invokeListeners(boolean start) {
262262
if (start) {
263-
for (ISuiteListener sl : Lists.newArrayList(listeners.values())) {
263+
for (ISuiteListener sl : ListenerOrderDeterminer.order(listeners.values())) {
264264
sl.onStart(this);
265265
}
266266
} else {
267-
List<ISuiteListener> suiteListenersReversed = Lists.newReversedArrayList(listeners.values());
267+
List<ISuiteListener> suiteListenersReversed =
268+
ListenerOrderDeterminer.reversedOrder(listeners.values());
268269
for (ISuiteListener sl : suiteListenersReversed) {
269270
sl.onFinish(this);
270271
}

‎testng-core/src/main/java/org/testng/TestNG.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.testng.internal.DynamicGraph;
3030
import org.testng.internal.ExitCode;
3131
import org.testng.internal.IConfiguration;
32+
import org.testng.internal.ListenerOrderDeterminer;
3233
import org.testng.internal.OverrideProcessor;
3334
import org.testng.internal.ReporterConfig;
3435
import org.testng.internal.RuntimeBehavior;
@@ -1106,14 +1107,15 @@ private void runSuiteAlterationListeners() {
11061107
}
11071108

11081109
private void runExecutionListeners(boolean start) {
1109-
List<IExecutionListener> executionListeners = m_configuration.getExecutionListeners();
1110+
List<IExecutionListener> executionListeners =
1111+
ListenerOrderDeterminer.order(m_configuration.getExecutionListeners());
11101112
if (start) {
11111113
for (IExecutionListener l : executionListeners) {
11121114
l.onExecutionStart();
11131115
}
11141116
} else {
11151117
List<IExecutionListener> executionListenersReversed =
1116-
Lists.newReversedArrayList(executionListeners);
1118+
ListenerOrderDeterminer.reversedOrder(executionListeners);
11171119
for (IExecutionListener l : executionListenersReversed) {
11181120
l.onExecutionFinish();
11191121
}

‎testng-core/src/main/java/org/testng/TestRunner.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.testng.internal.IContainer;
3737
import org.testng.internal.ITestClassConfigInfo;
3838
import org.testng.internal.ITestResultNotifier;
39+
import org.testng.internal.ListenerOrderDeterminer;
3940
import org.testng.internal.MethodGroupsHelper;
4041
import org.testng.internal.MethodHelper;
4142
import org.testng.internal.ResultMap;
@@ -955,12 +956,13 @@ private void logStart() {
955956
*/
956957
private void fireEvent(boolean isStart) {
957958
if (isStart) {
958-
for (ITestListener itl : m_testListeners) {
959+
for (ITestListener itl : ListenerOrderDeterminer.order(m_testListeners)) {
959960
itl.onStart(this);
960961
}
961962

962963
} else {
963-
List<ITestListener> testListenersReversed = Lists.newReversedArrayList(m_testListeners);
964+
List<ITestListener> testListenersReversed =
965+
ListenerOrderDeterminer.reversedOrder(m_testListeners);
964966
for (ITestListener itl : testListenersReversed) {
965967
itl.onFinish(this);
966968
}

‎testng-core/src/main/java/org/testng/internal/ListenerOrderDeterminer.java

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
package org.testng.internal;
2+
3+
import java.util.ArrayList;
4+
import java.util.Arrays;
5+
import java.util.Collection;
6+
import java.util.Collections;
7+
import java.util.List;
8+
import java.util.Objects;
9+
import java.util.function.Predicate;
10+
import org.testng.collections.Lists;
11+
12+
/**
13+
* A Utility that helps us differentiate between a user's listener and TestNG's native listeners(
14+
* which also includes IntelliJ IDEA Listener)
15+
*/
16+
public final class ListenerOrderDeterminer {
17+
18+
private ListenerOrderDeterminer() {
19+
// Defeat instantiation
20+
}
21+
22+
private static final List<String> KNOWN =
23+
Arrays.asList(
24+
"org.testng.internal.ExitCodeListener",
25+
"org.testng.SuiteRunner",
26+
"org.testng.TestRunner.ConfigurationListener",
27+
"org.testng.reporters.DotTestListener",
28+
"org.testng.reporters.EmailableReporter",
29+
"org.testng.reporters.EmailableReporter2",
30+
"org.testng.reporters.ExitCodeListener",
31+
"org.testng.reporters.FailedReporter",
32+
"org.testng.reporters.JUnitReportReporter",
33+
"org.testng.reporters.JUnitXMLReporter",
34+
"org.testng.reporters.SuiteHTMLReporter",
35+
"org.testng.reporters.TestHTMLReporter",
36+
"org.testng.reporters.TextReporter",
37+
"org.testng.reporters.VerboseReporter",
38+
"org.testng.reporters.XMLReporter",
39+
"org.testng.reporters.jq.Main");
40+
41+
private static final Predicate<Class<?>> INTELLIJ_IDE_LISTENER =
42+
clazz -> clazz.getName().contains(RuntimeBehavior.getIntellijPackageName());
43+
44+
/**
45+
* @param original - The original collection of listeners
46+
* @return - A re-ordered collection wherein TestNG's known listeners are added at the end
47+
*/
48+
public static <T> List<T> order(Collection<T> original) {
49+
List<T> nativeListeners = new ArrayList<>();
50+
List<T> foreignListeners = new ArrayList<>();
51+
original.stream()
52+
.filter(Objects::nonNull)
53+
.forEach(
54+
each -> {
55+
if (isNativeListener(each.getClass())) {
56+
nativeListeners.add(each);
57+
} else {
58+
foreignListeners.add(each);
59+
}
60+
});
61+
return Lists.merge(foreignListeners, nativeListeners);
62+
}
63+
64+
/**
65+
* @param original - The original collection of listeners
66+
* @return - A reversed ordered list wherein the user listeners are found in reverse order
67+
* followed by TestNG known listeners also in reverse order.
68+
*/
69+
public static <T> List<T> reversedOrder(Collection<T> original) {
70+
List<T> nativeListeners = new ArrayList<>();
71+
List<T> foreignListeners = new ArrayList<>();
72+
original.stream()
73+
.filter(Objects::nonNull)
74+
.forEach(
75+
each -> {
76+
if (isNativeListener(each.getClass())) {
77+
nativeListeners.add(each);
78+
} else {
79+
foreignListeners.add(each);
80+
}
81+
});
82+
Collections.reverse(foreignListeners);
83+
Collections.reverse(nativeListeners);
84+
return Lists.merge(foreignListeners, nativeListeners);
85+
}
86+
87+
private static boolean isNativeListener(Class<?> clazz) {
88+
return KNOWN.stream()
89+
.anyMatch(
90+
each ->
91+
each.equalsIgnoreCase(clazz.getCanonicalName())
92+
|| INTELLIJ_IDE_LISTENER.test(clazz));
93+
}
94+
}

‎testng-core/src/main/java/org/testng/internal/TestListenerHelper.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ private TestListenerHelper() {
2323

2424
public static void runPreConfigurationListeners(
2525
ITestResult tr, ITestNGMethod tm, List<IConfigurationListener> listeners) {
26-
for (IConfigurationListener icl : listeners) {
26+
for (IConfigurationListener icl : ListenerOrderDeterminer.order(listeners)) {
2727
icl.beforeConfiguration(tr);
2828
try {
2929
icl.beforeConfiguration(tr, tm);
@@ -35,7 +35,8 @@ public static void runPreConfigurationListeners(
3535

3636
public static void runPostConfigurationListeners(
3737
ITestResult tr, ITestNGMethod tm, List<IConfigurationListener> listeners) {
38-
List<IConfigurationListener> listenersreversed = Lists.newReversedArrayList(listeners);
38+
List<IConfigurationListener> listenersreversed =
39+
ListenerOrderDeterminer.reversedOrder(listeners);
3940
for (IConfigurationListener icl : listenersreversed) {
4041
switch (tr.getStatus()) {
4142
case ITestResult.SKIP:

‎testng-core/src/main/java/org/testng/internal/invokers/BaseInvoker.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@
1010
import org.testng.ITestResult;
1111
import org.testng.SkipException;
1212
import org.testng.SuiteRunState;
13-
import org.testng.collections.Lists;
1413
import org.testng.collections.Maps;
1514
import org.testng.internal.IConfiguration;
1615
import org.testng.internal.ITestResultNotifier;
16+
import org.testng.internal.ListenerOrderDeterminer;
1717
import org.testng.internal.Utils;
1818
import org.testng.internal.annotations.IAnnotationFinder;
1919

@@ -60,8 +60,8 @@ protected void runInvokedMethodListeners(
6060
boolean isAfterInvocation = InvokedMethodListenerMethod.AFTER_INVOCATION == listenerMethod;
6161
Collection<IInvokedMethodListener> listeners =
6262
isAfterInvocation
63-
? Lists.newReversedArrayList(m_invokedMethodListeners)
64-
: m_invokedMethodListeners;
63+
? ListenerOrderDeterminer.reversedOrder(m_invokedMethodListeners)
64+
: ListenerOrderDeterminer.order(m_invokedMethodListeners);
6565
for (IInvokedMethodListener currentListener : listeners) {
6666
try {
6767
invoker.invokeListener(currentListener, invokedMethod);

‎testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,8 @@ public void runTestResultListener(ITestResult tr) {
272272
boolean isFinished = tr.getStatus() != ITestResult.STARTED;
273273
List<ITestListener> listeners =
274274
isFinished
275-
? Lists.newReversedArrayList(m_notifier.getTestListeners())
276-
: m_notifier.getTestListeners();
275+
? ListenerOrderDeterminer.reversedOrder(m_notifier.getTestListeners())
276+
: ListenerOrderDeterminer.order(m_notifier.getTestListeners());
277277
TestListenerHelper.runTestListeners(tr, listeners);
278278
}
279279

‎testng-core/src/main/java/org/testng/junit/JUnitTestRunner.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.testng.*;
1818
import org.testng.collections.Lists;
1919
import org.testng.internal.ITestResultNotifier;
20+
import org.testng.internal.ListenerOrderDeterminer;
2021
import org.testng.internal.TestListenerHelper;
2122
import org.testng.internal.invokers.InvokedMethod;
2223

@@ -94,8 +95,8 @@ public void endTest(Test test) {
9495
boolean isFinished = tr.getStatus() != ITestResult.STARTED;
9596
List<ITestListener> listeners =
9697
isFinished
97-
? Lists.newReversedArrayList(m_parentRunner.getTestListeners())
98-
: m_parentRunner.getTestListeners();
98+
? ListenerOrderDeterminer.reversedOrder(m_parentRunner.getTestListeners())
99+
: ListenerOrderDeterminer.order(m_parentRunner.getTestListeners());
99100
TestListenerHelper.runTestListeners(tr, listeners);
100101
}
101102

‎testng-core/src/test/java/test/listeners/ListenersTest.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.testng.TestNG;
1515
import org.testng.annotations.DataProvider;
1616
import org.testng.annotations.Test;
17+
import org.testng.internal.ExitCode;
1718
import org.testng.xml.XmlSuite;
1819
import test.SimpleBaseTest;
1920
import test.listeners.issue2638.DummyInvokedMethodListener;
@@ -23,6 +24,7 @@
2324
import test.listeners.issue2685.SampleTestFailureListener;
2425
import test.listeners.issue2752.ListenerSample;
2526
import test.listeners.issue2752.TestClassSample;
27+
import test.listeners.issue2771.TestCaseSample;
2628

2729
public class ListenersTest extends SimpleBaseTest {
2830

@@ -92,6 +94,13 @@ public void testWiringInOfListenersInMultipleTestTagsWithListenerInSuite() {
9294
assertThat(logs.get("Xml_Test_2")).containsAll(expected);
9395
}
9496

97+
@Test(description = "GITHUB-2771")
98+
public void testEnsureNativeListenersAreRunAlwaysAtEnd() {
99+
TestNG testng = create(TestCaseSample.class);
100+
testng.run();
101+
assertThat(testng.getStatus()).isEqualTo(ExitCode.FAILED);
102+
}
103+
95104
private void setupTest(boolean addExplicitListener) {
96105
TestNG testng = new TestNG();
97106
XmlSuite xmlSuite = createXmlSuite("Xml_Suite");

‎testng-core/src/test/java/test/listeners/issue2771/CustomSoftAssert.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package test.listeners.issue2771;
2+
3+
import org.testng.ITestResult;
4+
import org.testng.reporters.ExitCodeListener;
5+
6+
public class CustomSoftAssert extends ExitCodeListener {
7+
@Override
8+
public void onTestSuccess(ITestResult result) {
9+
result.setStatus(ITestResult.FAILURE);
10+
result.setThrowable(new AssertionError("There have been some failed soft asserts"));
11+
}
12+
}

‎testng-core/src/test/java/test/listeners/issue2771/TestCaseSample.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package test.listeners.issue2771;
2+
3+
import org.testng.annotations.Listeners;
4+
import org.testng.annotations.Test;
5+
6+
@Listeners(CustomSoftAssert.class)
7+
public class TestCaseSample {
8+
@Test
9+
public void someCustomSoftAsserts() {
10+
// doing custom soft asserts
11+
// which will trigger test failure in listener
12+
}
13+
}

0 commit comments

Comments
 (0)