Streamline invocation numbers in failed xml file

CHANGES.txt

@@ -1,4 +1,5 @@
 Current (7.11.0)
+Fixed: GITHUB-3180: TestNG testng-failed.xml 'invocation-numbers' values are not calculated correctly with retry and dataproviders (Krishnan Mahadevan)
 Fixed: GITHUB-3170: Specifying dataProvider and successPercentage causes test to always pass (Krishnan Mahadevan)
 Fixed: GITHUB-3028: Execution stalls when using "use-global-thread-pool" (Krishnan Mahadevan)
 Fixed: GITHUB-3122: Update JCommander to 1.83 (Antoine Dessaigne)

testng-asserts/src/test/java/org/testng/AssertTest.java

@@ -582,21 +582,21 @@ public void testAssertNotEqualsWithNull() {
 
   @Test(description = "GITHUB-3140")
   public void testAssertEqualsDeepSet() {
-    var expectedSet = new HashSet<>();
+    var expectedSet = new LinkedHashSet<>();
     expectedSet.add(new Contrived(1));
     expectedSet.add(new Contrived[] {new Contrived(1)});
-    var actualSet = new HashSet<>();
+    var actualSet = new LinkedHashSet<>();
     actualSet.add(new Contrived(1));
     actualSet.add(new Contrived[] {new Contrived(1)});
     Assert.assertEqualsDeep(actualSet, expectedSet);
   }
 
   @Test(description = "GITHUB-3140", expectedExceptions = AssertionError.class)
   public void testAssertEqualsDeepSetFail() {
-    var expectedSet = new HashSet<>();
+    var expectedSet = new LinkedHashSet<>();
     expectedSet.add(new Contrived(1));
     expectedSet.add(new Contrived[] {new Contrived(1)});
-    var actualSet = new HashSet<>();
+    var actualSet = new LinkedHashSet<>();
     actualSet.add(new Contrived(1));
     actualSet.add(new Contrived[] {new Contrived(2)});
     Assert.assertEqualsDeep(actualSet, expectedSet);

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

@@ -764,7 +764,9 @@ private ITestResult invokeMethod(
         if (null != testResult.getMethod().getFactoryMethodParamsInfo()) {
           parametersIndex = testResult.getMethod().getFactoryMethodParamsInfo().getIndex();
         }
-        arguments.getTestMethod().addFailedInvocationNumber(parametersIndex);
+        if (!willRetryMethod) {
+          arguments.getTestMethod().addFailedInvocationNumber(parametersIndex);
+        }
       }
 
       //

testng-core/src/main/java/org/testng/reporters/FailedReporter.java

@@ -8,7 +8,9 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import org.testng.IReporter;
@@ -42,6 +44,7 @@ public class FailedReporter implements IReporter {
   public static final String TESTNG_FAILED_XML = "testng-failed.xml";
 
   private XmlSuite m_xmlSuite;
+  private final Map<String, Map<String, String>> keyCache = new ConcurrentHashMap<>();
 
   public FailedReporter() {}
 
@@ -69,11 +72,12 @@ protected void generateFailureSuite(XmlSuite xmlSuite, ISuite suite, String outp
       ISuiteResult suiteResult = entry.getValue();
       ITestContext testContext = suiteResult.getTestContext();
 
-      generateXmlTest(
-          testContext.getCurrentXmlTest(),
-          testContext,
-          testContext.getFailedTests().getAllResults(),
-          testContext.getSkippedTests().getAllResults());
+      boolean shouldWriteIntoFile = generateXmlTest(testContext);
+      XmlTest current = testContext.getCurrentXmlTest();
+      failedSuite
+          .getTests()
+          .removeIf(it -> !shouldWriteIntoFile && it.getName().equals(current.getName()));
+      clearKeyCache(testContext);
     }
 
     if (null != failedSuite.getTests() && !failedSuite.getTests().isEmpty()) {
@@ -88,71 +92,108 @@ protected void generateFailureSuite(XmlSuite xmlSuite, ISuite suite, String outp
     }
   }
 
-  private void generateXmlTest(
-      XmlTest xmlTest,
-      ITestContext context,
-      Set<ITestResult> failedTests,
-      Set<ITestResult> skippedTests) {
+  private void clearKeyCache(ITestContext ctx) {
+    keyCache.remove(ctx.getName());
+  }
+
+  private static String key(ITestResult it) {
+    String prefix = it.getMethod().getQualifiedName() + it.getInstance().toString();
+    if (it.getParameters().length != 0) {
+      return prefix + Arrays.toString(it.getParameters());
+    }
+    return prefix + it.getMethod().getCurrentInvocationCount();
+  }
+
+  private static Map<String, String> buildMap(Set<ITestResult> passed) {
+    return passed
+        .parallelStream()
+        .map(FailedReporter::key)
+        .collect(
+            Collectors.toUnmodifiableMap(Function.identity(), Function.identity(), (s1, s2) -> s1));
+  }
+
+  private boolean isFlakyTest(Set<ITestResult> passed, ITestResult result) {
+    String ctxKey = result.getTestContext().getName();
+    String individualKey = key(result);
+    return keyCache.computeIfAbsent(ctxKey, k -> buildMap(passed)).containsKey(individualKey);
+  }
+
+  private boolean generateXmlTest(ITestContext context) {
+    XmlTest xmlTest = context.getCurrentXmlTest();
+    Set<ITestResult> failedTests = context.getFailedTests().getAllResults();
+    Set<ITestResult> skippedTests = context.getSkippedTests().getAllResults();
     // Note:  we can have skipped tests and no failed tests
     // if a method depends on nonexistent groups
-    if (!skippedTests.isEmpty() || !failedTests.isEmpty()) {
-      Set<ITestNGMethod> methodsToReRun = Sets.newHashSet();
-
-      // Get the transitive closure of all the failed methods and the methods
-      // they depend on
-      Set<ITestResult> allTests = Sets.newHashSet();
-      allTests.addAll(failedTests);
-      allTests.addAll(skippedTests);
-      ITestNGMethod[] allTestMethods = context.getAllTestMethods();
-      for (ITestResult failedTest : allTests) {
-        ITestNGMethod current = failedTest.getMethod();
-        if (!current.isTest()) { // Don't count configuration methods
-          continue;
-        }
-        methodsToReRun.add(current);
-        List<ITestNGMethod> methodsDependedUpon =
-            MethodHelper.getMethodsDependedUpon(current, allTestMethods, MethodSorting.basedOn());
+    if (skippedTests.isEmpty() && failedTests.isEmpty()) {
+      return false;
+    }
+    Set<ITestNGMethod> methodsToReRun = Sets.newHashSet();
+    Set<ITestResult> passedTests = context.getPassedTests().getAllResults();
+
+    // Get the transitive closure of all the failed methods and the methods
+    // they depend on
+    Set<ITestResult> allTests = Sets.newHashSet();
+    allTests.addAll(failedTests);
+    allTests.addAll(skippedTests);
+    ITestNGMethod[] allTestMethods = context.getAllTestMethods();
+    for (ITestResult failedTest : allTests) {
+      ITestNGMethod current = failedTest.getMethod();
+      if (!current.isTest()) { // Don't count configuration methods
+        continue;
+      }
+      boolean repetitiveTest = failedTest.getMethod().getInvocationCount() > 0;
+      boolean isDataDriven = failedTest.getMethod().isDataDriven();
+      if ((repetitiveTest || isDataDriven) && isFlakyTest(passedTests, failedTest)) {
+        continue;
+      }
+      methodsToReRun.add(current);
+      List<ITestNGMethod> methodsDependedUpon =
+          MethodHelper.getMethodsDependedUpon(current, allTestMethods, MethodSorting.basedOn());
 
-        for (ITestNGMethod m : methodsDependedUpon) {
-          if (m.isTest()) {
-            methodsToReRun.add(m);
-          }
+      for (ITestNGMethod m : methodsDependedUpon) {
+        if (m.isTest()) {
+          methodsToReRun.add(m);
         }
       }
+    }
 
-      //
-      // Now we have all the right methods.  Go through the list of
-      // all the methods that were run and only pick those that are
-      // in the methodToReRun map.  Since the methods are already
-      // sorted, we don't need to sort them again.
-      //
-      List<ITestNGMethod> result = Lists.newArrayList();
-      Set<ITestNGMethod> relevantConfigs = Sets.newHashSet();
-      for (ITestNGMethod m : allTestMethods) {
-        if (RuntimeBehavior.isMemoryFriendlyMode()) {
-          // We are doing this because the `m` would not be of type
-          // LiteWeightTestNGMethod and hence the hashCode() and equals()
-          // computation would be different.
-          m = new LiteWeightTestNGMethod(m);
-        }
-        if (methodsToReRun.contains(m)) {
-          result.add(m);
-          getAllApplicableConfigs(relevantConfigs, m.getTestClass());
-          getAllGroupApplicableConfigs(context, relevantConfigs, m);
-        }
+    //
+    // Now we have all the right methods.  Go through the list of
+    // all the methods that were run and only pick those that are
+    // in the methodToReRun map.  Since the methods are already
+    // sorted, we don't need to sort them again.
+    //
+    List<ITestNGMethod> result = Lists.newArrayList();
+    Set<ITestNGMethod> relevantConfigs = Sets.newHashSet();
+    for (ITestNGMethod m : allTestMethods) {
+      if (RuntimeBehavior.isMemoryFriendlyMode()) {
+        // We are doing this because the `m` would not be of type
+        // LiteWeightTestNGMethod and hence the hashCode() and equals()
+        // computation would be different.
+        m = new LiteWeightTestNGMethod(m);
+      }
+      if (methodsToReRun.contains(m)) {
+        result.add(m);
+        getAllApplicableConfigs(relevantConfigs, m.getTestClass());
+        getAllGroupApplicableConfigs(context, relevantConfigs, m);
       }
+    }
 
-      Set<ITestNGMethod> upstreamConfigFailures =
-          Stream.of(
-                  context.getFailedConfigurations().getAllMethods(),
-                  context.getSkippedConfigurations().getAllMethods())
-              .flatMap(Collection::stream)
-              .filter(FailedReporter::isNotClassLevelConfigurationMethod)
-              .collect(Collectors.toSet());
-      result.addAll(upstreamConfigFailures);
-      result.addAll(relevantConfigs);
-      createXmlTest(context, result, xmlTest);
+    if (methodsToReRun.isEmpty()) {
+      return false;
     }
+
+    Set<ITestNGMethod> upstreamConfigFailures =
+        Stream.of(
+                context.getFailedConfigurations().getAllMethods(),
+                context.getSkippedConfigurations().getAllMethods())
+            .flatMap(Collection::stream)
+            .filter(FailedReporter::isNotClassLevelConfigurationMethod)
+            .collect(Collectors.toSet());
+    result.addAll(upstreamConfigFailures);
+    result.addAll(relevantConfigs);
+    createXmlTest(context, result, xmlTest);
+    return true;
   }
 
   private static boolean isNotClassLevelConfigurationMethod(ITestNGMethod each) {