testng-team / testng

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

configfailurepolicy=continue only works for BeforeTest when using TestNG XML file #2731

Open Mei-self opened 2 years ago

Mei-self commented 2 years ago

TestNG Version

7.5

Expected behavior

if configfailurepolicy=continue and any BeforeXXX method fails, the remaining test method should execute not skip

Actual behavior

only BeforeTest fails, the remaining test method would execute

Is the issue reproducible on runner?

Test case sample

<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd">
<suite name="TestSuite" configfailurepolicy="continue">
<test name="Test">
<classes>
<class name="TestConfigFail"/>
</classes>
</test>
</suite>
public class TestConfigFail {

    @BeforeSuite
    public void beforeSuite() {
        Assert.fail("This before suite is fail");
    }

    @BeforeTest
    public void beforeTest() {
        Assert.fail("This before test is fail");
    }

    @BeforeClass
    public void beforeClass(){
        Assert.fail("This before class is fail");
    }

    @BeforeMethod
    public void beforeMethod(){
        Assert.fail("This before method is fail");
    }

    @Test
    public void test() {
        System.out.println("This is test method");
    }
}

Contribution guidelines

Incase you plan to raise a pull request to fix this issue, please make sure you refer our Contributing section for detailed set of steps.

krmahadevan commented 2 years ago

@Wang-Meimei - Is this something that was broken in 7.5 or has this never worked ? Can u please check in 7.4.0 and 6.14.3 and post back your findings ?

Mei-self commented 2 years ago

@Wang-Meimei - Is this something that was broken in 7.5 or has this never worked ? Can u please check in 7.4.0 and 6.14.3 and post back your findings ?

It seems this never worked. But from the document, it should work. So maybe need to update document?

juherr commented 2 years ago

It seems this never worked. But from the document, it should work. So maybe need to update document?

Yes, as we know now it is no a regression, we need to determine the expected behavior and fix the code or the doc.

I think the issue is valid and the method should be run. @krmahadevan wdyt?

krmahadevan commented 2 years ago

It seems this never worked. But from the document, it should work. So maybe need to update document?

Yes, as we know now it is no a regression, we need to determine the expected behavior and fix the code or the doc.

I think the issue is valid and the method should be run. @krmahadevan wdyt?

I dont know if this is an issue @juherr .

If there is a higher and lower order configuration, then we should not try to execute the lower order configuration.

For .e.g,

Lets say I have

In this case, we should not try to execute the @BeforeTest because this is a configuration method. But then one can argue that if one has to get to the level of running a @Test method, then all the above level configurations need to be run.

So am not sure what the behaviour should be.

@bj-9527 - Please note.

juherr commented 2 years ago

IMO, the only difference between continue and skip is that continue will execute the method, and skip won't execute and will directly set the SKIP status.

But when I see #2735 changes in tests, I wonder if the behavior change should not be configurable.

bj-9527 commented 2 years ago

It could be config by FailurePolicy which could be set as SKIP or CONTINUE, then the behavior could change.

So am not sure what the behaviour should be.

For, this change, the SKIP are working as before, the CONTINUE will execute the following method even previous config failed.