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; + } + } +}