Skip to content

Commit 8b0b358

Browse files
krmahadevankrmahadevan
committed
Streamline listener invocation handling exceptions
Closes #3238
1 parent 4c92177 commit 8b0b358

28 files changed

Lines changed: 588 additions & 71 deletions

‎.gitignore‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,5 @@ z_build
2727
outputDir/
2828
**/Version.java
2929
**/bin/
30+
.kotlin/
31+
.sdkmanrc

‎CHANGES.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
Current (7.13.0)
2+
Fixed: GITHUB-3238: Tests never finish if helper throws exception while executing parallel tests (Krishnan Mahadevan)
23
Fixed: GITHUB-3120: ITestNGListenerFactory is broken and never invoked (Krishnan Mahadevan)
34

45
7.12.0

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.testng.reporters.JUnitXMLReporter;
2424
import org.testng.reporters.TestHTMLReporter;
2525
import org.testng.reporters.TextReporter;
26+
import org.testng.util.Invocable;
2627
import org.testng.xml.XmlSuite;
2728
import org.testng.xml.XmlTest;
2829

@@ -239,14 +240,14 @@ private void invokeListeners(boolean start) {
239240
for (ISuiteListener sl :
240241
ListenerOrderDeterminer.order(
241242
listeners.values(), this.configuration.getListenerComparator())) {
242-
sl.onStart(this);
243+
Invocable.runQuietly(() -> sl.onStart(this));
243244
}
244245
} else {
245246
List<ISuiteListener> suiteListenersReversed =
246247
ListenerOrderDeterminer.reversedOrder(
247248
listeners.values(), this.configuration.getListenerComparator());
248249
for (ISuiteListener sl : suiteListenersReversed) {
249-
sl.onFinish(this);
250+
Invocable.runQuietly(() -> sl.onFinish(this));
250251
}
251252
}
252253
}

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import org.testng.reporters.XMLReporter;
6161
import org.testng.reporters.jq.Main;
6262
import org.testng.thread.IThreadWorkerFactory;
63+
import org.testng.util.Invocable;
6364
import org.testng.util.Strings;
6465
import org.testng.xml.IPostProcessor;
6566
import org.testng.xml.XmlClass;
@@ -1116,7 +1117,7 @@ private void runSuiteAlterationListeners() {
11161117
Collection<IAlterSuiteListener> original =
11171118
sort(m_alterSuiteListeners.values(), m_configuration.getListenerComparator());
11181119
for (IAlterSuiteListener l : original) {
1119-
l.alter(m_suites);
1120+
Invocable.runQuietly(() -> l.alter(m_suites));
11201121
}
11211122
}
11221123

@@ -1126,7 +1127,7 @@ private void runExecutionListeners(boolean start) {
11261127
m_configuration.getExecutionListeners(), m_configuration.getListenerComparator());
11271128
if (start) {
11281129
for (IExecutionListener l : executionListeners) {
1129-
l.onExecutionStart();
1130+
Invocable.runQuietly(l::onExecutionStart);
11301131
}
11311132
// Invoke our exit code listener after all the user's listeners have run.
11321133
exitCodeListener.onExecutionStart();
@@ -1135,7 +1136,7 @@ private void runExecutionListeners(boolean start) {
11351136
ListenerOrderDeterminer.reversedOrder(
11361137
m_configuration.getExecutionListeners(), m_configuration.getListenerComparator());
11371138
for (IExecutionListener l : executionListenersReversed) {
1138-
l.onExecutionFinish();
1139+
Invocable.runQuietly(l::onExecutionFinish);
11391140
}
11401141
// Invoke our exit code listener after all the user's listeners have run.
11411142
exitCodeListener.onExecutionFinish();
@@ -1162,7 +1163,7 @@ private void generateReports(List<ISuite> suiteRunners) {
11621163
"TestNG",
11631164
2,
11641165
"Time taken by " + reporter + ": " + (System.currentTimeMillis() - start) + " ms");
1165-
} catch (Exception ex) {
1166+
} catch (Throwable ex) {
11661167
System.err.println("[TestNG] Reporter " + reporter + " failed");
11671168
ex.printStackTrace(System.err);
11681169
}

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import org.testng.internal.objects.IObjectDispenser;
6060
import org.testng.thread.IThreadWorkerFactory;
6161
import org.testng.thread.IWorker;
62+
import org.testng.util.Invocable;
6263
import org.testng.util.Strings;
6364
import org.testng.util.TimeUtils;
6465
import org.testng.xml.XmlClass;
@@ -756,7 +757,12 @@ private ITestNGMethod[] intercept(ITestNGMethod[] methods) {
756757
List<IMethodInterceptor> original =
757758
sort(m_methodInterceptors, m_configuration.getListenerComparator());
758759
for (IMethodInterceptor m_methodInterceptor : original) {
759-
methodInstances = m_methodInterceptor.intercept(methodInstances, this);
760+
List<IMethodInstance> temp = methodInstances;
761+
List<IMethodInstance> result =
762+
Invocable.callQuietly(() -> m_methodInterceptor.intercept(temp, this));
763+
if (result != null) {
764+
methodInstances = result;
765+
}
760766
}
761767

762768
List<ITestNGMethod> result = MethodHelper.methodInstancesToMethods(methodInstances);
@@ -891,7 +897,7 @@ private void fireEvent(boolean isStart) {
891897
if (isStart) {
892898
for (ITestListener itl :
893899
ListenerOrderDeterminer.order(m_testListeners, m_configuration.getListenerComparator())) {
894-
itl.onStart(this);
900+
Invocable.runQuietly(() -> itl.onStart(this));
895901
}
896902
this.exitCodeListener.onStart(this);
897903

@@ -900,7 +906,7 @@ private void fireEvent(boolean isStart) {
900906
ListenerOrderDeterminer.reversedOrder(
901907
m_testListeners, m_configuration.getListenerComparator());
902908
for (ITestListener itl : testListenersReversed) {
903-
itl.onFinish(this);
909+
Invocable.runQuietly(() -> itl.onFinish(this));
904910
}
905911
this.exitCodeListener.onFinish(this);
906912
}

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.testng.collections.Lists;
1313
import org.testng.collections.Maps;
1414
import org.testng.collections.Sets;
15+
import org.testng.util.Invocable;
1516

1617
/** Representation of the graph of methods. */
1718
public class DynamicGraph<T> implements IDynamicGraph<T> {
@@ -48,7 +49,7 @@ public void addEdges(int weight, T from, Iterable<T> tos) {
4849
}
4950
}
5051

51-
/** @return a set of all the nodes that don't depend on any other nodes. */
52+
/** @return a list of all the nodes that don't depend on any other nodes. */
5253
public List<T> getFreeNodes() {
5354
// Get a list of nodes that are ready and have no outgoing edges.
5455
Set<T> free = Sets.newLinkedHashSet(m_nodesReady);
@@ -141,8 +142,9 @@ public void setStatus(T node, Status status) {
141142
default:
142143
throw new IllegalArgumentException("Unsupported status: " + status);
143144
}
144-
145-
this.visualisers.forEach(visualiser -> visualiser.consumeDotDefinition(toDot()));
145+
for (IExecutionVisualiser visualiser : visualisers) {
146+
Invocable.runQuietly(() -> visualiser.consumeDotDefinition(toDot()));
147+
}
146148
}
147149

148150
/** @return the number of nodes in this graph. */

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.testng.internal.reflect.MethodMatcher;
3737
import org.testng.internal.reflect.MethodMatcherContext;
3838
import org.testng.internal.reflect.ReflectionRecipes;
39+
import org.testng.util.Invocable;
3940
import org.testng.util.Strings;
4041
import org.testng.xml.XmlSuite;
4142
import org.testng.xml.XmlTest;
@@ -800,8 +801,10 @@ public static ParameterHolder handleParameters(
800801
do {
801802

802803
for (IDataProviderListener dataProviderListener : holder.getListeners()) {
803-
dataProviderListener.beforeDataProviderExecution(
804-
dataProviderMethod, testMethod, methodParams.context);
804+
Invocable.runQuietly(
805+
() ->
806+
dataProviderListener.beforeDataProviderExecution(
807+
dataProviderMethod, testMethod, methodParams.context));
805808
}
806809

807810
try {
@@ -818,7 +821,8 @@ public static ParameterHolder handleParameters(
818821
thrownException = null;
819822
} catch (RuntimeException e) {
820823
for (IDataProviderListener each : holder.getListeners()) {
821-
each.onDataProviderFailure(testMethod, methodParams.context, e);
824+
Invocable.runQuietly(
825+
() -> each.onDataProviderFailure(testMethod, methodParams.context, e));
822826
}
823827
if (shouldRetry) {
824828
shouldRetry = retry.retry(dataProviderMethod);
@@ -841,8 +845,10 @@ public static ParameterHolder handleParameters(
841845
}
842846

843847
for (IDataProviderListener dataProviderListener : holder.getListeners()) {
844-
dataProviderListener.afterDataProviderExecution(
845-
dataProviderMethod, testMethod, methodParams.context);
848+
Invocable.runQuietly(
849+
() ->
850+
dataProviderListener.afterDataProviderExecution(
851+
dataProviderMethod, testMethod, methodParams.context));
846852
}
847853

848854
// If the data provider is restricting the indices to return, filter them out
@@ -855,9 +861,12 @@ public static ParameterHolder handleParameters(
855861

856862
testMethod.setMoreInvocationChecker(filteredParameters::hasNext);
857863
for (IDataProviderInterceptor interceptor : holder.getInterceptors()) {
864+
Iterator<Object[]> temp = filteredParameters;
858865
filteredParameters =
859-
interceptor.intercept(
860-
filteredParameters, dataProviderMethod, testMethod, methodParams.context);
866+
Invocable.call(
867+
() ->
868+
interceptor.intercept(
869+
temp, dataProviderMethod, testMethod, methodParams.context));
861870
}
862871

863872
if (dataProviderMethod instanceof DataProviderMethodRemovable) {

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

Lines changed: 15 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.testng.TestNGException;
1818
import org.testng.annotations.IListenersAnnotation;
1919
import org.testng.internal.annotations.IAnnotationFinder;
20+
import org.testng.util.Invocable;
2021

2122
/** A helper class that internally houses some of the listener related actions support. */
2223
public final class TestListenerHelper {
@@ -34,12 +35,8 @@ public static void runPreConfigurationListeners(
3435
List<IConfigurationListener> original = ListenerOrderDeterminer.order(listeners, comparator);
3536

3637
for (IConfigurationListener icl : original) {
37-
icl.beforeConfiguration(tr);
38-
try {
39-
icl.beforeConfiguration(tr, tm);
40-
} catch (Exception e) {
41-
ignoreInternalGradleException(e);
42-
}
38+
Invocable.runQuietly(() -> icl.beforeConfiguration(tr));
39+
Invocable.runQuietly(() -> icl.beforeConfiguration(tr, tm));
4340
}
4441
}
4542

@@ -55,44 +52,23 @@ public static void runPostConfigurationListeners(
5552
for (IConfigurationListener icl : listenersreversed) {
5653
switch (tr.getStatus()) {
5754
case ITestResult.SKIP:
58-
icl.onConfigurationSkip(tr);
59-
try {
60-
icl.onConfigurationSkip(tr, tm);
61-
} catch (Exception e) {
62-
ignoreInternalGradleException(e);
63-
}
55+
Invocable.runQuietly(() -> icl.onConfigurationSkip(tr));
56+
Invocable.runQuietly(() -> icl.onConfigurationSkip(tr, tm));
6457
break;
6558
case ITestResult.FAILURE:
66-
icl.onConfigurationFailure(tr);
67-
try {
68-
icl.onConfigurationFailure(tr, tm);
69-
} catch (Exception e) {
70-
ignoreInternalGradleException(e);
71-
}
59+
Invocable.runQuietly(() -> icl.onConfigurationFailure(tr));
60+
Invocable.runQuietly(() -> icl.onConfigurationFailure(tr, tm));
7261
break;
7362
case ITestResult.SUCCESS:
74-
icl.onConfigurationSuccess(tr);
75-
try {
76-
icl.onConfigurationSuccess(tr, tm);
77-
} catch (Exception e) {
78-
ignoreInternalGradleException(e);
79-
}
63+
Invocable.runQuietly(() -> icl.onConfigurationSuccess(tr));
64+
Invocable.runQuietly(() -> icl.onConfigurationSuccess(tr, tm));
8065
break;
8166
default:
8267
throw new AssertionError("Unexpected value: " + tr.getStatus());
8368
}
8469
}
8570
}
8671

87-
// This method is added because Gradle which builds TestNG seems to be using an older version
88-
// of TestNG that doesn't know about the new methods that we added and so it causes
89-
// the TestNG build to keep failing.
90-
private static void ignoreInternalGradleException(Exception e) {
91-
if (!e.getClass().getPackage().getName().startsWith("org.gradle.internal")) {
92-
throw new ListenerInvocationException(e);
93-
}
94-
}
95-
9672
/**
9773
* Iterates through a bunch of listeners and invokes them.
9874
*
@@ -103,23 +79,23 @@ public static void runTestListeners(ITestResult tr, List<ITestListener> listener
10379
for (ITestListener itl : listeners) {
10480
switch (tr.getStatus()) {
10581
case ITestResult.SKIP:
106-
itl.onTestSkipped(tr);
82+
Invocable.runQuietly(() -> itl.onTestSkipped(tr));
10783
break;
10884
case ITestResult.SUCCESS_PERCENTAGE_FAILURE:
109-
itl.onTestFailedButWithinSuccessPercentage(tr);
85+
Invocable.runQuietly(() -> itl.onTestFailedButWithinSuccessPercentage(tr));
11086
break;
11187
case ITestResult.FAILURE:
11288
if (ITestResult.wasFailureDueToTimeout(tr)) {
113-
itl.onTestFailedWithTimeout(tr);
89+
Invocable.runQuietly(() -> itl.onTestFailedWithTimeout(tr));
11490
} else {
115-
itl.onTestFailure(tr);
91+
Invocable.runQuietly(() -> itl.onTestFailure(tr));
11692
}
11793
break;
11894
case ITestResult.SUCCESS:
119-
itl.onTestSuccess(tr);
95+
Invocable.runQuietly(() -> itl.onTestSuccess(tr));
12096
break;
12197
case ITestResult.STARTED:
122-
itl.onTestStart(tr);
98+
Invocable.runQuietly(() -> itl.onTestStart(tr));
12399
break;
124100
default:
125101
throw new AssertionError("Unknown status: " + tr.getStatus());

‎testng-core/src/main/java/org/testng/internal/annotations/JDK15AnnotationFinder.java‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.testng.annotations.TestInstance;
4242
import org.testng.internal.ConstructorOrMethod;
4343
import org.testng.internal.collections.Pair;
44+
import org.testng.util.Invocable;
4445

4546
/** This class implements IAnnotationFinder with JDK5 annotations */
4647
public class JDK15AnnotationFinder implements IAnnotationFinder {
@@ -169,20 +170,26 @@ private void transform(
169170
if (m_transformer instanceof org.testng.internal.annotations.IAnnotationTransformer) {
170171
org.testng.internal.annotations.IAnnotationTransformer transformer =
171172
(org.testng.internal.annotations.IAnnotationTransformer) m_transformer;
172-
transformer.transform(
173-
(ITestAnnotation) a, testClass, testConstructor, testMethod, whichClass);
173+
Invocable.runQuietly(
174+
() ->
175+
transformer.transform(
176+
(ITestAnnotation) a, testClass, testConstructor, testMethod, whichClass));
174177
} else {
175-
m_transformer.transform((ITestAnnotation) a, testClass, testConstructor, testMethod);
178+
Invocable.runQuietly(
179+
() ->
180+
m_transformer.transform(
181+
(ITestAnnotation) a, testClass, testConstructor, testMethod));
176182
}
177183
} else if (a instanceof IConfigurationAnnotation) {
178184
IConfigurationAnnotation configuration = (IConfigurationAnnotation) a;
179-
m_transformer.transform(configuration, testClass, testConstructor, testMethod);
185+
Invocable.runQuietly(
186+
() -> m_transformer.transform(configuration, testClass, testConstructor, testMethod));
180187
} else if (a instanceof IDataProviderAnnotation) {
181-
m_transformer.transform((IDataProviderAnnotation) a, testMethod);
188+
Invocable.runQuietly(() -> m_transformer.transform((IDataProviderAnnotation) a, testMethod));
182189
} else if (a instanceof IFactoryAnnotation) {
183-
m_transformer.transform((IFactoryAnnotation) a, testMethod);
190+
Invocable.runQuietly(() -> m_transformer.transform((IFactoryAnnotation) a, testMethod));
184191
} else if (a instanceof IListenersAnnotation) {
185-
m_transformer.transform((IListenersAnnotation) a, testClass);
192+
Invocable.runQuietly(() -> m_transformer.transform((IListenersAnnotation) a, testClass));
186193
}
187194
}
188195

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import org.testng.IInvokedMethodListener;
88
import org.testng.ITestContext;
99
import org.testng.ITestResult;
10+
import org.testng.util.Invocable;
1011

1112
/**
1213
* Hides complexity of calling methods of {@link IInvokedMethodListener}.
@@ -52,12 +53,14 @@ public InvokedMethodListenerInvoker(
5253
public void invokeListener(
5354
IInvokedMethodListener listenerInstance, IInvokedMethod invokedMethod) {
5455
if (this.m_listenerMethod == BEFORE_INVOCATION) {
55-
listenerInstance.beforeInvocation(invokedMethod, m_testResult);
56-
listenerInstance.beforeInvocation(invokedMethod, m_testResult, m_testContext);
56+
Invocable.runQuietly(() -> listenerInstance.beforeInvocation(invokedMethod, m_testResult));
57+
Invocable.runQuietly(
58+
() -> listenerInstance.beforeInvocation(invokedMethod, m_testResult, m_testContext));
5759
}
5860
if (this.m_listenerMethod == AFTER_INVOCATION) {
59-
listenerInstance.afterInvocation(invokedMethod, m_testResult);
60-
listenerInstance.afterInvocation(invokedMethod, m_testResult, m_testContext);
61+
Invocable.runQuietly(() -> listenerInstance.afterInvocation(invokedMethod, m_testResult));
62+
Invocable.runQuietly(
63+
() -> listenerInstance.afterInvocation(invokedMethod, m_testResult, m_testContext));
6164
}
6265
}
6366
}

0 commit comments

Comments
 (0)