Skip to content

Commit 5cd2cb3

Browse files
sbrannensbrannen
committed
Make AbstractTestNGSpringContextTests thread-safe regarding tracked exceptions
Prior to this commit, AbstractTestNGSpringContextTests was not thread-safe with regard to tracked exceptions. To address that, AbstractTestNGSpringContextTests now tracks the test exception via a ThreadLocal. Closes gh-35528
1 parent e9fb5eb commit 5cd2cb3

File tree

7 files changed

+159
-28
lines changed

7 files changed

+159
-28
lines changed

‎spring-test/src/main/java/org/springframework/test/context/testng/AbstractTestNGSpringContextTests.java‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,7 @@ public abstract class AbstractTestNGSpringContextTests implements IHookable, App
7474

7575
private final TestContextManager testContextManager;
7676

77-
@Nullable
78-
private Throwable testException;
77+
private final ThreadLocal<Throwable> testException = new ThreadLocal<>();
7978

8079

8180
/**
@@ -141,31 +140,33 @@ protected void springTestContextBeforeTestMethod(Method testMethod) throws Excep
141140
public void run(IHookCallBack callBack, ITestResult testResult) {
142141
Method testMethod = testResult.getMethod().getConstructorOrMethod().getMethod();
143142
boolean beforeCallbacksExecuted = false;
143+
Throwable currentException = null;
144144

145145
try {
146146
this.testContextManager.beforeTestExecution(this, testMethod);
147147
beforeCallbacksExecuted = true;
148148
}
149149
catch (Throwable ex) {
150-
this.testException = ex;
150+
currentException = ex;
151151
}
152152

153153
if (beforeCallbacksExecuted) {
154154
callBack.runTestMethod(testResult);
155-
this.testException = getTestResultException(testResult);
155+
currentException = getTestResultException(testResult);
156156
}
157157

158158
try {
159-
this.testContextManager.afterTestExecution(this, testMethod, this.testException);
159+
this.testContextManager.afterTestExecution(this, testMethod, currentException);
160160
}
161161
catch (Throwable ex) {
162-
if (this.testException == null) {
163-
this.testException = ex;
162+
if (currentException == null) {
163+
currentException = ex;
164164
}
165165
}
166166

167-
if (this.testException != null) {
168-
throwAsUncheckedException(this.testException);
167+
if (currentException != null) {
168+
this.testException.set(currentException);
169+
throwAsUncheckedException(currentException);
169170
}
170171
}
171172

@@ -180,10 +181,10 @@ public void run(IHookCallBack callBack, ITestResult testResult) {
180181
@AfterMethod(alwaysRun = true)
181182
protected void springTestContextAfterTestMethod(Method testMethod) throws Exception {
182183
try {
183-
this.testContextManager.afterTestMethod(this, testMethod, this.testException);
184+
this.testContextManager.afterTestMethod(this, testMethod, this.testException.get());
184185
}
185186
finally {
186-
this.testException = null;
187+
this.testException.remove();
187188
}
188189
}
189190

‎spring-test/src/test/java/org/springframework/test/context/cache/ClassLevelDirtiesContextTestNGTests.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,9 @@ private void runTestClassAndAssertStats(Class<?> testClass, int expectedTestCoun
145145
testNG.setVerbose(0);
146146
testNG.run();
147147

148-
assertThat(listener.testFailureCount).as("Failures for test class [" + testClass + "].").isEqualTo(expectedTestFailureCount);
149-
assertThat(listener.testStartCount).as("Tests started for test class [" + testClass + "].").isEqualTo(expectedTestStartedCount);
150-
assertThat(listener.testSuccessCount).as("Successful tests for test class [" + testClass + "].").isEqualTo(expectedTestFinishedCount);
148+
assertThat(listener.testFailureCount.get()).as("Failures for test class [" + testClass + "].").isEqualTo(expectedTestFailureCount);
149+
assertThat(listener.testStartCount.get()).as("Tests started for test class [" + testClass + "].").isEqualTo(expectedTestStartedCount);
150+
assertThat(listener.testSuccessCount.get()).as("Successful tests for test class [" + testClass + "].").isEqualTo(expectedTestFinishedCount);
151151
}
152152

153153
private void assertBehaviorForCleanTestCase() {

‎spring-test/src/test/java/org/springframework/test/context/testng/FailingBeforeAndAfterMethodsTestNGTests.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,10 @@ void runTestAndAssertCounters(Class<?> clazz, int expectedTestStartCount,
6464

6565
String name = clazz.getSimpleName();
6666

67-
assertThat(listener.testStartCount).as("tests started for [" + name + "] ==> ").isEqualTo(expectedTestStartCount);
68-
assertThat(listener.testSuccessCount).as("successful tests for [" + name + "] ==> ").isEqualTo(expectedTestSuccessCount);
69-
assertThat(listener.testFailureCount).as("failed tests for [" + name + "] ==> ").isEqualTo(expectedFailureCount);
70-
assertThat(listener.failedConfigurationsCount).as("failed configurations for [" + name + "] ==> ").isEqualTo(expectedFailedConfigurationsCount);
67+
assertThat(listener.testStartCount.get()).as("tests started for [" + name + "] ==> ").isEqualTo(expectedTestStartCount);
68+
assertThat(listener.testSuccessCount.get()).as("successful tests for [" + name + "] ==> ").isEqualTo(expectedTestSuccessCount);
69+
assertThat(listener.testFailureCount.get()).as("failed tests for [" + name + "] ==> ").isEqualTo(expectedFailureCount);
70+
assertThat(listener.failedConfigurationsCount.get()).as("failed configurations for [" + name + "] ==> ").isEqualTo(expectedFailedConfigurationsCount);
7171
}
7272

7373
static List<Arguments> testData() {

‎spring-test/src/test/java/org/springframework/test/context/testng/TestNGConcurrencyTests.java‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
/*
2+
* Copyright 2002-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.test.context.testng;
18+
19+
import org.testng.TestNG;
20+
import org.testng.annotations.Test;
21+
import org.testng.xml.XmlSuite.ParallelMode;
22+
23+
import org.springframework.context.annotation.Configuration;
24+
import org.springframework.test.context.ContextConfiguration;
25+
26+
import static org.assertj.core.api.Assertions.assertThat;
27+
28+
/**
29+
* Integration tests for concurrent TestNG tests.
30+
*
31+
* @author Sam Brannen
32+
* @since 6.2.12
33+
* @see <a href="https://github.com/spring-projects/spring-framework/issues/35528">gh-35528</a>
34+
*/
35+
class TestNGConcurrencyTests {
36+
37+
@org.junit.jupiter.api.Test
38+
void runTestsInParallel() throws Exception {
39+
TrackingTestNGTestListener listener = new TrackingTestNGTestListener();
40+
41+
TestNG testNG = new TestNG();
42+
testNG.addListener(listener);
43+
testNG.setTestClasses(new Class<?>[] { ConcurrentTestCase.class });
44+
testNG.setParallel(ParallelMode.METHODS);
45+
testNG.setThreadCount(5);
46+
testNG.setVerbose(0);
47+
testNG.run();
48+
49+
assertThat(listener.testStartCount.get()).as("tests started").isEqualTo(10);
50+
assertThat(listener.testSuccessCount.get()).as("successful tests").isEqualTo(10);
51+
assertThat(listener.testFailureCount.get()).as("failed tests").isEqualTo(0);
52+
assertThat(listener.failedConfigurationsCount.get()).as("failed configurations").isEqualTo(0);
53+
assertThat(listener.throwables).isEmpty();
54+
}
55+
56+
57+
@ContextConfiguration
58+
static class ConcurrentTestCase extends AbstractTestNGSpringContextTests {
59+
60+
@Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "Message1")
61+
public void message1() {
62+
throw new RuntimeException("Message1");
63+
}
64+
65+
@Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "Message2")
66+
public void message2() {
67+
throw new RuntimeException("Message2");
68+
}
69+
70+
@Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "Message3")
71+
public void message3() {
72+
throw new RuntimeException("Message3");
73+
}
74+
75+
@Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "Message4")
76+
public void message4() {
77+
throw new RuntimeException("Message4");
78+
}
79+
80+
@Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "Message5")
81+
public void message5() {
82+
throw new RuntimeException("Message5");
83+
}
84+
85+
@Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "Message6")
86+
public void message6() {
87+
throw new RuntimeException("Message6");
88+
}
89+
90+
@Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "Message7")
91+
public void message7() {
92+
throw new RuntimeException("Message7");
93+
}
94+
95+
@Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "Message8")
96+
public void message8() {
97+
throw new RuntimeException("Message8");
98+
}
99+
100+
@Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "Message9")
101+
public void message9() {
102+
throw new RuntimeException("Message9");
103+
}
104+
105+
@Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "Message10")
106+
public void message10() {
107+
throw new RuntimeException("Message10");
108+
}
109+
110+
111+
@Configuration
112+
static class Config {
113+
}
114+
115+
}
116+
117+
}

‎spring-test/src/test/java/org/springframework/test/context/testng/TestNGTestSuite.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.test.context.testng;
1818

19+
import org.junit.platform.suite.api.IncludeClassNamePatterns;
1920
import org.junit.platform.suite.api.IncludeEngines;
2021
import org.junit.platform.suite.api.SelectPackages;
2122
import org.junit.platform.suite.api.Suite;
@@ -40,7 +41,8 @@
4041
* @since 5.3.11
4142
*/
4243
@Suite
43-
@IncludeEngines("testng")
44+
@IncludeEngines({"testng", "junit-jupiter"})
4445
@SelectPackages("org.springframework.test.context.testng")
46+
@IncludeClassNamePatterns(".*Tests?$")
4547
class TestNGTestSuite {
4648
}

‎spring-test/src/test/java/org/springframework/test/context/testng/TrackingTestNGTestListener.java‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@
1616

1717
package org.springframework.test.context.testng;
1818

19+
import java.util.ArrayList;
20+
import java.util.List;
21+
import java.util.concurrent.atomic.AtomicInteger;
22+
1923
import org.testng.ITestContext;
2024
import org.testng.ITestListener;
2125
import org.testng.ITestResult;
@@ -29,18 +33,20 @@
2933
*/
3034
public class TrackingTestNGTestListener implements ITestListener {
3135

32-
public int testStartCount = 0;
36+
public final List<Throwable> throwables = new ArrayList<>();
37+
38+
public final AtomicInteger testStartCount = new AtomicInteger();
3339

34-
public int testSuccessCount = 0;
40+
public final AtomicInteger testSuccessCount = new AtomicInteger();
3541

36-
public int testFailureCount = 0;
42+
public final AtomicInteger testFailureCount = new AtomicInteger();
3743

38-
public int failedConfigurationsCount = 0;
44+
public final AtomicInteger failedConfigurationsCount = new AtomicInteger();
3945

4046

4147
@Override
4248
public void onFinish(ITestContext testContext) {
43-
this.failedConfigurationsCount += testContext.getFailedConfigurations().size();
49+
this.failedConfigurationsCount.addAndGet(testContext.getFailedConfigurations().size());
4450
}
4551

4652
@Override
@@ -53,7 +59,12 @@ public void onTestFailedButWithinSuccessPercentage(ITestResult testResult) {
5359

5460
@Override
5561
public void onTestFailure(ITestResult testResult) {
56-
this.testFailureCount++;
62+
this.testFailureCount.incrementAndGet();
63+
64+
Throwable throwable = testResult.getThrowable();
65+
if (throwable != null) {
66+
this.throwables.add(throwable);
67+
}
5768
}
5869

5970
@Override
@@ -62,12 +73,12 @@ public void onTestSkipped(ITestResult testResult) {
6273

6374
@Override
6475
public void onTestStart(ITestResult testResult) {
65-
this.testStartCount++;
76+
this.testStartCount.incrementAndGet();
6677
}
6778

6879
@Override
6980
public void onTestSuccess(ITestResult testResult) {
70-
this.testSuccessCount++;
81+
this.testSuccessCount.incrementAndGet();
7182
}
7283

7384
}

‎src/checkstyle/checkstyle-suppressions.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@
115115
<suppress files="src[\\/]test[\\/]java[\\/]org[\\/]springframework[\\/]test[\\/]web[\\/](client|reactive|servlet)[\\/].+Tests" checks="IllegalImport" id="bannedHamcrestImports"/>
116116
<suppress files="src[\\/]test[\\/]java[\\/]org[\\/]springframework[\\/]test[\\/]context[\\/](aot|junit4)" checks="SpringJUnit5"/>
117117
<suppress files="AutowiredConfigurationErrorsIntegrationTests" checks="SpringJUnit5" message="Lifecycle method .+ should not be private"/>
118-
<suppress files="org[\\/]springframework[\\/]test[\\/]context[\\/].+[\\/](ExpectedExceptionSpringRunnerTests|StandardJUnit4FeaturesTests|ProgrammaticTxMgmtTestNGTests)" checks="RegexpSinglelineJava" id="expectedExceptionAnnotation"/>
118+
<suppress files="org[\\/]springframework[\\/]test[\\/]context[\\/].+[\\/](ExpectedExceptionSpringRunnerTests|StandardJUnit4FeaturesTests|TestNGConcurrencyTests|ProgrammaticTxMgmtTestNGTests)" checks="RegexpSinglelineJava" id="expectedExceptionAnnotation"/>
119119

120120
<!-- spring-web -->
121121
<suppress files="SpringHandlerInstantiator" checks="JavadocStyle"/>

0 commit comments

Comments
 (0)