From 290532eb56d2e60e4cd39ecf66a2cd2204acdc85 Mon Sep 17 00:00:00 2001 From: morskyrv <18137385+morskyrv@users.noreply.github.com> Date: Sun, 12 Dec 2021 05:47:48 +0200 Subject: [PATCH] Fix: AfterGroups config annotation does not consider retries for tests (#2690) --- CHANGES.txt | 1 + .../testng/internal/invokers/TestInvoker.java | 101 +++++++++--------- .../test/retryAnalyzer/RetryAnalyzerTest.java | 30 ++++++ .../issue2684/RerunAnalyzer.java | 20 ++++ .../SampleTestClassWithGroupConfigs.java | 54 ++++++++++ 5 files changed, 158 insertions(+), 48 deletions(-) create mode 100644 testng-core/src/test/java/test/retryAnalyzer/issue2684/RerunAnalyzer.java create mode 100644 testng-core/src/test/java/test/retryAnalyzer/issue2684/SampleTestClassWithGroupConfigs.java diff --git a/CHANGES.txt b/CHANGES.txt index 185e4c2d01..d1184fcf5f 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ Current +Fixed: GITHUB-2684: AfterGroups config annotation does not consider retries for tests (Roman Morskyi) Fixed: GITHUB-2689: Yaml parser: implement loadClasses flag (Dzmitry Sankouski) Fixed: GITHUB-2676: NPE is triggered when working with ITestObjectFactory (Krishnan Mahadevan) Fixed: GITHUB-2674: Run onTestSkipped for each value from data provider (Krishnan Mahadevan) diff --git a/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java index 4d180aa0c4..bf10ca7b90 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java @@ -508,34 +508,18 @@ private static void setTestStatus(ITestResult result, int status) { private static class StatusHolder { boolean handled = false; + int originalStatus; int status; } - private void handleInvocationResults( + private void handleInvocationResult( ITestNGMethod testMethod, ITestResult testResult, FailureContext failure, StatusHolder holder, - boolean wasResultUnaltered) { - // - // Go through all the results and create a TestResult for each of them - // - List resultsToRetry = Lists.newArrayList(); - - Throwable ite = testResult.getThrowable(); - int status = - computeTestStatusComparingTestResultAndStatusHolder(testResult, holder, wasResultUnaltered); - boolean handled = holder.handled; - IRetryAnalyzer retryAnalyzer = testMethod.getRetryAnalyzer(testResult); - - boolean willRetry = - retryAnalyzer != null - && status == ITestResult.FAILURE - && failure.instances != null - && retryAnalyzer.retry(testResult); + boolean willRetry) { if (willRetry) { - resultsToRetry.add(testResult); Object instance = testResult.getInstance(); if (!failure.instances.contains(instance)) { failure.instances.add(instance); @@ -543,17 +527,30 @@ private void handleInvocationResults( testResult.setStatus(ITestResult.SKIP); testResult.setWasRetried(true); } else { - testResult.setStatus(status); - if (status == ITestResult.FAILURE && !handled) { + testResult.setStatus(holder.status); + if (holder.status == ITestResult.FAILURE && !holder.handled) { int count = failure.count++; if (testMethod.isDataDriven()) { count = 0; } - handleException(ite, testMethod, testResult, count); + handleException(testResult.getThrowable(), testMethod, testResult, count); } } } + private boolean shouldRetryTestMethod( + ITestNGMethod testMethod, + ITestResult testResult, + FailureContext failure, + StatusHolder holder) { + IRetryAnalyzer retryAnalyzer = testMethod.getRetryAnalyzer(testResult); + + return retryAnalyzer != null + && holder.status == ITestResult.FAILURE + && failure.instances != null + && retryAnalyzer.retry(testResult); + } + // pass both paramValues and paramIndex to be thread safe in case parallel=true + dataprovider. private ITestResult invokeMethod( TestMethodArguments arguments, XmlSuite suite, FailureContext failureContext) { @@ -597,7 +594,8 @@ private ITestResult invokeMethod( testResult.setMethod(arguments.getTestMethod()); invokedMethod = new InvokedMethod(startTime, result); invokeListenersForSkippedTestResult(result, invokedMethod); - runAfterGroupsConfigurations(arguments, suite, testResult); + runAfterConfigurations(arguments, suite, testResult); + runAfterGroupsConfigurations(arguments); return result; } @@ -699,11 +697,12 @@ private ITestResult invokeMethod( StatusHolder holder = considerExceptions( arguments.getTestMethod(), testResult, expectedExceptionClasses, failureContext); - int statusBeforeListenerInvocation = testResult.getStatus(); runInvokedMethodListeners(AFTER_INVOCATION, invokedMethod, testResult); - boolean wasResultUnaltered = statusBeforeListenerInvocation == testResult.getStatus(); - handleInvocationResults( - arguments.getTestMethod(), testResult, failureContext, holder, wasResultUnaltered); + updateStatusHolderAccordingToTestResult(testResult, holder); + boolean willRetryMethod = + shouldRetryTestMethod(arguments.getTestMethod(), testResult, failureContext, holder); + handleInvocationResult( + arguments.getTestMethod(), testResult, failureContext, holder, willRetryMethod); // If this method has a data provider and just failed, memorize the number // at which it failed. @@ -730,7 +729,10 @@ private ITestResult invokeMethod( collectResults(arguments.getTestMethod(), testResult); - runAfterGroupsConfigurations(arguments, suite, testResult); + runAfterConfigurations(arguments, suite, testResult); + if (!willRetryMethod) { + runAfterGroupsConfigurations(arguments); + } // Reset the test result last. If we do this too early, Reporter.log() // invocations from listeners will be discarded @@ -740,13 +742,15 @@ private ITestResult invokeMethod( return testResult; } - private void runAfterGroupsConfigurations( + private void runAfterConfigurations( TestMethodArguments arguments, XmlSuite suite, TestResult testResult) { ITestNGMethod[] teardownConfigMethods = TestNgMethodUtils.filterTeardownConfigurationMethods( arguments.getTestMethod(), arguments.getAfterMethods()); runConfigMethods(arguments, suite, testResult, teardownConfigMethods); + } + private void runAfterGroupsConfigurations(TestMethodArguments arguments) { GroupConfigMethodArguments grpArgs = new GroupConfigMethodArguments.Builder() .forTestMethod(arguments.getTestMethod()) @@ -795,50 +799,51 @@ public ITestResult registerSkippedTestResult( private StatusHolder considerExceptions( ITestNGMethod tm, - ITestResult testresult, + ITestResult testResult, ExpectedExceptionsHolder exceptionsHolder, FailureContext failure) { StatusHolder holder = new StatusHolder(); - holder.status = testresult.getStatus(); + int status = testResult.getStatus(); holder.handled = false; - Throwable ite = testresult.getThrowable(); - if (holder.status == ITestResult.FAILURE && ite != null) { + Throwable ite = testResult.getThrowable(); + if (status == ITestResult.FAILURE && ite != null) { // Invocation caused an exception, see if the method was annotated with @ExpectedException if (exceptionsHolder != null) { if (exceptionsHolder.isExpectedException(ite)) { - testresult.setStatus(ITestResult.SUCCESS); - holder.status = ITestResult.SUCCESS; + testResult.setStatus(ITestResult.SUCCESS); + status = ITestResult.SUCCESS; } else { if (isSkipExceptionAndSkip(ite)) { - holder.status = ITestResult.SKIP; + status = ITestResult.SKIP; } else { - testresult.setThrowable(exceptionsHolder.wrongException(ite)); - holder.status = ITestResult.FAILURE; + testResult.setThrowable(exceptionsHolder.wrongException(ite)); + status = ITestResult.FAILURE; } } } else { - handleException(ite, tm, testresult, failure.count++); + handleException(ite, tm, testResult, failure.count++); holder.handled = true; - holder.status = testresult.getStatus(); + status = testResult.getStatus(); } - } else if (holder.status != ITestResult.SKIP && exceptionsHolder != null) { + } else if (status != ITestResult.SKIP && exceptionsHolder != null) { TestException exception = exceptionsHolder.noException(tm); if (exception != null) { - testresult.setThrowable(exception); - holder.status = ITestResult.FAILURE; + testResult.setThrowable(exception); + status = ITestResult.FAILURE; } } + holder.originalStatus = testResult.getStatus(); + holder.status = status; return holder; } - private static int computeTestStatusComparingTestResultAndStatusHolder( - ITestResult testResult, StatusHolder holder, boolean wasResultUnaltered) { - if (wasResultUnaltered) { - return holder.status; + private static void updateStatusHolderAccordingToTestResult( + ITestResult testResult, StatusHolder holder) { + if (holder.originalStatus != testResult.getStatus()) { + holder.status = testResult.getStatus(); } - return testResult.getStatus(); } private class MethodInvocationAgent { diff --git a/testng-core/src/test/java/test/retryAnalyzer/RetryAnalyzerTest.java b/testng-core/src/test/java/test/retryAnalyzer/RetryAnalyzerTest.java index a8d1f01204..d544e77946 100644 --- a/testng-core/src/test/java/test/retryAnalyzer/RetryAnalyzerTest.java +++ b/testng-core/src/test/java/test/retryAnalyzer/RetryAnalyzerTest.java @@ -37,6 +37,7 @@ import test.retryAnalyzer.issue1946.RetryAnalyzer; import test.retryAnalyzer.issue1946.TestclassSample1; import test.retryAnalyzer.issue1946.TestclassSample2; +import test.retryAnalyzer.issue2684.SampleTestClassWithGroupConfigs; public class RetryAnalyzerTest extends SimpleBaseTest { @Test @@ -257,6 +258,35 @@ public void testFailedRetryWithParameters() { Assert.assertEquals(RetryTestSample.count, 3); } + @Test(description = "GITHUB-2684") + public void testAfterConfigurationsInvokedAfterRetriedMethod() { + XmlSuite xmlSuite = createXmlSuite("2684_suite"); + createXmlTest(xmlSuite, "2684_test", SampleTestClassWithGroupConfigs.class); + createXmlGroups(xmlSuite, "2684_group"); + TestNG testng = create(xmlSuite); + InvokedMethodNameListener listener = new InvokedMethodNameListener(); + testng.addListener(listener); + testng.run(); + + String[] expected = { + "beforeSuite", + "beforeTest", + "beforeClass", + "beforeGroups", + "beforeMethod", + "testMethod", + "afterMethod", + "beforeMethod", + "testMethod", + "afterMethod", + "afterGroups", + "afterClass", + "afterTest", + "afterSuite" + }; + assertThat(listener.getInvokedMethodNames()).containsExactly(expected); + } + private ITestResult runAssertions(Set results, String methodName) { assertThat(results).hasSize(1); ITestResult firstResult = results.iterator().next(); diff --git a/testng-core/src/test/java/test/retryAnalyzer/issue2684/RerunAnalyzer.java b/testng-core/src/test/java/test/retryAnalyzer/issue2684/RerunAnalyzer.java new file mode 100644 index 0000000000..43fe0df1b9 --- /dev/null +++ b/testng-core/src/test/java/test/retryAnalyzer/issue2684/RerunAnalyzer.java @@ -0,0 +1,20 @@ +package test.retryAnalyzer.issue2684; + +import org.testng.IRetryAnalyzer; +import org.testng.ITestResult; + +public class RerunAnalyzer implements IRetryAnalyzer { + + public static final int maxRetryCount = 1; + public static int secondTestRetryCount = 0; + private int retryCount = 0; + + @Override + public boolean retry(ITestResult iTestResult) { + if (retryCount < maxRetryCount) { + retryCount++; + return true; + } + return false; + } +} diff --git a/testng-core/src/test/java/test/retryAnalyzer/issue2684/SampleTestClassWithGroupConfigs.java b/testng-core/src/test/java/test/retryAnalyzer/issue2684/SampleTestClassWithGroupConfigs.java new file mode 100644 index 0000000000..ebd51cfc29 --- /dev/null +++ b/testng-core/src/test/java/test/retryAnalyzer/issue2684/SampleTestClassWithGroupConfigs.java @@ -0,0 +1,54 @@ +package test.retryAnalyzer.issue2684; + +import static org.testng.Assert.assertTrue; + +import org.testng.annotations.AfterClass; +import org.testng.annotations.AfterGroups; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.AfterSuite; +import org.testng.annotations.AfterTest; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.BeforeGroups; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.BeforeSuite; +import org.testng.annotations.BeforeTest; +import org.testng.annotations.Test; + +public class SampleTestClassWithGroupConfigs { + + @BeforeSuite(alwaysRun = true) + public void beforeSuite() {} + + @BeforeTest(alwaysRun = true) + public void beforeTest() {} + + @BeforeGroups("2684_group") + public void beforeGroups() {} + + @BeforeClass(alwaysRun = true) + public void beforeClass() {} + + @BeforeMethod(alwaysRun = true) + public void beforeMethod() {} + + @Test(groups = "2684_group", retryAnalyzer = RerunAnalyzer.class) + public void testMethod() { + RerunAnalyzer.secondTestRetryCount++; + assertTrue(RerunAnalyzer.secondTestRetryCount > RerunAnalyzer.maxRetryCount); + } + + @AfterMethod(alwaysRun = true) + public void afterMethod() {} + + @AfterClass(alwaysRun = true) + public void afterClass() {} + + @AfterGroups("2684_group") + public void afterGroups() {} + + @AfterTest(alwaysRun = true) + public void afterTest() {} + + @AfterSuite(alwaysRun = true) + public void afterSuite() {} +}