Streamline data providers & retrial of failed tests

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

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 <code>7.8.0</code>
+ */
+@Deprecated
 public class DataProviderInvocationException extends TestNGException {
 
   public DataProviderInvocationException(String string, Throwable t) {

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<String, String> 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));

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);

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

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