Streamline data providers & retrial of failed tests

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