Handling invocation-number to xml for DataProvider + Factory (#2516)

Closes #2517

CHANGES.txt

@@ -1,4 +1,5 @@
 Current
+Fixed: GITHUB-2517: Factory data-provider parameters not displayed in 'testng-failed.xml' file (Pavel Sakharchuk)
 Fixed: GITHUB-279: Guice dependency injection into listeners and reporters (Krishnan Mahadevan)
 Fixed: GITHUB-2504: Data provider data cannot be populated to onTestStart hook when BeforeMethod is failed (Krishnan Mahadevan)
 Fixed: GITHUB-2489: Hierarchical base- and test-class @AfterClass methods out of order using groups (Krishnan Mahadevan)

core/src/main/java/org/testng/internal/FactoryMethod.java

@@ -181,22 +181,23 @@ public IParameterInfo[] invoke() {
                     com.getDeclaringClass().getName(), com.getName());
           }
           if (indices == null || indices.isEmpty()) {
+            int finalPosition = position;
             result.addAll(Arrays.stream(testInstances).map(instance ->
-              new ParameterInfo(instance, parameters)
+              new ParameterInfo(instance, finalPosition, parameters)
             ).collect(Collectors.toList()));
           } else {
             for (Integer index : indices) {
               int i = index - position;
               if (i >= 0 && i < testInstances.length) {
-                result.add(new ParameterInfo(testInstances[i], parameters));
+                result.add(new ParameterInfo(testInstances[i], position, parameters));
               }
             }
           }
           position += testInstances.length;
         } else {
           if (indices == null || indices.isEmpty() || indices.contains(position)) {
             Object instance  = m_objectFactory.newInstance(com.getConstructor(), parameters);
-            result.add(new ParameterInfo(instance,  parameters));
+            result.add(new ParameterInfo(instance, position, parameters));
           }
           position++;
         }

core/src/main/java/org/testng/internal/IParameterInfo.java

@@ -10,6 +10,11 @@ public interface IParameterInfo {
    */
   Object getInstance();
 
+  /**
+   * @return - The actual index of instance associated with a factory method
+   */
+  int getIndex();
+
   /**
    * @return - The parameters associated with the factory method as an array.
    */

core/src/main/java/org/testng/internal/ParameterInfo.java

@@ -2,10 +2,12 @@
 
 public class ParameterInfo implements IParameterInfo {
   private Object instance;
+  private final int index;
   private Object[] parameters;
 
-  public ParameterInfo(Object instance, Object[] parameters) {
+  public ParameterInfo(Object instance, int index, Object[] parameters) {
     this.instance = instance;
+    this.index = index;
     this.parameters = parameters;
   }
 
@@ -14,6 +16,11 @@ public Object getInstance() {
     return instance;
   }
 
+  @Override
+  public int getIndex() {
+    return index;
+  }
+
   @Override
   public Object[] getParameters() {
     return parameters;

core/src/main/java/org/testng/internal/invokers/TestInvoker.java

@@ -634,9 +634,14 @@ private ITestResult invokeMethod(
       // that it has parameters, so might have to revisit this if bugs get reported
       // for the case where this method has parameters that don't come from a data
       // provider
-      if (testResult.getThrowable() != null && arguments.getParameterValues().length > 0) {
+      if (testResult.getThrowable() != null &&
+              (arguments.getParameterValues().length > 0 || testResult.getFactoryParameters().length > 0)) {
+        int parametersIndex = arguments.getParametersIndex();
+        if (null != testResult.getMethod().getFactoryMethodParamsInfo()) {
+          parametersIndex = testResult.getMethod().getFactoryMethodParamsInfo().getIndex();
+        }
         arguments.getTestMethod()
-            .addFailedInvocationNumber(arguments.getParametersIndex());
+                .addFailedInvocationNumber(parametersIndex);
       }
 
       //

core/src/test/java/test/failedreporter/issue2517/DataProviderWithFactoryFailedReporterSample.java

@@ -0,0 +1,28 @@
+package test.failedreporter.issue2517;
+
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Factory;
+import org.testng.annotations.Test;
+
+public class DataProviderWithFactoryFailedReporterSample {
+    private Integer data;
+
+    @Factory(dataProvider = "dp")
+    public DataProviderWithFactoryFailedReporterSample(Integer data) {
+        this.data = data;
+    }
+
+    @DataProvider
+    public static Object[][] dp() {
+        return new Object[][]{
+                new Object[]{0}, new Object[]{1}, new Object[]{2},
+        };
+    }
+
+    @Test
+    public void f1() {
+        if (data == 1) {
+            throw new RuntimeException();
+        }
+    }
+}

core/src/test/java/test/failedreporter/issue2517/DataProviderWithFactoryFailedReporterTest.java

@@ -0,0 +1,55 @@
+package test.failedreporter.issue2517;
+
+import org.testng.Assert;
+import org.testng.TestNG;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+import org.testng.collections.Lists;
+import test.SimpleBaseTest;
+
+import java.io.File;
+import java.util.List;
+
+public class DataProviderWithFactoryFailedReporterTest extends SimpleBaseTest {
+    private File mTempDirectory;
+
+    @BeforeMethod
+    public void setUp() {
+        mTempDirectory = createDirInTempDir("testng-tmp-" + System.currentTimeMillis() % 1000);
+    }
+
+    @AfterMethod
+    public void tearDown() {
+        deleteDir(mTempDirectory);
+    }
+
+    @Test
+    public void failedMethodWithDataProviderAndFactoryShouldHaveInvocationNumbers() {
+        testFailedReporter(
+                new String[]{"f1"},
+                "<include name=\"%s\" invocation-numbers=\"1\"/>",
+                DataProviderWithFactoryFailedReporterSample.class);
+    }
+
+    private void testFailedReporter(String[] expectedMethods, String expectedLine, Class<?>... cls) {
+        triggerTest(cls);
+        runAssertions(mTempDirectory, expectedMethods, expectedLine, 1);
+    }
+
+    private void triggerTest(Class<?>... cls) {
+        TestNG tng = create(mTempDirectory.toPath(), cls);
+        tng.setUseDefaultListeners(true);
+        tng.run();
+    }
+
+    private static void runAssertions(
+            File outputDir, String[] expectedMethods, String expectedLine, int expected) {
+        File failed = new File(outputDir, "testng-failed.xml");
+        for (String s : expectedMethods) {
+            List<String> resultLines = Lists.newArrayList();
+            grep(failed, String.format(expectedLine, s), resultLines);
+            Assert.assertEquals(resultLines.size(), expected);
+        }
+    }
+}

core/src/test/resources/testng.xml

@@ -132,6 +132,7 @@
       <class name="test.aftergroups.AfterGroupsBehaviorTest"/>
       <class name="test.testng285.TestNG285Test" />
       <class name="test.failedreporter.FailedReporterTest" />
+      <class name="test.failedreporter.issue2517.DataProviderWithFactoryFailedReporterTest" />
       <class name="test.attributes.AttributeTest"/>
       <class name="test.attributes.issue2346.IssueTest"/>
       <class name="test.verify.VerifyTest"/>