Unexpected test runs count with retry analyzer

Closes #2798

Author: krmahadevan

CHANGES.txt

@@ -4,6 +4,7 @@ New: Ability to provide custom error message for assertThrows\expectThrows metho
 Fixed: GITHUB-2780: Use SpotBugs instead of abandoned FindBugs
 Fixed: GITHUB-2801: JUnitReportReporter is too slow
 Fixed: GITHUB-2807: buildStackTrace should be fail-safe
+Fixed: GITHUB-2798: Parallel executions coupled with retry analyzer results in duplicate retry analyzer instances being created (Krishnan Mahadevan)
 
 7.6.1
 Fixed: GITHUB-2761: Exception: ERROR java.nio.file.NoSuchFileException: /tmp/testngXmlPathInJar-15086412835569336174 (Krishnan Mahadevan)

testng-core/src/main/java/org/testng/internal/BaseTestMethod.java

@@ -58,7 +58,7 @@ public abstract class BaseTestMethod implements ITestNGMethod, IInvocationStatus
   private boolean m_enabled;
 
   private final String m_methodName;
-  // If a depended group is not found
+  // If a depends on group is not found
   private String m_missingGroup;
   private String m_description = null;
   protected AtomicInteger m_currentInvocationCount = new AtomicInteger(0);
@@ -449,10 +449,6 @@ protected IAnnotationFinder getAnnotationFinder() {
     return m_annotationFinder;
   }
 
-  protected IClass getIClass() {
-    return m_testClass;
-  }
-
   static StringBuilder stringify(String cls, ConstructorOrMethod method) {
     StringBuilder result = new StringBuilder(cls).append(".").append(method.getName()).append("(");
     return result.append(method.stringifyParameterTypes()).append(")");
@@ -788,16 +784,13 @@ private IRetryAnalyzer getRetryAnalyzerConsideringMethodParameters(ITestResult t
       return this.m_retryAnalyzer;
     }
     final String keyAsString = getSimpleName() + "#" + getParameterInvocationCount();
-    final IRetryAnalyzer currentRetryAnalyzerInMap = m_testMethodToRetryAnalyzer.get(keyAsString);
-    if (currentRetryAnalyzerInMap != null) {
-      return currentRetryAnalyzerInMap;
-    }
-    BasicAttributes ba = new BasicAttributes(null, this.m_retryAnalyzerClass);
-    CreationAttributes attributes = new CreationAttributes(tr.getTestContext(), ba, null);
-    IRetryAnalyzer instance =
-        (IRetryAnalyzer) Dispenser.newInstance(m_objectFactory).dispense(attributes);
-    m_testMethodToRetryAnalyzer.put(keyAsString, instance);
-    return instance;
+    return m_testMethodToRetryAnalyzer.computeIfAbsent(
+        keyAsString,
+        key -> {
+          BasicAttributes ba = new BasicAttributes(null, this.m_retryAnalyzerClass);
+          CreationAttributes attributes = new CreationAttributes(tr.getTestContext(), ba, null);
+          return (IRetryAnalyzer) Dispenser.newInstance(m_objectFactory).dispense(attributes);
+        });
   }
 
   private static boolean isNotParameterisedTest(ITestResult tr) {

testng-core/src/test/java/test/retryAnalyzer/RetryAnalyzerTest.java

@@ -8,6 +8,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 import org.testng.Assert;
 import org.testng.ITestResult;
@@ -22,7 +23,6 @@
 import test.SimpleBaseTest;
 import test.retryAnalyzer.dataprovider.issue2163.TestClassPoweredByDataProviderSample;
 import test.retryAnalyzer.github1519.MyListener;
-import test.retryAnalyzer.github1519.TestClassSample;
 import test.retryAnalyzer.github1600.Github1600Listener;
 import test.retryAnalyzer.github1600.Github1600TestSample;
 import test.retryAnalyzer.github1706.DataDrivenSample;
@@ -38,8 +38,20 @@
 import test.retryAnalyzer.issue1946.TestclassSample1;
 import test.retryAnalyzer.issue1946.TestclassSample2;
 import test.retryAnalyzer.issue2684.SampleTestClassWithGroupConfigs;
+import test.retryAnalyzer.issue2798.HashCodeAwareRetryAnalyzer;
+import test.retryAnalyzer.issue2798.TestClassSample;
 
 public class RetryAnalyzerTest extends SimpleBaseTest {
+
+  @Test(description = "GITHUB-2798")
+  public void ensureNoDuplicateRetryAnalyzerInstancesAreCreated() {
+    create(TestClassSample.class).run();
+    Map<Integer, Long> collected =
+        HashCodeAwareRetryAnalyzer.hashCodes.stream()
+            .collect(Collectors.groupingBy(Function.identity(), Collectors.counting()));
+    assertThat(collected.keySet()).hasSize(1);
+  }
+
   @Test
   public void testInvocationCounts() {
     TestNG tng = create(InvocationCountTest.class);
@@ -61,10 +73,10 @@ public void testInvocationCounts() {
 
   @Test
   public void testIfRetryIsInvokedBeforeListener() {
-    TestNG tng = create(TestClassSample.class);
+    TestNG tng = create(test.retryAnalyzer.github1519.TestClassSample.class);
     tng.addListener(new MyListener());
     tng.run();
-    assertThat(TestClassSample.messages)
+    assertThat(test.retryAnalyzer.github1519.TestClassSample.messages)
         .containsExactly("afterInvocation", "retry", "afterInvocation");
   }
 

testng-core/src/test/java/test/retryAnalyzer/issue2798/HashCodeAwareRetryAnalyzer.java

@@ -0,0 +1,20 @@
+package test.retryAnalyzer.issue2798;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.testng.IRetryAnalyzer;
+import org.testng.ITestResult;
+
+public class HashCodeAwareRetryAnalyzer implements IRetryAnalyzer {
+
+  public static final List<Integer> hashCodes = new ArrayList<>();
+
+  int cnt = 0;
+  static final int threshold = 2;
+
+  @Override
+  public synchronized boolean retry(ITestResult result) {
+    hashCodes.add(this.hashCode());
+    return cnt++ < threshold;
+  }
+}

testng-core/src/test/java/test/retryAnalyzer/issue2798/TestClassSample.java

@@ -0,0 +1,21 @@
+package test.retryAnalyzer.issue2798;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Iterator;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+public class TestClassSample {
+
+  @DataProvider(name = "test data", parallel = true)
+  public Iterator<Object[]> data() {
+    return Arrays.asList(new Object[] {"1"}, new Object[] {"2"}).iterator();
+  }
+
+  @Test(dataProvider = "test data", retryAnalyzer = HashCodeAwareRetryAnalyzer.class)
+  public void foo(String ignored) throws IOException {
+    throw new FileNotFoundException("this can be retried");
+  }
+}