testng-team / testng

TestNG testing framework
https://testng.org
Apache License 2.0
1.98k stars 1.02k forks source link

BUG: Parameter alwaysRun=true for before-methods forces execution of those methods #1622

Open kool79 opened 6 years ago

kool79 commented 6 years ago

TestNG Version

6.12, 6.11, ...

Expected behavior

According to official documentation parameter 'alwaysRun' for before-methods is used only to enable/disable filtering by groups. It must not force method execution if higher-level configuration(s) failed. In any case alwaysRun should not be used both for filtering and for execution forcing

Actual behavior

BeforeTest/BeforeClass/BeforeMethod with (alwaysRun = true) is executed when higher - level configuration failed (BeforeSuite/BeforeTest/BeforeClass)

Is the issue reproductible on runner?

Test case sample

Please, share the test case (as small as possible) which shows the issue


public class AlwaysRunTests {
@BeforeSuite
public void failedBeforeSuite() {
    System.out.println("I'm beforeSuite. I should be alone in console log. Going to fail...");
    throw new RuntimeException();
}

@BeforeTest(alwaysRun = true)
public void beforeTest() {
    System.out.println("BUG: I'm beforeTest and I was invoked (((( ");
    throw new RuntimeException();
}

@BeforeClass(alwaysRun = true)
public void beforeClass() {
    System.out.println("BUG: I'm beforeClass and I was invoked (((( ");
    throw new RuntimeException();
}

@BeforeMethod(alwaysRun = true)
public void beforeMethod() {
    System.out.println("BUG: I'm beforeMethod and I was invoked (((( ");
    throw new RuntimeException();
}

@Test
public void testMethod() {
    System.out.println("I'm testMethod");
}

}


Console output (stacktraces truncated):

I'm beforeSuite. I should be alone in console log. Going to fail...

java.lang.RuntimeException at qa.ok.testng.AlwaysRunTests.failedBeforeSuite(AlwaysRunTests.java:14)

BUG: I'm beforeTest and I was invoked ((((

java.lang.RuntimeException at qa.ok.testng.AlwaysRunTests.beforeTest(AlwaysRunTests.java:21)

BUG: I'm beforeClass and I was invoked ((((

java.lang.RuntimeException at qa.ok.testng.AlwaysRunTests.beforeClass(AlwaysRunTests.java:28)

BUG: I'm beforeMethod and I was invoked ((((

java.lang.RuntimeException at qa.ok.testng.AlwaysRunTests.beforeMethod(AlwaysRunTests.java:35) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:108) at org.testng.internal.Invoker.invokeConfigurationMethod(Invoker.java:523) at org.testng.internal.Invoker.invokeConfigurations(Invoker.java:224) at org.testng.internal.Invoker.invokeMethod(Invoker.java:599) at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:877) at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1201) at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:125) at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:109) at org.testng.TestRunner.privateRun(TestRunner.java:776) at org.testng.TestRunner.run(TestRunner.java:634) at org.testng.SuiteRunner.runTest(SuiteRunner.java:425) at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:420) at org.testng.SuiteRunner.privateRun(SuiteRunner.java:385) at org.testng.SuiteRunner.run(SuiteRunner.java:334) at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52) at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86) at org.testng.TestNG.runSuitesSequentially(TestNG.java:1318) at org.testng.TestNG.runSuitesLocally(TestNG.java:1243) at org.testng.TestNG.runSuites(TestNG.java:1161) at org.testng.TestNG.run(TestNG.java:1129) at org.testng.IDEARemoteTestNG.run(IDEARemoteTestNG.java:72) at org.testng.RemoteTestNGStarter.main(RemoteTestNGStarter.java:123)

Test ignored.

Default Suite Total tests run: 1, Failures: 0, Skips: 1 Configuration Failures: 4, Skips: 0

juherr commented 6 years ago

Thanks for the report.

How do you run the class?

Then, there are 2 interpretations:

Related to #1574 Ping @krmahadevan

kool79 commented 6 years ago

no group is no group. I run this without any groups defined, with default IntelliJ IDEA runner both from context menu on test-method and from context menu on testng.xml:

<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd" >
<suite name="my-suite">
    <test name ="my-test">
        <packages>
            <package name="qa.ok.testng.*"/>
        </packages>
    </test>
</suite>

(package qa.ok.testng.* contains only one class "AlwaysRunTests" listed in my prev. comment) No other settings/listeners/etc are present. I've attached zip with pom, java and testng.xml: always-run-test.zip

juherr commented 6 years ago

I reword:

no group is the default group

Your current suite will run all tests in the package, right? And not just tests without group.

BTW, it is debatable and we can only quick fix regression for the moment.

kool79 commented 6 years ago

Your current suite will run all tests in the package, right?

Yes. I understand. You mean my tests-list can also have some tests with groups so this can be the reason why Before-methods invoked. But I don't want to run those methods if any of the 'parent' configurations failed. In any case current behavior does not corresponds to documentation. I don't know if it is the documentation bug or it the bug in code which was introduced after documentation was published. In documentation the parameter "alwaysRun" has 2 different meanings for before- and after- methods :

  • For before methods (beforeSuite, beforeTest, beforeTestClass and beforeTestMethod, but not beforeGroups): If set to true, this configuration method will be run regardless of what groups it belongs to.
  • For after methods (afterSuite, afterClass, ...): If set to true, this configuration method will be run even if one or more methods invoked previously failed or was skipped.

So first is about groups (always run regardless on group) and second - about failed/skipped (alwaysRun regardless on failed/skipped). But currently 'alwaysRun' for before-methods includes behavior for after-methods: when set to true: 1) configuration method will be run even if one or more methods invoked previously failed or was skipped AND 2) configuration method will be run regardless of what groups it belongs to.

kool79 commented 6 years ago

I've found this bug was introduced in testng-6.9.5 when fixing #420 Below the log for v6.9.4 (note that no other before methods were invoked except before-suite):

  D:\dev\AC\git\test-testng\testng.xml
I'm beforeSuite. I should be alone in console log. Going to fail...

java.lang.RuntimeException
    at qa.ok.testng.AlwaysRunTests.failedBeforeSuite(AlwaysRunTests.java:14)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85)
    at org.testng.internal.Invoker.invokeConfigurationMethod(Invoker.java:517)
    at org.testng.internal.Invoker.invokeConfigurations(Invoker.java:213)
    at org.testng.internal.Invoker.invokeConfigurations(Invoker.java:140)
    at org.testng.SuiteRunner.privateRun(SuiteRunner.java:296)
    at org.testng.SuiteRunner.run(SuiteRunner.java:259)
    at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
    at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
    at org.testng.TestNG.runSuitesSequentially(TestNG.java:1199)
    at org.testng.TestNG.runSuitesLocally(TestNG.java:1124)
    at org.testng.TestNG.run(TestNG.java:1032)
    at org.testng.IDEARemoteTestNG.run(IDEARemoteTestNG.java:72)
    at org.testng.RemoteTestNGStarter.main(RemoteTestNGStarter.java:123)
Test ignored.
===============================================
my-suite
Total tests run: 1, Failures: 0, Skips: 1
Configuration Failures: 1, Skips: 3
===============================================
juherr commented 6 years ago

Ok, thanks for digging :+1:

krmahadevan commented 6 years ago

@juherr - I am not sure how to go about fixing this because :

I am not able to figure out how to determine if the user ran with group filtering or if the user ran tests as is. The reason I say this is because a user may have applied a method selector (either as implementation or as a beanshell expression) to filter methods.

So if I cannot determine if the tests are being run by applying groups, I will not be able to determine if alwaysRun=true should be considered or not.

kool79 commented 6 years ago

Guys, sorry but I just want to prevent breaking changes in testng, so will try to explain my opinion: Before testng v6.9.5 parameter alwaysRun had very different meaning in @Before- and @After- methods. Below I tried to describe behavior for that version (based on official testng documentation, javadocs and implementation):

Last note is very impotent. It means: even when if you force to execute @BeforeMethod after @BeforeClass was failed , all the methods which belongs to class will be skipped. So why we need to force execution of @BeforeMethod if test ignored in any way???

I think this behavior ( v6.9.5) should be saved because it was well-designed by @cbeust and it have meaning. There are no reasonable situations in testing when you should continue to execute 'child' prerequisites after 'parent' failed.

@krmahadevan, If I understand properly from PR 9d5e6d7, you want to force execution for before-methods only when configfailurepolicy = continue. Please use SRP principle for parameters in testng. Currently (v13.1) configfailurepolicy used to avoid multiple execution of the same configuration method. (Same means: method with concrete signature which belongs to concrete instance) Please don't use this parameter for any other purpose. This will break compatibility and will make process of testng configuration ugly.

Finally: IMHO to fix current issue you should revert changes which were made in scope of #420 (328620f) - it was wrong fix. Maybe it fixed behavior in particular case but it does not fix root cause. I'm almost sure that issue #420 can be reproduced with or without alwaysRun on @BeforeSuite

juherr commented 6 years ago

@kool79 It looks you have a good knowledge of the issue. Maybe you can propose a pull request with the fix?

krmahadevan commented 6 years ago

Sorry.. by mistake clicked the wrong button.

ntulele commented 6 years ago

@kool79 Just want to "upvote" this issue: I agree with your assessment, and find the current behavior frustrating. Given your extensive knowledge of the issue, would you mind trying to submit a fix? (i.e., reverting #420 but also proposing a better fix for that?)

aTan-aka-Xellos commented 6 years ago

+1, our team affected by this issue. Is there any work-around to force skip of before/after methods by manipulation with IContext for example?

hg0605 commented 5 years ago

any workaround for this issue?

krmahadevan commented 5 years ago

@hg0605 - No there aren't any workarounds for this issue at the moment.

cnealaccela commented 4 years ago

Any update on this? We want to terminate the suite at the BeforeSuite level, but are noticing all the beforeMethods are still running.

bdrx312 commented 1 month ago

I just ran into this too. I have the exact same use case where I have a @BeforeMethod that I want to run for all methods regardless of the group so I thought that using alwaysRun = true would be the solution. However, I want it to be still be skipped if the @Test is going to be skipped due to a failure like an @BeforeSuite method failing. Is there any work around for this?