From 048cd4a5e92239e3988822d12be776b7525c50cd Mon Sep 17 00:00:00 2001 From: Oleksandr Kulychok Date: Sat, 23 Nov 2019 05:45:49 +0200 Subject: [PATCH] Fix: alwaysRun should not force invocation of Before-methods Resolves cbeust/testng#1622 --- CHANGES.txt | 39 ++++++++++--------- .../org/testng/internal/ConfigInvoker.java | 29 +++++++------- .../org/testng/internal/MethodHelper.java | 18 ++++++++- 3 files changed, 50 insertions(+), 36 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 9ea7cad900..b27dd8de4b 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ Current +Fixed: GITHUB-1622: Parameter alwaysRun=true for before-methods forces execution of those methods (Oleksandr Kulychok) Fixed: GITHUB-2180: Write scenario details for Retried tests (Devendra Raju K) Fixed: GITHUB-2124: JUnit Report should contain the output of org.testng.Reporter (Krishnan Mahadevan) Fixed: GITHUB-2171: Ability to embed attachments and make them available on TestNG XML report (Krishnan Mahadevan) @@ -481,9 +482,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 @@ -500,7 +501,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 @@ -685,7 +686,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() @@ -829,10 +830,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) @@ -849,9 +850,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 @@ -966,8 +967,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 @@ -1060,7 +1061,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 (http://forums.opensymphony.com/thread.jspa?threadID=6594&messageID=122294#122294) Added: Support for concurrent execution for invocationCount=1 threadPoolSize>1 and @DataProvider (http://forums.opensymphony.com/thread.jspa?threadID=64738&tstart=0) @@ -1096,7 +1097,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 @@ -1133,7 +1134,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 @@ -1237,7 +1238,7 @@ Added: Can now specify listener classes 5.0.1 Fixed: reports generated by SuiteHTMLReporter do not work with JDK1.4 - + =========================================================================== 5.0 @@ -1323,7 +1324,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) http://forums.opensymphony.com/thread.jspa?threadID=17225&messageID=33805#33805 - + =========================================================================== 4.5 @@ -1426,7 +1427,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 http://forums.opensymphony.com/thread.jspa?threadID=6707 Eclipse plug-in: @@ -1436,7 +1437,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: @@ -1501,7 +1502,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/ConfigInvoker.java b/src/main/java/org/testng/internal/ConfigInvoker.java index 5565d59f03..361c9d21a9 100644 --- a/src/main/java/org/testng/internal/ConfigInvoker.java +++ b/src/main/java/org/testng/internal/ConfigInvoker.java @@ -1,9 +1,5 @@ package org.testng.internal; -import static org.testng.internal.Invoker.SAME_CLASS; -import static org.testng.internal.invokers.InvokedMethodListenerMethod.AFTER_INVOCATION; -import static org.testng.internal.invokers.InvokedMethodListenerMethod.BEFORE_INVOCATION; - import java.lang.reflect.InvocationTargetException; import java.util.Arrays; import java.util.Collection; @@ -11,6 +7,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; + import org.testng.IClass; import org.testng.IConfigurable; import org.testng.IInvokedMethodListener; @@ -30,6 +27,10 @@ import org.testng.xml.XmlClass; import org.testng.xml.XmlSuite; +import static org.testng.internal.Invoker.SAME_CLASS; +import static org.testng.internal.invokers.InvokedMethodListenerMethod.AFTER_INVOCATION; +import static org.testng.internal.invokers.InvokedMethodListenerMethod.BEFORE_INVOCATION; + class ConfigInvoker extends BaseInvoker implements IConfigInvoker { /** @@ -239,26 +240,22 @@ public void invokeConfigurations(ConfigMethodArguments arguments) { // - the test is enabled and // - the Configuration method belongs to the same class or a parent configurationAnnotation = AnnotationHelper.findConfiguration(annotationFinder(), method); - boolean alwaysRun = MethodHelper.isAlwaysRun(configurationAnnotation); - boolean canProcessMethod = - MethodHelper.isEnabled(objectClass, annotationFinder()) || alwaysRun; - if (!canProcessMethod) { - log( - 3, - "Skipping " - + Utils.detailedMethodName(tm, true) - + " because " - + objectClass.getName() - + " is not enabled"); + + if (!MethodHelper.isEnabled(objectClass, annotationFinder())) { + log(3, "Skipping " + Utils.detailedMethodName(tm, true) + " because class " + + objectClass.getName() + " is not enabled"); continue; } if (MethodHelper.isDisabled(configurationAnnotation)) { log(3, "Skipping " + Utils.detailedMethodName(tm, true) + " because it is not enabled"); continue; } + + boolean forceRun = MethodHelper.isAlwaysRun(configurationAnnotation) && + MethodHelper.isAfterMethod(configurationAnnotation); if (hasConfigurationFailureFor(arguments.getTestMethod(), tm.getGroups() , arguments.getTestClass(), - arguments.getInstance()) && !alwaysRun) { + arguments.getInstance()) && !forceRun) { log(3, "Skipping " + Utils.detailedMethodName(tm, true)); InvokedMethod invokedMethod = new InvokedMethod(arguments.getInstance(), tm, System.currentTimeMillis(), testResult); diff --git a/src/main/java/org/testng/internal/MethodHelper.java b/src/main/java/org/testng/internal/MethodHelper.java index a3f67263f2..1be36a2e8d 100644 --- a/src/main/java/org/testng/internal/MethodHelper.java +++ b/src/main/java/org/testng/internal/MethodHelper.java @@ -11,8 +11,8 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Pattern; - import java.util.stream.Collectors; + import org.testng.IInvokedMethod; import org.testng.IMethodInstance; import org.testng.ITestClass; @@ -226,6 +226,22 @@ static boolean isAlwaysRun(IConfigurationAnnotation configurationAnnotation) { return alwaysRun; } + /** + * @return true when configurationAnnotation belongs to {@code @After...} method + */ + static boolean isAfterMethod(IConfigurationAnnotation configurationAnnotation) { + if (null == configurationAnnotation) { + return false; + } + + return ((configurationAnnotation.getAfterSuite() + || configurationAnnotation.getAfterTest() + || configurationAnnotation.getAfterTestClass() + || configurationAnnotation.getAfterTestMethod() + || configurationAnnotation.getAfterGroups().length != 0)); + } + + /** Extracts the unique list of ITestNGMethods. */ public static List uniqueMethodList(Collection> methods) { Set resultSet = Sets.newHashSet();