diff --git a/CHANGES.txt b/CHANGES.txt index 4a8bf0245..99f86edc3 100644 --- a/CHANGES.txt +++ b/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) diff --git a/testng-asserts/src/test/java/org/testng/AssertTest.java b/testng-asserts/src/test/java/org/testng/AssertTest.java index ba97f0c67..4b626c3a0 100644 --- a/testng-asserts/src/test/java/org/testng/AssertTest.java +++ b/testng-asserts/src/test/java/org/testng/AssertTest.java @@ -582,10 +582,10 @@ 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); @@ -593,10 +593,10 @@ public void testAssertEqualsDeepSet() { @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); diff --git a/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java index 1269a251a..b48d458ef 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java +++ b/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); + } } // diff --git a/testng-core/src/main/java/org/testng/reporters/FailedReporter.java b/testng-core/src/main/java/org/testng/reporters/FailedReporter.java index 068af3808..4ed3e1a2e 100644 --- a/testng-core/src/main/java/org/testng/reporters/FailedReporter.java +++ b/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> keyCache = new ConcurrentHashMap<>(); public FailedReporter() {} @@ -65,15 +68,14 @@ protected void generateFailureSuite(XmlSuite xmlSuite, ISuite suite, String outp Map results = suite.getResults(); + boolean shouldWriteIntoFile = true; + for (Map.Entry entry : results.entrySet()) { ISuiteResult suiteResult = entry.getValue(); ITestContext testContext = suiteResult.getTestContext(); - generateXmlTest( - testContext.getCurrentXmlTest(), - testContext, - testContext.getFailedTests().getAllResults(), - testContext.getSkippedTests().getAllResults()); + shouldWriteIntoFile = shouldWriteIntoFile && generateXmlTest(testContext); + clearKeyCache(testContext); } if (null != failedSuite.getTests() && !failedSuite.getTests().isEmpty()) { @@ -83,20 +85,48 @@ protected void generateFailureSuite(XmlSuite xmlSuite, ISuite suite, String outp Lists.merge(failedSuite.getListeners(), xmlSuite.getParentSuite().getListeners()); failedSuite.setListeners(merged); } - Utils.writeUtf8File(outputDir, TESTNG_FAILED_XML, failedSuite.toXml()); - Utils.writeUtf8File(suite.getOutputDirectory(), TESTNG_FAILED_XML, failedSuite.toXml()); + if (shouldWriteIntoFile) { + Utils.writeUtf8File(outputDir, TESTNG_FAILED_XML, failedSuite.toXml()); + Utils.writeUtf8File(suite.getOutputDirectory(), TESTNG_FAILED_XML, failedSuite.toXml()); + } + } + } + + 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 void generateXmlTest( - XmlTest xmlTest, - ITestContext context, - Set failedTests, - Set skippedTests) { + private static Map buildMap(Set passed) { + return passed + .parallelStream() + .map(FailedReporter::key) + .collect( + Collectors.toUnmodifiableMap(Function.identity(), Function.identity(), (s1, s2) -> s1)); + } + + private boolean isFlakyTest(Set 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 failedTests = context.getFailedTests().getAllResults(); + Set 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 methodsToReRun = Sets.newHashSet(); + Set passedTests = context.getPassedTests().getAllResults(); // Get the transitive closure of all the failed methods and the methods // they depend on @@ -109,6 +139,11 @@ private void generateXmlTest( 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 methodsDependedUpon = MethodHelper.getMethodsDependedUpon(current, allTestMethods, MethodSorting.basedOn()); @@ -142,6 +177,10 @@ private void generateXmlTest( } } + if (methodsToReRun.isEmpty()) { + return false; + } + Set upstreamConfigFailures = Stream.of( context.getFailedConfigurations().getAllMethods(), @@ -152,7 +191,9 @@ private void generateXmlTest( result.addAll(upstreamConfigFailures); result.addAll(relevantConfigs); createXmlTest(context, result, xmlTest); + return true; } + return false; } private static boolean isNotClassLevelConfigurationMethod(ITestNGMethod each) { diff --git a/testng-core/src/test/java/test/invocationcount/FailedInvocationCountTest.java b/testng-core/src/test/java/test/invocationcount/FailedInvocationCountTest.java index 53788b724..510f04f3f 100644 --- a/testng-core/src/test/java/test/invocationcount/FailedInvocationCountTest.java +++ b/testng-core/src/test/java/test/invocationcount/FailedInvocationCountTest.java @@ -2,7 +2,17 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.io.StringReader; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.concurrent.atomic.AtomicInteger; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.xpath.XPath; +import javax.xml.xpath.XPathConstants; +import javax.xml.xpath.XPathExpression; +import javax.xml.xpath.XPathFactory; +import org.assertj.core.api.SoftAssertions; import org.testng.Assert; import org.testng.IInvokedMethod; import org.testng.IInvokedMethodListener; @@ -11,10 +21,17 @@ import org.testng.TestNG; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; +import org.testng.reporters.FailedReporter; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; +import org.xml.sax.InputSource; import test.SimpleBaseTest; import test.invocationcount.issue1719.IssueTest; import test.invocationcount.issue3170.DataDrivenWithSuccessPercentageAndInvocationCountDefinedSample; import test.invocationcount.issue3170.DataDrivenWithSuccessPercentageDefinedSample; +import test.invocationcount.issue3180.SampleTestContainer; public class FailedInvocationCountTest extends SimpleBaseTest { @@ -97,4 +114,74 @@ public Object[][] dp() { } }; } + + @DataProvider(name = "github-3180") + public Object[][] getTestData() { + String[] nothing = new String[] {""}; + boolean noXml = false; + boolean xmlSeen = true; + return new Object[][] { + // Test has flaky iterations which pass eventually. So no failed xml should be seen. + {SampleTestContainer.TestContainsFlakyDataDrivenTest.class, noXml, nothing}, + // Repetitive test that eventually passes. So no failed xml should be seen. + {SampleTestContainer.TestContainsPercentageDrivenTest.class, noXml, nothing}, + // Not a test that repeats. So no invocation count attribute should be seen + {SampleTestContainer.TestWithNormalFailingTest.class, xmlSeen, nothing}, + // Flaky test. So ensure only flaky invocations are referenced + {SampleTestContainer.TestWithSomeFailingIterations.class, xmlSeen, new String[] {"0 2"}}, + // Flaky test. So ensure only flaky invocations are referenced + { + SampleTestContainer.TestContainsAlwaysFailingDataDrivenTest.class, + xmlSeen, + new String[] {"0"} + }, + // This is a combination of all the earlier permutations. + // So we should see an xml that contains only the true cases from earlier + { + SampleTestContainer.TestContainsAllCombinations.class, + xmlSeen, + new String[] {"", "0 2", "0"} + } + }; + } + + @Test(description = "GITHUB-3180", dataProvider = "github-3180") + public void ensureInvocationCountHonoursRetriesWhenUsingDataProviders( + Class cls, boolean isXmlGenerated, String[] invocationCountValue) throws Exception { + String reportsDir = createDirInTempDir("3180").getAbsolutePath(); + TestNG testng = create(Paths.get(reportsDir), cls); + testng.setUseDefaultListeners(false); + testng.addListener(new FailedReporter()); + testng.run(); + Path xml = Paths.get(reportsDir, "testng-failed.xml"); + assertThat(xml.toFile().exists()).isEqualTo(isXmlGenerated); + if (!isXmlGenerated) { + // Do not validate anything if the xml is NOT generated + return; + } + String xmlContent = Files.readString(xml); + Document document = document(xmlContent); + XPathFactory xPathFactory = XPathFactory.newInstance(); + XPath xPath = xPathFactory.newXPath(); + XPathExpression xPathExpression = xPath.compile("//methods/include"); + + NodeList nodeList = (NodeList) xPathExpression.evaluate(document, XPathConstants.NODESET); + assertThat(nodeList.getLength()).isEqualTo(invocationCountValue.length); + SoftAssertions softly = new SoftAssertions(); + for (int i = 0; i < nodeList.getLength(); i++) { + Node node = nodeList.item(i); + assertThat(node).isNotNull(); + assertThat(node.getNodeType()).isEqualTo(Node.ELEMENT_NODE); + Element element = (Element) node; + String value = element.getAttribute("invocation-numbers"); + softly.assertThat(value).isEqualTo(invocationCountValue[i]); + } + softly.assertAll(); + } + + private static Document document(String xmlContent) throws Exception { + return DocumentBuilderFactory.newInstance() + .newDocumentBuilder() + .parse(new InputSource(new StringReader(xmlContent))); + } } diff --git a/testng-core/src/test/java/test/invocationcount/issue3180/RetryAnalyzer.java b/testng-core/src/test/java/test/invocationcount/issue3180/RetryAnalyzer.java new file mode 100644 index 000000000..8feedd5e6 --- /dev/null +++ b/testng-core/src/test/java/test/invocationcount/issue3180/RetryAnalyzer.java @@ -0,0 +1,17 @@ +package test.invocationcount.issue3180; + +import org.testng.IRetryAnalyzer; +import org.testng.ITestResult; + +public class RetryAnalyzer implements IRetryAnalyzer { + + int counter = 0; + int retryLimit = 3; + + /* + * Retry method + */ + public boolean retry(ITestResult result) { + return counter++ < retryLimit; + } +} diff --git a/testng-core/src/test/java/test/invocationcount/issue3180/SampleTestCase.java b/testng-core/src/test/java/test/invocationcount/issue3180/SampleTestCase.java new file mode 100644 index 000000000..9a62565ef --- /dev/null +++ b/testng-core/src/test/java/test/invocationcount/issue3180/SampleTestCase.java @@ -0,0 +1,23 @@ +package test.invocationcount.issue3180; + +import static org.testng.Assert.assertEquals; + +import java.util.concurrent.atomic.AtomicInteger; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class SampleTestCase { + + @Test(dataProvider = "dp", retryAnalyzer = RetryAnalyzer.class) + public void testDataProvider(AtomicInteger counter) { + assertEquals(counter.incrementAndGet(), 3); + } + + @Test + public void normalTestMethod() {} + + @DataProvider(name = "dp") + public Object[][] createTestData() { + return new Object[][] {{new AtomicInteger(0)}}; + } +} diff --git a/testng-core/src/test/java/test/invocationcount/issue3180/SampleTestContainer.java b/testng-core/src/test/java/test/invocationcount/issue3180/SampleTestContainer.java new file mode 100644 index 000000000..92040dd8c --- /dev/null +++ b/testng-core/src/test/java/test/invocationcount/issue3180/SampleTestContainer.java @@ -0,0 +1,106 @@ +package test.invocationcount.issue3180; + +import static org.testng.Assert.assertEquals; + +import java.util.concurrent.atomic.AtomicInteger; +import org.testng.Assert; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class SampleTestContainer { + @DataProvider + public static Object[][] atomicNumbers() { + return new Object[][] {{new AtomicInteger(0)}}; + } + + @DataProvider + public static Object[][] numbers() { + return new Object[][] {{1}, {2}, {3}}; + } + + public static class TestContainsFlakyDataDrivenTest { + + @Test( + dataProvider = "atomicNumbers", + dataProviderClass = SampleTestContainer.class, + retryAnalyzer = RetryAnalyzer.class) + public void testDataProvider(AtomicInteger counter) { + assertEquals(counter.incrementAndGet(), 3); + } + } + + public static class TestWithNormalFailingTest { + @Test(retryAnalyzer = RetryAnalyzer.class) + public void alwaysFailingRegularTest() { + Assert.fail(); + } + } + + public static class TestWithSomeFailingIterations { + + @Test( + dataProvider = "numbers", + dataProviderClass = SampleTestContainer.class, + retryAnalyzer = RetryAnalyzer.class) + public void failsForOddNumbersOnly(int i) { + assertEquals(i % 2, 0); + } + } + + public static class TestContainsAlwaysFailingDataDrivenTest { + @Test( + dataProvider = "atomicNumbers", + dataProviderClass = SampleTestContainer.class, + retryAnalyzer = RetryAnalyzer.class) + public void alwaysFailingDataDrivenTest(AtomicInteger ignored) { + Assert.fail(); + } + } + + public static class TestContainsPercentageDrivenTest { + private final AtomicInteger switcher = new AtomicInteger(1); + + @Test(invocationCount = 10, successPercentage = 90, retryAnalyzer = RetryAnalyzer.class) + public void invocationCountTestWhichEventuallyPassesDueToSuccessFactors() { + Assert.assertTrue(switcher.getAndIncrement() < 9); + } + } + + public static class TestContainsAllCombinations { + private final AtomicInteger switcher = new AtomicInteger(1); + + @Test( + dataProvider = "atomicNumbers", + dataProviderClass = SampleTestContainer.class, + retryAnalyzer = RetryAnalyzer.class) + public void testDataProvider(AtomicInteger counter) { + assertEquals(counter.incrementAndGet(), 3); + } + + @Test(retryAnalyzer = RetryAnalyzer.class) + public void alwaysFailingRegularTest() { + Assert.fail(); + } + + @Test( + dataProvider = "numbers", + dataProviderClass = SampleTestContainer.class, + retryAnalyzer = RetryAnalyzer.class) + public void failsForOddNumbersOnly(int i) { + assertEquals(i % 2, 0); + } + + @Test( + dataProvider = "atomicNumbers", + dataProviderClass = SampleTestContainer.class, + retryAnalyzer = RetryAnalyzer.class) + public void alwaysFailingDataDrivenTest(AtomicInteger ignored) { + Assert.fail(); + } + + @Test(invocationCount = 10, successPercentage = 90) + public void invocationCountTestWhichEventuallyPassesDueToSuccessFactors() { + Assert.assertTrue(switcher.getAndIncrement() < 9); + } + } +}