From 8b5d6a87cab6c1c9c09737e6c7045bfff5807f0e Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Wed, 4 Sep 2024 12:20:01 +0530 Subject: [PATCH] Specifying dataProvider & successPercentage causes test to always pass Closes #3170 --- CHANGES.txt | 1 + .../internal/invokers/ITestInvoker.java | 3 + .../testng/internal/invokers/TestInvoker.java | 3 +- .../FailedInvocationCountTest.java | 56 ++++++++ .../invocationcount/issue1719/IssueTest.java | 124 ++++++++++++++++-- .../issue1719/TestclassSample.java | 56 ++++++-- ...entageAndInvocationCountDefinedSample.java | 18 +++ ...venWithSuccessPercentageDefinedSample.java | 18 +++ 8 files changed, 257 insertions(+), 22 deletions(-) create mode 100644 testng-core/src/test/java/test/invocationcount/issue3170/DataDrivenWithSuccessPercentageAndInvocationCountDefinedSample.java create mode 100644 testng-core/src/test/java/test/invocationcount/issue3170/DataDrivenWithSuccessPercentageDefinedSample.java diff --git a/CHANGES.txt b/CHANGES.txt index 9dcce35f1d..4a8bf02451 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ Current (7.11.0) +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) Fixed: GITHUB-3135: assertEquals on arrays - Failure message is missing information about the array index when an array element is unexpectedly null or non-null (Albert Choi) diff --git a/testng-core/src/main/java/org/testng/internal/invokers/ITestInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/ITestInvoker.java index 83cca83215..bda5b3b14c 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/ITestInvoker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/ITestInvoker.java @@ -1,6 +1,8 @@ package org.testng.internal.invokers; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import org.testng.IInvokedMethod; @@ -19,6 +21,7 @@ class FailureContext { AtomicInteger count = new AtomicInteger(0); List instances = Lists.newArrayList(); AtomicBoolean representsRetriedMethod = new AtomicBoolean(false); + final Map counter = new HashMap<>(); } List invokeTestMethods( 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 e7ef781766..1269a251af 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 @@ -557,7 +557,8 @@ private void handleInvocationResult( if (holder.status == ITestResult.FAILURE && !holder.handled) { int count = failure.count.getAndIncrement(); if (testMethod.isDataDriven()) { - count = 0; + String key = Arrays.toString(testResult.getParameters()); + count = failure.counter.computeIfAbsent(key, k -> new AtomicInteger()).incrementAndGet(); } handleException(testResult.getThrowable(), testMethod, testResult, count); } diff --git a/testng-core/src/test/java/test/invocationcount/FailedInvocationCountTest.java b/testng-core/src/test/java/test/invocationcount/FailedInvocationCountTest.java index e8dd38f0b1..53788b7244 100644 --- a/testng-core/src/test/java/test/invocationcount/FailedInvocationCountTest.java +++ b/testng-core/src/test/java/test/invocationcount/FailedInvocationCountTest.java @@ -1,10 +1,20 @@ package test.invocationcount; +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.concurrent.atomic.AtomicInteger; import org.testng.Assert; +import org.testng.IInvokedMethod; +import org.testng.IInvokedMethodListener; +import org.testng.ITestResult; import org.testng.TestListenerAdapter; import org.testng.TestNG; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import test.SimpleBaseTest; +import test.invocationcount.issue1719.IssueTest; +import test.invocationcount.issue3170.DataDrivenWithSuccessPercentageAndInvocationCountDefinedSample; +import test.invocationcount.issue3170.DataDrivenWithSuccessPercentageDefinedSample; public class FailedInvocationCountTest extends SimpleBaseTest { @@ -41,4 +51,50 @@ public void verifyAttributeShouldStop() { Assert.assertEquals(tla.getFailedTests().size(), 7); Assert.assertEquals(tla.getSkippedTests().size(), 5); } + + @Test(dataProvider = "dp") + public void ensureSuccessPercentageWorksFineWith(Class clazz, IssueTest.Expected expected) { + TestNG testng = create(clazz); + AtomicInteger failed = new AtomicInteger(0); + AtomicInteger passed = new AtomicInteger(0); + AtomicInteger failedWithInSuccessPercentage = new AtomicInteger(0); + testng.addListener( + new IInvokedMethodListener() { + @Override + public void afterInvocation(IInvokedMethod method, ITestResult testResult) { + + switch (testResult.getStatus()) { + case ITestResult.SUCCESS: + passed.incrementAndGet(); + break; + case ITestResult.FAILURE: + failed.incrementAndGet(); + break; + case ITestResult.SUCCESS_PERCENTAGE_FAILURE: + failedWithInSuccessPercentage.incrementAndGet(); + break; + default: + } + } + }); + testng.run(); + assertThat(passed.get()).isEqualTo(expected.success()); + assertThat(failed.get()).isEqualTo(expected.failures()); + assertThat(failedWithInSuccessPercentage.get()) + .isEqualTo(expected.failedWithinSuccessPercentage()); + } + + @DataProvider(name = "dp") + public Object[][] dp() { + return new Object[][] { + { + DataDrivenWithSuccessPercentageAndInvocationCountDefinedSample.class, + new IssueTest.Expected().failures(10) + }, + { + DataDrivenWithSuccessPercentageDefinedSample.class, + new IssueTest.Expected().failures(3).success(1) + } + }; + } } diff --git a/testng-core/src/test/java/test/invocationcount/issue1719/IssueTest.java b/testng-core/src/test/java/test/invocationcount/issue1719/IssueTest.java index fd2a0f6598..2a130b0094 100644 --- a/testng-core/src/test/java/test/invocationcount/issue1719/IssueTest.java +++ b/testng-core/src/test/java/test/invocationcount/issue1719/IssueTest.java @@ -1,22 +1,128 @@ package test.invocationcount.issue1719; -import static org.assertj.core.api.Assertions.assertThat; - +import java.util.Set; +import org.assertj.core.api.SoftAssertions; +import org.testng.ITestResult; import org.testng.TestNG; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import test.SimpleBaseTest; public class IssueTest extends SimpleBaseTest { - @Test - public void testSuccessPercentageCalculation() { - TestNG testng = create(TestclassSample.class); + @Test(dataProvider = "dp") + public void testSuccessPercentageCalculation(Class clazz, Expected expected) { + TestNG testng = create(clazz); DummyReporter listener = new DummyReporter(); testng.addListener(listener); testng.run(); - assertThat(listener.getFailures()).isEmpty(); - assertThat(listener.getSkip()).isEmpty(); - assertThat(listener.getSuccess()).isEmpty(); - assertThat(listener.getFailedWithinSuccessPercentage()).hasSize(5); + SoftAssertions assertions = new SoftAssertions(); + String msg = + pretty( + "[failedWithinSuccessPercentage]", + expected.failedWithinSuccessPercentage, + listener.getFailedWithinSuccessPercentage()); + assertions + .assertThat(listener.getFailedWithinSuccessPercentage()) + .withFailMessage(msg) + .hasSize(expected.failedWithinSuccessPercentage); + + msg = pretty("[skip]", expected.skip, listener.getSkip()); + assertions.assertThat(listener.getSkip()).withFailMessage(msg).hasSize(expected.skip); + + msg = pretty("[success]", expected.success, listener.getSuccess()); + assertions.assertThat(listener.getSuccess()).withFailMessage(msg).hasSize(expected.success); + + msg = pretty("[failures]", expected.failures, listener.getFailures()); + assertions.assertThat(listener.getFailures()).withFailMessage(msg).hasSize(expected.failures); + + assertions.assertAll(); + } + + private static String pretty(String prefix, int expected, Set actual) { + return prefix + " test. Expected: " + expected + ", Actual: " + actual.size(); + } + + @DataProvider(name = "dp") + public Object[][] dp() { + return new Object[][] { + { + TestclassSample.DataDrivenTestHavingZeroSuccessPercentageAndNoInvocationCount.class, + new Expected().failures(2) + }, + { + TestclassSample.DataDrivenTestHavingValidSuccessPercentageAndInvocationCount.class, + // Parameter - Invocation Count - Expected Test Result + // . 1 1 Failed Within success percentage (30% expected) + // 1 2 Passed (Remember this is a flaky test simulation) + // 2 1 Failed Within success percentage (30% expected) + // 2 2 Failed + new Expected().failures(1).failedWithinSuccessPercentage(2).success(1) + }, + { + TestclassSample.RegularTestWithZeroSuccessPercentage.class, + new Expected().failedWithinSuccessPercentage(1) + }, + { + TestclassSample.RegularTestWithZeroSuccessPercentageAndInvocationCount.class, + new Expected().failedWithinSuccessPercentage(2) + }, + }; + } + + public static class Expected { + private int failures = 0; + private int success = 0; + private int skip = 0; + private int failedWithinSuccessPercentage = 0; + + public int failures() { + return failures; + } + + public Expected failures(int failures) { + this.failures = failures; + return this; + } + + public int success() { + return success; + } + + public Expected success(int success) { + this.success = success; + return this; + } + + public int skip() { + return skip; + } + + public Expected skip(int skip) { + this.skip = skip; + return this; + } + + public int failedWithinSuccessPercentage() { + return failedWithinSuccessPercentage; + } + + public Expected failedWithinSuccessPercentage(int failedWithinSuccessPercentage) { + this.failedWithinSuccessPercentage = failedWithinSuccessPercentage; + return this; + } + + @Override + public String toString() { + return "{failures=" + + failures + + ", success=" + + success + + ", skip=" + + skip + + ", failedWithinSuccessPercentage=" + + failedWithinSuccessPercentage + + '}'; + } } } diff --git a/testng-core/src/test/java/test/invocationcount/issue1719/TestclassSample.java b/testng-core/src/test/java/test/invocationcount/issue1719/TestclassSample.java index 34820ac876..efd981b286 100644 --- a/testng-core/src/test/java/test/invocationcount/issue1719/TestclassSample.java +++ b/testng-core/src/test/java/test/invocationcount/issue1719/TestclassSample.java @@ -1,28 +1,60 @@ package test.invocationcount.issue1719; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; public class TestclassSample { - @Test(successPercentage = 0, dataProvider = "dp") - public void dataDrivenTestMethod(int i) { - Assert.fail("Failing iteration:" + i); - } - @DataProvider(name = "dp") - public Object[][] getData() { + public static Object[][] getData() { return new Object[][] {{1}, {2}}; } - @Test(successPercentage = 0) - public void simpleTestMethod() { - Assert.fail(); + public static class DataDrivenTestHavingZeroSuccessPercentageAndNoInvocationCount { + @Test(successPercentage = 0, dataProvider = "dp", dataProviderClass = TestclassSample.class) + public void dataDrivenTestMethod(int i) { + Assert.fail("Failing iteration:" + i); + } + } + + public static class DataDrivenTestHavingValidSuccessPercentageAndInvocationCount { + + private boolean shouldFail = true; + private Map tracker = new HashMap<>(); + + @Test( + successPercentage = 30, + dataProvider = "dp", + invocationCount = 2, + dataProviderClass = TestclassSample.class) + public void dataDrivenTestMethodWithInvocationCount(int i) { + int current = tracker.computeIfAbsent(i, k -> new AtomicInteger()).incrementAndGet(); + String msg = String.format("Parameter [%d], Invocation [%d]", i, current); + if (i != 1) { // If the parameter is NOT 1, then just fail + Assert.fail("Failing test " + msg); + } + if (shouldFail) { // If the parameter is 1, then simulate a flaky test that passes and fails + shouldFail = false; + Assert.fail("Failing test " + msg); + } + } + } + + public static class RegularTestWithZeroSuccessPercentage { + @Test(successPercentage = 0) + public void simpleTestMethod() { + Assert.fail(); + } } - @Test(successPercentage = 0, invocationCount = 2) - public void testMethodWithMultipleInvocations() { - Assert.fail(); + public static class RegularTestWithZeroSuccessPercentageAndInvocationCount { + @Test(successPercentage = 0, invocationCount = 2) + public void testMethodWithMultipleInvocations() { + Assert.fail(); + } } } diff --git a/testng-core/src/test/java/test/invocationcount/issue3170/DataDrivenWithSuccessPercentageAndInvocationCountDefinedSample.java b/testng-core/src/test/java/test/invocationcount/issue3170/DataDrivenWithSuccessPercentageAndInvocationCountDefinedSample.java new file mode 100644 index 0000000000..fbdce2525c --- /dev/null +++ b/testng-core/src/test/java/test/invocationcount/issue3170/DataDrivenWithSuccessPercentageAndInvocationCountDefinedSample.java @@ -0,0 +1,18 @@ +package test.invocationcount.issue3170; + +import static org.testng.Assert.assertEquals; + +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class DataDrivenWithSuccessPercentageAndInvocationCountDefinedSample { + @Test(dataProvider = "test", invocationCount = 10, successPercentage = 90) + public void sampleTestCase(String string) { + assertEquals(string, "1"); + } + + @DataProvider(name = "test") + public Object[][] testProvider() { + return new Object[][] {{"2"}}; + } +} diff --git a/testng-core/src/test/java/test/invocationcount/issue3170/DataDrivenWithSuccessPercentageDefinedSample.java b/testng-core/src/test/java/test/invocationcount/issue3170/DataDrivenWithSuccessPercentageDefinedSample.java new file mode 100644 index 0000000000..7d3e8fa39e --- /dev/null +++ b/testng-core/src/test/java/test/invocationcount/issue3170/DataDrivenWithSuccessPercentageDefinedSample.java @@ -0,0 +1,18 @@ +package test.invocationcount.issue3170; + +import static org.testng.Assert.assertEquals; + +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class DataDrivenWithSuccessPercentageDefinedSample { + @Test(dataProvider = "test", successPercentage = 99) + public void sampleTestCase(String string) { + assertEquals(string, "1"); + } + + @DataProvider(name = "test") + public Object[][] testProvider() { + return new Object[][] {{"1"}, {"2"}, {"3"}, {"4"}}; + } +}