From 1d581c1a9ee4b6c534cda47a08a79d0d40a97c7b Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Fri, 17 Mar 2023 19:09:28 +0530 Subject: [PATCH] Streamline data providers & retrial of failed tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #2884 When a data driven test fails, then ensure that we don’t call the data provider once again when retrying the test (we seem to invoke the data provider multiple times, but ignore the value) As part of fixing this, getting rid of the test for #2157. #2157 - deals with null pointer exceptions arising Out of data provider that was re-invoked as a result Of us retrying a failed test. But now since a data provider is NOT going to be re-invoked the changes for #2157 become irrelevant. --- CHANGES.txt | 1 + .../DataProviderInvocationException.java | 7 ++- .../testng/internal/invokers/TestInvoker.java | 24 +-------- .../test/dataprovider/DataProviderTest.java | 8 +++ .../dataprovider/FailingDataProviderTest.java | 12 ----- ...sWithDataProviderThatThrowsExceptions.java | 49 ------------------- .../issue2884/TestClassSample.java | 48 ++++++++++++++++++ 7 files changed, 64 insertions(+), 85 deletions(-) delete mode 100644 testng-core/src/test/java/test/dataprovider/issue2157/TestClassWithDataProviderThatThrowsExceptions.java create mode 100644 testng-core/src/test/java/test/dataprovider/issue2884/TestClassSample.java diff --git a/CHANGES.txt b/CHANGES.txt index 97548b3288..c038b580ed 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ Current +Fixed: GITHUB-2884: Discrepancies with DataProvider and Retry of failed tests (Krishnan Mahadevan) Fixed: GITHUB-2879: Test listeners specified in parent testng.xml file are not included in testng-failed.xml file (Krishnan Mahadevan) Fixed: GITHUB-2866: TestNG.xml doesn't honour Parallel value of a clone (Krishnan Mahadevan) Fixed: GITHUB-2875: JUnitReportReporter should capture the test case output at the test case level diff --git a/testng-core/src/main/java/org/testng/DataProviderInvocationException.java b/testng-core/src/main/java/org/testng/DataProviderInvocationException.java index 752bf18ec6..068681cfb4 100644 --- a/testng-core/src/main/java/org/testng/DataProviderInvocationException.java +++ b/testng-core/src/main/java/org/testng/DataProviderInvocationException.java @@ -1,6 +1,11 @@ package org.testng; -/** Represents any issues that arise out of invoking a data provider method. */ +/** + * Represents any issues that arise out of invoking a data provider method. + * + * @deprecated - This class stands deprecated as of 7.8.0 + */ +@Deprecated public class DataProviderInvocationException extends TestNGException { public DataProviderInvocationException(String string, Throwable t) { 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 89c0fdffb5..31bd88c4d1 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 @@ -21,7 +21,6 @@ import java.util.function.Consumer; import java.util.stream.Collectors; import org.testng.DataProviderHolder; -import org.testng.DataProviderInvocationException; import org.testng.IClassListener; import org.testng.IDataProviderListener; import org.testng.IHookable; @@ -238,33 +237,12 @@ public FailureContext retryFailed( failure.representsRetriedMethod = true; do { failure.instances = Lists.newArrayList(); - Map allParameters = Maps.newHashMap(); - int verbose = testContext.getCurrentXmlTest().getVerbose(); - // TODO: This recreates all the parameters every time when we only need - // one specific set. Should optimize it by only recreating the set needed. - ParameterHandler handler = - new ParameterHandler( - m_configuration.getObjectFactory(), annotationFinder(), this.holder, verbose); - - ParameterBag bag = - handler.createParameters( - arguments.getTestMethod(), arguments.getParameters(), allParameters, testContext); - ITestResult errorResult = bag.errorResult; - - if (errorResult != null) { - Throwable cause = errorResult.getThrowable(); - String m = errorResult.getMethod().getMethodName(); - String msg = - String.format( - "Encountered problems when gathering parameter values for [%s]. Root cause: ", m); - throw new DataProviderInvocationException(msg, cause); - } Object[] parameterValues = arguments.getParameterValues(); TestMethodArguments tma = new TestMethodArguments.Builder() .usingArguments(arguments) .withParameterValues(parameterValues) - .withParameters(allParameters) + .withParameters(arguments.getParameters()) .build(); result.add(invokeMethod(tma, testContext.getSuite().getXmlSuite(), failure)); diff --git a/testng-core/src/test/java/test/dataprovider/DataProviderTest.java b/testng-core/src/test/java/test/dataprovider/DataProviderTest.java index a94208cd64..9a0b089b5f 100644 --- a/testng-core/src/test/java/test/dataprovider/DataProviderTest.java +++ b/testng-core/src/test/java/test/dataprovider/DataProviderTest.java @@ -473,6 +473,14 @@ public void testDataProvidersThatReturnNull() { assertThat(result.getThrowable().getMessage()).contains(msg); } + @Test + public void ensureDataProviderNotInvokedMultipleTimesForRetriedTests() { + TestNG testng = create(test.dataprovider.issue2884.TestClassSample.class); + testng.run(); + assertThat(test.dataprovider.issue2884.TestClassSample.dataProviderInvocationCount.get()) + .isEqualTo(1); + } + @Test public void retryWithDataProvider() { TestNG testng = create(DataProviderRetryTest.class); diff --git a/testng-core/src/test/java/test/dataprovider/FailingDataProviderTest.java b/testng-core/src/test/java/test/dataprovider/FailingDataProviderTest.java index 358990fb6c..965752dd73 100644 --- a/testng-core/src/test/java/test/dataprovider/FailingDataProviderTest.java +++ b/testng-core/src/test/java/test/dataprovider/FailingDataProviderTest.java @@ -2,12 +2,9 @@ import static org.assertj.core.api.Assertions.assertThat; -import org.testng.DataProviderInvocationException; -import org.testng.ITestResult; import org.testng.annotations.Test; import test.InvokedMethodNameListener; import test.SimpleBaseTest; -import test.dataprovider.issue2157.TestClassWithDataProviderThatThrowsExceptions; public class FailingDataProviderTest extends SimpleBaseTest { @@ -36,13 +33,4 @@ public void failingDataProviderAndInvocationCount() { "testShouldSkipEvenIfSuccessPercentage", "testShouldSkipEvenIfSuccessPercentage"); } - - @Test(description = "GITHUB-2157") - public void abortWhenDataProviderThrowsException() { - InvokedMethodNameListener listener = run(TestClassWithDataProviderThatThrowsExceptions.class); - ITestResult result = listener.getResult("testMethod"); - Throwable cause = result.getThrowable(); - assertThat(cause).isInstanceOf(DataProviderInvocationException.class); - assertThat(result.getStatus()).isEqualTo(ITestResult.FAILURE); - } } diff --git a/testng-core/src/test/java/test/dataprovider/issue2157/TestClassWithDataProviderThatThrowsExceptions.java b/testng-core/src/test/java/test/dataprovider/issue2157/TestClassWithDataProviderThatThrowsExceptions.java deleted file mode 100644 index 898295e34d..0000000000 --- a/testng-core/src/test/java/test/dataprovider/issue2157/TestClassWithDataProviderThatThrowsExceptions.java +++ /dev/null @@ -1,49 +0,0 @@ -package test.dataprovider.issue2157; - -import java.util.concurrent.atomic.AtomicInteger; -import org.testng.Assert; -import org.testng.IRetryAnalyzer; -import org.testng.ITestResult; -import org.testng.annotations.DataProvider; -import org.testng.annotations.Test; - -public class TestClassWithDataProviderThatThrowsExceptions { - - @Test(dataProvider = "dp", retryAnalyzer = SimplyRetry.class) - public void testMethod(String i) { - if ("First".equalsIgnoreCase(i) || "Second".equalsIgnoreCase(i)) { - Assert.fail(); - } - } - - private static AtomicInteger counter = new AtomicInteger(); - - @DataProvider(name = "dp") - public static Object[][] dpWithException() { - return new Object[][] { - {foo()}, - }; - } - - private static String foo() { - counter.getAndIncrement(); - - if (counter.get() == 1) { - return "First"; - } - if (counter.get() == 2) { - return "Second"; - } - throw new RuntimeException("TestNG doesn't handle an exception"); - } - - public static class SimplyRetry implements IRetryAnalyzer { - - private static int attempts = 1; - - @Override - public boolean retry(ITestResult result) { - return attempts++ != 3; - } - } -} diff --git a/testng-core/src/test/java/test/dataprovider/issue2884/TestClassSample.java b/testng-core/src/test/java/test/dataprovider/issue2884/TestClassSample.java new file mode 100644 index 0000000000..639af7bc27 --- /dev/null +++ b/testng-core/src/test/java/test/dataprovider/issue2884/TestClassSample.java @@ -0,0 +1,48 @@ +package test.dataprovider.issue2884; + +import java.util.concurrent.atomic.AtomicInteger; +import org.testng.Assert; +import org.testng.IRetryAnalyzer; +import org.testng.ITestResult; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class TestClassSample { + + public static final AtomicInteger dataProviderInvocationCount = new AtomicInteger(0); + + @Test(dataProvider = "dp", retryAnalyzer = TryAgain.class) + public void testMethod(Pojo user) { + Assert.fail(); + } + + @DataProvider(name = "dp") + public static Object[][] getTestData() { + dataProviderInvocationCount.incrementAndGet(); + return new Object[][] {{new Pojo().setName("John")}}; + } + + public static class TryAgain implements IRetryAnalyzer { + + private int counter = 1; + + @Override + public boolean retry(ITestResult result) { + return counter++ != 3; + } + } + + public static class Pojo { + + private String name; + + public String getName() { + return name; + } + + public Pojo setName(String name) { + this.name = name; + return this; + } + } +}