Skip to content

Commit 290532e

Browse files
morskyrvmorskyrv
authored
Fix: AfterGroups config annotation does not consider retries for tests (#2690)
1 parent 2a756e5 commit 290532e

File tree

5 files changed

+158
-48
lines changed

5 files changed

+158
-48
lines changed

‎CHANGES.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
Current
2+
Fixed: GITHUB-2684: AfterGroups config annotation does not consider retries for tests (Roman Morskyi)
23
Fixed: GITHUB-2689: Yaml parser: implement loadClasses flag (Dzmitry Sankouski)
34
Fixed: GITHUB-2676: NPE is triggered when working with ITestObjectFactory (Krishnan Mahadevan)
45
Fixed: GITHUB-2674: Run onTestSkipped for each value from data provider (Krishnan Mahadevan)

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

Lines changed: 53 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -508,52 +508,49 @@ private static void setTestStatus(ITestResult result, int status) {
508508
private static class StatusHolder {
509509

510510
boolean handled = false;
511+
int originalStatus;
511512
int status;
512513
}
513514

514-
private void handleInvocationResults(
515+
private void handleInvocationResult(
515516
ITestNGMethod testMethod,
516517
ITestResult testResult,
517518
FailureContext failure,
518519
StatusHolder holder,
519-
boolean wasResultUnaltered) {
520-
//
521-
// Go through all the results and create a TestResult for each of them
522-
//
523-
List<ITestResult> resultsToRetry = Lists.newArrayList();
524-
525-
Throwable ite = testResult.getThrowable();
526-
int status =
527-
computeTestStatusComparingTestResultAndStatusHolder(testResult, holder, wasResultUnaltered);
528-
boolean handled = holder.handled;
529-
IRetryAnalyzer retryAnalyzer = testMethod.getRetryAnalyzer(testResult);
530-
531-
boolean willRetry =
532-
retryAnalyzer != null
533-
&& status == ITestResult.FAILURE
534-
&& failure.instances != null
535-
&& retryAnalyzer.retry(testResult);
520+
boolean willRetry) {
536521

537522
if (willRetry) {
538-
resultsToRetry.add(testResult);
539523
Object instance = testResult.getInstance();
540524
if (!failure.instances.contains(instance)) {
541525
failure.instances.add(instance);
542526
}
543527
testResult.setStatus(ITestResult.SKIP);
544528
testResult.setWasRetried(true);
545529
} else {
546-
testResult.setStatus(status);
547-
if (status == ITestResult.FAILURE && !handled) {
530+
testResult.setStatus(holder.status);
531+
if (holder.status == ITestResult.FAILURE && !holder.handled) {
548532
int count = failure.count++;
549533
if (testMethod.isDataDriven()) {
550534
count = 0;
551535
}
552-
handleException(ite, testMethod, testResult, count);
536+
handleException(testResult.getThrowable(), testMethod, testResult, count);
553537
}
554538
}
555539
}
556540

541+
private boolean shouldRetryTestMethod(
542+
ITestNGMethod testMethod,
543+
ITestResult testResult,
544+
FailureContext failure,
545+
StatusHolder holder) {
546+
IRetryAnalyzer retryAnalyzer = testMethod.getRetryAnalyzer(testResult);
547+
548+
return retryAnalyzer != null
549+
&& holder.status == ITestResult.FAILURE
550+
&& failure.instances != null
551+
&& retryAnalyzer.retry(testResult);
552+
}
553+
557554
// pass both paramValues and paramIndex to be thread safe in case parallel=true + dataprovider.
558555
private ITestResult invokeMethod(
559556
TestMethodArguments arguments, XmlSuite suite, FailureContext failureContext) {
@@ -597,7 +594,8 @@ private ITestResult invokeMethod(
597594
testResult.setMethod(arguments.getTestMethod());
598595
invokedMethod = new InvokedMethod(startTime, result);
599596
invokeListenersForSkippedTestResult(result, invokedMethod);
600-
runAfterGroupsConfigurations(arguments, suite, testResult);
597+
runAfterConfigurations(arguments, suite, testResult);
598+
runAfterGroupsConfigurations(arguments);
601599

602600
return result;
603601
}
@@ -699,11 +697,12 @@ private ITestResult invokeMethod(
699697
StatusHolder holder =
700698
considerExceptions(
701699
arguments.getTestMethod(), testResult, expectedExceptionClasses, failureContext);
702-
int statusBeforeListenerInvocation = testResult.getStatus();
703700
runInvokedMethodListeners(AFTER_INVOCATION, invokedMethod, testResult);
704-
boolean wasResultUnaltered = statusBeforeListenerInvocation == testResult.getStatus();
705-
handleInvocationResults(
706-
arguments.getTestMethod(), testResult, failureContext, holder, wasResultUnaltered);
701+
updateStatusHolderAccordingToTestResult(testResult, holder);
702+
boolean willRetryMethod =
703+
shouldRetryTestMethod(arguments.getTestMethod(), testResult, failureContext, holder);
704+
handleInvocationResult(
705+
arguments.getTestMethod(), testResult, failureContext, holder, willRetryMethod);
707706

708707
// If this method has a data provider and just failed, memorize the number
709708
// at which it failed.
@@ -730,7 +729,10 @@ private ITestResult invokeMethod(
730729

731730
collectResults(arguments.getTestMethod(), testResult);
732731

733-
runAfterGroupsConfigurations(arguments, suite, testResult);
732+
runAfterConfigurations(arguments, suite, testResult);
733+
if (!willRetryMethod) {
734+
runAfterGroupsConfigurations(arguments);
735+
}
734736

735737
// Reset the test result last. If we do this too early, Reporter.log()
736738
// invocations from listeners will be discarded
@@ -740,13 +742,15 @@ private ITestResult invokeMethod(
740742
return testResult;
741743
}
742744

743-
private void runAfterGroupsConfigurations(
745+
private void runAfterConfigurations(
744746
TestMethodArguments arguments, XmlSuite suite, TestResult testResult) {
745747
ITestNGMethod[] teardownConfigMethods =
746748
TestNgMethodUtils.filterTeardownConfigurationMethods(
747749
arguments.getTestMethod(), arguments.getAfterMethods());
748750
runConfigMethods(arguments, suite, testResult, teardownConfigMethods);
751+
}
749752

753+
private void runAfterGroupsConfigurations(TestMethodArguments arguments) {
750754
GroupConfigMethodArguments grpArgs =
751755
new GroupConfigMethodArguments.Builder()
752756
.forTestMethod(arguments.getTestMethod())
@@ -795,50 +799,51 @@ public ITestResult registerSkippedTestResult(
795799

796800
private StatusHolder considerExceptions(
797801
ITestNGMethod tm,
798-
ITestResult testresult,
802+
ITestResult testResult,
799803
ExpectedExceptionsHolder exceptionsHolder,
800804
FailureContext failure) {
801805
StatusHolder holder = new StatusHolder();
802-
holder.status = testresult.getStatus();
806+
int status = testResult.getStatus();
803807
holder.handled = false;
804808

805-
Throwable ite = testresult.getThrowable();
806-
if (holder.status == ITestResult.FAILURE && ite != null) {
809+
Throwable ite = testResult.getThrowable();
810+
if (status == ITestResult.FAILURE && ite != null) {
807811

808812
// Invocation caused an exception, see if the method was annotated with @ExpectedException
809813
if (exceptionsHolder != null) {
810814
if (exceptionsHolder.isExpectedException(ite)) {
811-
testresult.setStatus(ITestResult.SUCCESS);
812-
holder.status = ITestResult.SUCCESS;
815+
testResult.setStatus(ITestResult.SUCCESS);
816+
status = ITestResult.SUCCESS;
813817
} else {
814818
if (isSkipExceptionAndSkip(ite)) {
815-
holder.status = ITestResult.SKIP;
819+
status = ITestResult.SKIP;
816820
} else {
817-
testresult.setThrowable(exceptionsHolder.wrongException(ite));
818-
holder.status = ITestResult.FAILURE;
821+
testResult.setThrowable(exceptionsHolder.wrongException(ite));
822+
status = ITestResult.FAILURE;
819823
}
820824
}
821825
} else {
822-
handleException(ite, tm, testresult, failure.count++);
826+
handleException(ite, tm, testResult, failure.count++);
823827
holder.handled = true;
824-
holder.status = testresult.getStatus();
828+
status = testResult.getStatus();
825829
}
826-
} else if (holder.status != ITestResult.SKIP && exceptionsHolder != null) {
830+
} else if (status != ITestResult.SKIP && exceptionsHolder != null) {
827831
TestException exception = exceptionsHolder.noException(tm);
828832
if (exception != null) {
829-
testresult.setThrowable(exception);
830-
holder.status = ITestResult.FAILURE;
833+
testResult.setThrowable(exception);
834+
status = ITestResult.FAILURE;
831835
}
832836
}
837+
holder.originalStatus = testResult.getStatus();
838+
holder.status = status;
833839
return holder;
834840
}
835841

836-
private static int computeTestStatusComparingTestResultAndStatusHolder(
837-
ITestResult testResult, StatusHolder holder, boolean wasResultUnaltered) {
838-
if (wasResultUnaltered) {
839-
return holder.status;
842+
private static void updateStatusHolderAccordingToTestResult(
843+
ITestResult testResult, StatusHolder holder) {
844+
if (holder.originalStatus != testResult.getStatus()) {
845+
holder.status = testResult.getStatus();
840846
}
841-
return testResult.getStatus();
842847
}
843848

844849
private class MethodInvocationAgent {

‎testng-core/src/test/java/test/retryAnalyzer/RetryAnalyzerTest.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import test.retryAnalyzer.issue1946.RetryAnalyzer;
3838
import test.retryAnalyzer.issue1946.TestclassSample1;
3939
import test.retryAnalyzer.issue1946.TestclassSample2;
40+
import test.retryAnalyzer.issue2684.SampleTestClassWithGroupConfigs;
4041

4142
public class RetryAnalyzerTest extends SimpleBaseTest {
4243
@Test
@@ -257,6 +258,35 @@ public void testFailedRetryWithParameters() {
257258
Assert.assertEquals(RetryTestSample.count, 3);
258259
}
259260

261+
@Test(description = "GITHUB-2684")
262+
public void testAfterConfigurationsInvokedAfterRetriedMethod() {
263+
XmlSuite xmlSuite = createXmlSuite("2684_suite");
264+
createXmlTest(xmlSuite, "2684_test", SampleTestClassWithGroupConfigs.class);
265+
createXmlGroups(xmlSuite, "2684_group");
266+
TestNG testng = create(xmlSuite);
267+
InvokedMethodNameListener listener = new InvokedMethodNameListener();
268+
testng.addListener(listener);
269+
testng.run();
270+
271+
String[] expected = {
272+
"beforeSuite",
273+
"beforeTest",
274+
"beforeClass",
275+
"beforeGroups",
276+
"beforeMethod",
277+
"testMethod",
278+
"afterMethod",
279+
"beforeMethod",
280+
"testMethod",
281+
"afterMethod",
282+
"afterGroups",
283+
"afterClass",
284+
"afterTest",
285+
"afterSuite"
286+
};
287+
assertThat(listener.getInvokedMethodNames()).containsExactly(expected);
288+
}
289+
260290
private ITestResult runAssertions(Set<ITestResult> results, String methodName) {
261291
assertThat(results).hasSize(1);
262292
ITestResult firstResult = results.iterator().next();

‎testng-core/src/test/java/test/retryAnalyzer/issue2684/RerunAnalyzer.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package test.retryAnalyzer.issue2684;
2+
3+
import org.testng.IRetryAnalyzer;
4+
import org.testng.ITestResult;
5+
6+
public class RerunAnalyzer implements IRetryAnalyzer {
7+
8+
public static final int maxRetryCount = 1;
9+
public static int secondTestRetryCount = 0;
10+
private int retryCount = 0;
11+
12+
@Override
13+
public boolean retry(ITestResult iTestResult) {
14+
if (retryCount < maxRetryCount) {
15+
retryCount++;
16+
return true;
17+
}
18+
return false;
19+
}
20+
}

‎testng-core/src/test/java/test/retryAnalyzer/issue2684/SampleTestClassWithGroupConfigs.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package test.retryAnalyzer.issue2684;
2+
3+
import static org.testng.Assert.assertTrue;
4+
5+
import org.testng.annotations.AfterClass;
6+
import org.testng.annotations.AfterGroups;
7+
import org.testng.annotations.AfterMethod;
8+
import org.testng.annotations.AfterSuite;
9+
import org.testng.annotations.AfterTest;
10+
import org.testng.annotations.BeforeClass;
11+
import org.testng.annotations.BeforeGroups;
12+
import org.testng.annotations.BeforeMethod;
13+
import org.testng.annotations.BeforeSuite;
14+
import org.testng.annotations.BeforeTest;
15+
import org.testng.annotations.Test;
16+
17+
public class SampleTestClassWithGroupConfigs {
18+
19+
@BeforeSuite(alwaysRun = true)
20+
public void beforeSuite() {}
21+
22+
@BeforeTest(alwaysRun = true)
23+
public void beforeTest() {}
24+
25+
@BeforeGroups("2684_group")
26+
public void beforeGroups() {}
27+
28+
@BeforeClass(alwaysRun = true)
29+
public void beforeClass() {}
30+
31+
@BeforeMethod(alwaysRun = true)
32+
public void beforeMethod() {}
33+
34+
@Test(groups = "2684_group", retryAnalyzer = RerunAnalyzer.class)
35+
public void testMethod() {
36+
RerunAnalyzer.secondTestRetryCount++;
37+
assertTrue(RerunAnalyzer.secondTestRetryCount > RerunAnalyzer.maxRetryCount);
38+
}
39+
40+
@AfterMethod(alwaysRun = true)
41+
public void afterMethod() {}
42+
43+
@AfterClass(alwaysRun = true)
44+
public void afterClass() {}
45+
46+
@AfterGroups("2684_group")
47+
public void afterGroups() {}
48+
49+
@AfterTest(alwaysRun = true)
50+
public void afterTest() {}
51+
52+
@AfterSuite(alwaysRun = true)
53+
public void afterSuite() {}
54+
}

0 commit comments

Comments
 (0)