From 87c1f96a8e81fde6c4aa0295bad25c32ea0e4224 Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Tue, 1 Nov 2022 10:33:29 +0530 Subject: [PATCH] Unexpected test runs count with retry analyzer Closes #2798 --- CHANGES.txt | 1 + .../org/testng/internal/BaseTestMethod.java | 23 +++++++------------ .../test/retryAnalyzer/RetryAnalyzerTest.java | 18 ++++++++++++--- .../issue2798/HashCodeAwareRetryAnalyzer.java | 20 ++++++++++++++++ .../issue2798/TestClassSample.java | 21 +++++++++++++++++ 5 files changed, 65 insertions(+), 18 deletions(-) create mode 100644 testng-core/src/test/java/test/retryAnalyzer/issue2798/HashCodeAwareRetryAnalyzer.java create mode 100644 testng-core/src/test/java/test/retryAnalyzer/issue2798/TestClassSample.java diff --git a/CHANGES.txt b/CHANGES.txt index 9e8f972c63..7fb85330c7 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -5,6 +5,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) diff --git a/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java b/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java index 7c984e51bc..c0cddaecf8 100644 --- a/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java +++ b/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) { diff --git a/testng-core/src/test/java/test/retryAnalyzer/RetryAnalyzerTest.java b/testng-core/src/test/java/test/retryAnalyzer/RetryAnalyzerTest.java index d544e77946..6d7074dce5 100644 --- a/testng-core/src/test/java/test/retryAnalyzer/RetryAnalyzerTest.java +++ b/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 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"); } diff --git a/testng-core/src/test/java/test/retryAnalyzer/issue2798/HashCodeAwareRetryAnalyzer.java b/testng-core/src/test/java/test/retryAnalyzer/issue2798/HashCodeAwareRetryAnalyzer.java new file mode 100644 index 0000000000..60a2f41249 --- /dev/null +++ b/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 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; + } +} diff --git a/testng-core/src/test/java/test/retryAnalyzer/issue2798/TestClassSample.java b/testng-core/src/test/java/test/retryAnalyzer/issue2798/TestClassSample.java new file mode 100644 index 0000000000..2179138ed8 --- /dev/null +++ b/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 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"); + } +}