From 8b92c32ff3882e4bf3a6b7cc0cb0235a5d5aa079 Mon Sep 17 00:00:00 2001 From: orange-buffalo Date: Thu, 30 Jan 2020 11:26:02 +1100 Subject: [PATCH] #217: removing conditional logic for parameters resolution handling and always marking the test as failed in case of any exception --- CHANGES.txt | 39 ++++++++++--------- .../java/org/testng/internal/TestInvoker.java | 13 ++----- .../DataProviderWithErrorSample.java | 4 +- .../dataprovider/FailingDataProviderTest.java | 18 ++++----- .../java/test/retryAnalyzer/ExitCodeTest.java | 17 ++++---- 5 files changed, 42 insertions(+), 49 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 02b31688a6..5243d7bf65 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -11,6 +11,7 @@ Fixed: GITHUB-1632: throwing SkipException sets iTestResult status to Failure in Fixed: GITHUB-2207: Allow dependency injector factory to be configured via args (dmikhievich) Fixed: GITHUB-2193: Ignore local url for DTD security check (Li.Zhao & Julien Herr) Fixed: GITHUB-1968: Upgrade to Gradle 6.0 +Fixed: GITHUB-217: Exception in DataProvider doesn't fail test run (Bogdan Ilchyshyn) 7.1.0 New : GITHUB-2199: Allow users to provide their own Injector for Dependency Injection (Krishnan Mahadevan) @@ -498,9 +499,9 @@ Added: Allow injection of java.lang.reflect.Constructor and org.testng.ITestNGMe Fixed: Assertions in the Assertions class were not failing properly. Fixed: GITHUB-337: ConfigurationMethod#m_instance set to Boolean.FALSE due to incorrect constructor call in clone() + auto-boxing (davidely) Fixed: Fix NPE for dependency methods/groups (Krishnan Mahadevan) -Fixed: preserve-order bug (found by VladSarrokhin). +Fixed: preserve-order bug (found by VladSarrokhin). Fixed: GITHUB-300: OutOfMemoryException from reporters when there are a lot of tests -Fixed: GITHUB-137: Main parameters with a default value should be overridden if a main parameter is specified +Fixed: GITHUB-137: Main parameters with a default value should be overridden if a main parameter is specified Fixed: GITHUB-107: Allow enum values without converting them to uppercase. Fixed: @Guice with no modules specified is now supported Fixed: Reporter.log() invoked from listeners were being discarded @@ -517,7 +518,7 @@ Added: Big performance improvement when generating the reports (Frank Pavageau) Added: allows you to specify group dependencies in testng.xml Added: Blow up early if trying to include/exclude an unknown method Added: can now be specified under (Storm Qi) -Added: GITHUB-243: Add Reporter Output per Test in XMLReporter (dunse) +Added: GITHUB-243: Add Reporter Output per Test in XMLReporter (dunse) Fixed: Better HTML escaping of the stack traces Fixed: The failed assertions now use [] as delimiters instead of <> (better for the HTML reports) Fixed: GITHUB-237: Wrong time format in XML reporter @@ -702,7 +703,7 @@ Eclipse: Added: New quick fix "Add static import org.testng.AssertJUnit.assertXXX" Added: New workspace wide setting: excluded stack traces, to provide shorter stack traces in the view Added: New "Clear results" icon in the tool bar -Added: When the search filter is modified, don't update the tree live if it is too big +Added: When the search filter is modified, don't update the tree live if it is too big Added: Two new @Test refactorings (pull to class level, push to method level) Added: JUnit conversion: @Ignore Added: JUnit conversion: assertArrayEquals() @@ -846,10 +847,10 @@ Added: -testnames (command line) and testnames (ant) Added: New ant task tag: propertyset (Todd Wells) Added: ITestNGListenerFactory Added: Passing command line properties via the ant task and doc update (Todd Wells) -Added: Hierarchical XmlSuites (Nalin Makar) +Added: Hierarchical XmlSuites (Nalin Makar) Added: Reporter#clear() Fixed: NullPointerException when a suite produces no results (Cefn Hoile) -Fixed: Identical configuration methods were not always invoked in the correct order in superclasses (Nalin Makar) +Fixed: Identical configuration methods were not always invoked in the correct order in superclasses (Nalin Makar) Fixed: @DataProvider(parallel = true) was passing incorrect parameters with injection Fixed: Replaced @Test(sequential) with @Test(singleThreaded) Fixed: If inherited configuration methods had defined deps, they could be invoked in incorrect order (Nalin Makar) @@ -866,9 +867,9 @@ Fixed: Issue78 NPE with non-public class. Now throws TestNG exception Fixed: NPE with @Optional null parameters (Yves Dessertine) Fixed: TESTNG-387 TestNG not rerunning test method with the right data set from Data Provider (Francois Reynaud) Fixed: Show correct number of pass/failed numbers for tests using @DataProvider -Fixed: Return correct method status and exception (if any) in InvokedMethodListener.afterInvocation() -Fixed: Trivial fixes: TESTNG-241 (log message at Info), Issue2 (throw SAXException and not NPE for invalid testng xml) -Fixed: Configuration methods couldn't depend on an abstract method (Nalin Makar) +Fixed: Return correct method status and exception (if any) in InvokedMethodListener.afterInvocation() +Fixed: Trivial fixes: TESTNG-241 (log message at Info), Issue2 (throw SAXException and not NPE for invalid testng xml) +Fixed: Configuration methods couldn't depend on an abstract method (Nalin Makar) Fixed: TestNG#setTestClasses was not resetting m_suites Fixed: Exceptions thrown by IInvokedMethodListeners were not caught (Nalin Makar) Fixed: @Listeners now works on base classes as well @@ -983,8 +984,8 @@ Fixed: Quick fixes no longer introduce deprecated annotations (Greg Turnquist) 5.9 2009/04/09 -Added: New ant task boolean flag: delegateCommandSystemProperties (Justin) -Added: skipfailedinvocations under in testng-1.0.dtd (Gael Marziou / Stevo Slavic) +Added: New ant task boolean flag: delegateCommandSystemProperties (Justin) +Added: skipfailedinvocations under in testng-1.0.dtd (Gael Marziou / Stevo Slavic) Added: -testrunfactory on the command line and in the ant task (Vitalyi Pamajonkov) Added: TESTNG-298: parallel="classes", which allows entire classes to be run in the same thread Added: @BeforeMethod can now declare Object[] as a parameter, which will be filled by the parameters of the test method @@ -1077,7 +1078,7 @@ Added: ISuite now gives access to the current XmlSuite Fixed: TESTNG-139 dependsOnMethods gets confused when dependency is "protected" Fixed: TESTNG-141 junit attribute set to false in testng-failed.xml when it should be true Fixed: TESTNG-142 Exceptions in DataProvider are not reported as failed test -Added: Improved behavior for @Before/@AfterClass when using @Factory +Added: Improved behavior for @Before/@AfterClass when using @Factory (https://forums.opensymphony.com/thread.jspa?threadID=6594&messageID=122294#122294) Added: Support for concurrent execution for invocationCount=1 threadPoolSize>1 and @DataProvider (https://forums.opensymphony.com/thread.jspa?threadID=64738&tstart=0) @@ -1113,7 +1114,7 @@ Added: Method selectors receive a Context and can stop the chain with setStopped Fixed: XmlMethodSelector was always run first regardless of its priority Added: @BeforeGroups/@AfterGroups can live in classes without @Test methods Added: DataProvider can now take an ITestContext parameter -Fixed: Wasn't parsing correctly +Fixed: Wasn't parsing correctly Fixed: Annotation Transformers now work on class-level annotations Fixed: Some class-level @Test attributes were not always honored Added: Clean separation between @Test invocation events and @Configuration invocation events @@ -1150,7 +1151,7 @@ Eclipse plug-in Fixed: groups with multi-attribute javadoc annotations Fixed: consistent behavior for dependsOnMethods Fixed: consistent behavior for tests with dependsOnGroups (a warning is emitted) -Fixed: consistent merge of configuration arguments when an existing launch configuration exists +Fixed: consistent merge of configuration arguments when an existing launch configuration exists =========================================================================== 5.3 2006/10/30 @@ -1254,7 +1255,7 @@ Added: Can now specify listener classes 5.0.1 Fixed: reports generated by SuiteHTMLReporter do not work with JDK1.4 - + =========================================================================== 5.0 @@ -1340,7 +1341,7 @@ Fixed: TESTNG-18: Eclipse plugin ignores Factory annotation Fixed: TESTNG-21: Show differences when double clicking assertion exceptions Added: UI allows setting orientation (even more space) https://forums.opensymphony.com/thread.jspa?threadID=17225&messageID=33805#33805 - + =========================================================================== 4.5 @@ -1443,7 +1444,7 @@ Fixed: dependsOnGroups wasn't working on regular expressions Fixed: Bug in when directories contain spaces in their names Fixed: Introduced a JDK5 dependency in the JDK1.4 build (getEnclosingClass()) Fixed: Output directory in ant task was not honored if it didn't exist -Fixed: Problem with timeout according to +Fixed: Problem with timeout according to https://forums.opensymphony.com/thread.jspa?threadID=6707 Eclipse plug-in: @@ -1453,7 +1454,7 @@ Fixed: Bug in QuickFix implementation Added: Quick Fix for JUnit conversion (Annotations and JavaDoc) Fixed: Methods Run as TestNG test Added: Package level Run as TestNG test -Fixed: Resources from the linked directories are using a wrong path when +Fixed: Resources from the linked directories are using a wrong path when passed to command line TestNG IDEA plug-in: @@ -1518,7 +1519,7 @@ Fixed: TestNGException thrown when TestNG conditions are not fulfilled Documentation: - New assert classes -- New ways to launch +- New ways to launch - JUnitConverter documentation - new beforeSuite/afterSuite diff --git a/src/main/java/org/testng/internal/TestInvoker.java b/src/main/java/org/testng/internal/TestInvoker.java index 30751c6649..811eb19a08 100644 --- a/src/main/java/org/testng/internal/TestInvoker.java +++ b/src/main/java/org/testng/internal/TestInvoker.java @@ -32,7 +32,6 @@ import org.testng.SuiteRunState; import org.testng.SuiteRunner; import org.testng.TestException; -import org.testng.TestNGException; import org.testng.collections.Lists; import org.testng.collections.Maps; import org.testng.collections.Sets; @@ -793,15 +792,9 @@ public int invoke(int invCount) { arguments.getInstance()); if (bag.hasErrors()) { - ITestResult tr = bag.errorResult; - Throwable throwable = Objects.requireNonNull(tr).getThrowable(); - if (throwable instanceof TestNGException) { - tr.setStatus(ITestResult.FAILURE); - m_notifier.addFailedTest(arguments.getTestMethod(), tr); - } else { - tr.setStatus(ITestResult.SKIP); - m_notifier.addSkippedTest(arguments.getTestMethod(), tr); - } + ITestResult tr = Objects.requireNonNull(bag.errorResult); + tr.setStatus(ITestResult.FAILURE); + m_notifier.addFailedTest(arguments.getTestMethod(), tr); runTestResultListener(tr); result.add(tr); return invocationCount.get(); diff --git a/src/test/java/test/dataprovider/DataProviderWithErrorSample.java b/src/test/java/test/dataprovider/DataProviderWithErrorSample.java index bf72aa3ebd..a893a16d58 100644 --- a/src/test/java/test/dataprovider/DataProviderWithErrorSample.java +++ b/src/test/java/test/dataprovider/DataProviderWithErrorSample.java @@ -7,12 +7,12 @@ public class DataProviderWithErrorSample { @Test(dataProvider = "Data", invocationCount = 2) - public void testShouldSkip() { + public void testShouldFail() { Assert.fail(); } @Test(dataProvider = "Data", invocationCount = 2, successPercentage = 10) - public void testShouldSkipEvenIfSuccessPercentage() { + public void testShouldFailEvenIfSuccessPercentage() { Assert.fail(); } diff --git a/src/test/java/test/dataprovider/FailingDataProviderTest.java b/src/test/java/test/dataprovider/FailingDataProviderTest.java index 76339f54cd..ed229db756 100644 --- a/src/test/java/test/dataprovider/FailingDataProviderTest.java +++ b/src/test/java/test/dataprovider/FailingDataProviderTest.java @@ -1,5 +1,7 @@ package test.dataprovider; +import static org.assertj.core.api.Assertions.assertThat; + import org.testng.DataProviderInvocationException; import org.testng.ITestResult; import org.testng.annotations.Test; @@ -7,15 +9,13 @@ import test.SimpleBaseTest; import test.dataprovider.issue2157.TestClassWithDataProviderThatThrowsExceptions; -import static org.assertj.core.api.Assertions.assertThat; - public class FailingDataProviderTest extends SimpleBaseTest { - @Test(description = "TESTNG-142: Exceptions in DataProvider are not reported as failed test") + @Test(description = "TESTNG-142, GITHUB-217: Exceptions in DataProvider are not reported as failed test") public void failingDataProvider() { InvokedMethodNameListener listener = run(FailingDataProviderSample.class); - assertThat(listener.getSkippedMethodNames()).containsExactly("dpThrowingException"); + assertThat(listener.getFailedBeforeInvocationMethodNames()).containsExactly("dpThrowingException"); } @Test(description = "TESTNG-447: Abort when two data providers have the same name") @@ -29,12 +29,12 @@ public void duplicateDataProviders() { public void failingDataProviderAndInvocationCount() { InvokedMethodNameListener listener = run(DataProviderWithErrorSample.class); - assertThat(listener.getSkippedMethodNames()) + assertThat(listener.getFailedBeforeInvocationMethodNames()) .containsExactly( - "testShouldSkip", - "testShouldSkip", - "testShouldSkipEvenIfSuccessPercentage", - "testShouldSkipEvenIfSuccessPercentage"); + "testShouldFail", + "testShouldFail", + "testShouldFailEvenIfSuccessPercentage", + "testShouldFailEvenIfSuccessPercentage"); } @Test(description = "GITHUB-2157") diff --git a/src/test/java/test/retryAnalyzer/ExitCodeTest.java b/src/test/java/test/retryAnalyzer/ExitCodeTest.java index 610182f982..cdd47ee516 100644 --- a/src/test/java/test/retryAnalyzer/ExitCodeTest.java +++ b/src/test/java/test/retryAnalyzer/ExitCodeTest.java @@ -1,13 +1,12 @@ package test.retryAnalyzer; -import org.testng.ITestNGListener; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; + import org.testng.TestNG; import org.testng.annotations.Test; import test.SimpleBaseTest; -import static org.testng.Assert.assertEquals; -import static org.testng.Assert.assertTrue; - public class ExitCodeTest extends SimpleBaseTest { @Test public void exitsWithZeroOnSuccess() { @@ -26,23 +25,23 @@ public void exitsWithNonzeroOnFailure() { @Test public void exitsWithZeroAfterSuccessfulRetry() { TestNG tng = create(EventualSuccess.class); - tng.addListener((ITestNGListener) new TestResultPruner()); + tng.addListener(new TestResultPruner()); tng.run(); assertEquals(tng.getStatus(), 0); } @Test(description = "GITHUB-217") - public void exitWithNonzeroOnSkips() { + public void exitWithNonzeroOnDataProviderFailures() { TestNG tng = create(Issue217TestClassSample.class); tng.run(); - assertEquals(tng.getStatus(), 2); + assertEquals(tng.getStatus(), 1); } @Test(description = "GITHUB-217") - public void exitWithNonzeroOnSkips1() { + public void exitWithNonzeroOnDataProviderFailures1() { TestNG tng = create(Issue217TestClassSampleWithOneDataProvider.class); tng.run(); - assertEquals(tng.getStatus(), 2); + assertEquals(tng.getStatus(), 1); } }