testng-team / testng

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

When BeforeMethod throw exception, the Test(With Retry Listener and driven by dataProvider) does not skip #2531

Open Li-Vincent opened 3 years ago

Li-Vincent commented 3 years ago

TestNG Version

v7.4.0

Note: only the latest version is supported

Expected behavior

skip test When BeforeMethod throw exception

Actual behavior

When BeforeMethod throw exception, the Test(With Retry Listener and driven by dataProvider) does not skip

Is the issue reproductible on runner?

Test case sample

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

Test Scripts:

    package testScripts.sample.dataProvider;

    import listener.DefaultRetryAnalyzer;
    import org.testng.annotations.BeforeMethod;
    import org.testng.annotations.DataProvider;
    import org.testng.annotations.Test;

    public class DataProviderFailTest {
        @DataProvider(name = "arrayProvider")
        public Object[][] dataArray() {
            return new Object[][]{
                  {"Scenario_pass", 1.0d},
                  {"Scenario_fail", 2.0d},
                  {"Scenario_skip", 3.0d}
          };
        }

    @BeforeMethod()
    public void preCondition(Object[] params) {
        String scenarioName = (String) params[0];
        System.out.println("preCondition scenarioName is " + scenarioName);
        if (scenarioName.equals("Scenario_skip")) {
            throw new IllegalStateException("Config failed.", new UnsupportedOperationException());
        }
    }

    @Test(dataProvider = "arrayProvider", retryAnalyzer = DefaultRetryAnalyzer.class)
    public void testProvider(String scenarioName, double data) {
        System.out.println("scenarioName: " + scenarioName);
        System.out.println("data: " + data);
        if (scenarioName == "Scenario_fail") {
            throw new IllegalStateException("This Scenario should be failed.", new UnsupportedOperationException());
        }
    }
    }

RetryAnalyzer.java

    package listener;

    import org.testng.IRetryAnalyzer;
    import org.testng.ITestResult;

    public class DefaultRetryAnalyzer implements IRetryAnalyzer {
        private int retryCount = 1;
        private static final int maxRetryCount = 3; // max retry times

    @Override
    synchronized public boolean retry(ITestResult result) {
        if (retryCount < maxRetryCount) {
            retryCount++;
            return true;
        }
        return false;
    }
    }

TestXML set configfailurepolicy="continue"

Li-Vincent commented 3 years ago

Run Results: image

Li-Vincent commented 3 years ago

If I remove retryAnalyzer = DefaultRetryAnalyzer.class in @Test()

the result is ok. image

krmahadevan commented 3 years ago

@Li-Vincent - Here's what the documentation states in terms of configfailurepolicy from here

Whether TestNG should continue to execute the remaining tests in the suite or skip them if an @Before* method fails. Default behavior is skip.

Since your suite file is configured to continue, TestNG will ignore the @Beforex method results and continue to execute the Tests.

TestNG is working as designed here.

Li-Vincent commented 3 years ago

There is a question here. Skip will be ignored only when RetryAnalyzer.class is added, but skip will not be ignored even if configfailurepolicy = continue when RetryAnalyzer is not used.

juherr commented 3 years ago

@krmahadevan It looks there is an undocumented side-effect on skipped status (or configfailurepolicy) when a retry analyzer is used. I think using a retry analyzer should not have impacts on the result (except some retrials) Thoughts?

krmahadevan commented 3 years ago

@krmahadevan It looks there is an undocumented side-effect on skipped status (or configfailurepolicy) when a retry analyzer is used. I think using a retry analyzer should not have impacts on the result (except some retrials) Thoughts?

I am not sure @juherr Been super swamped at work and thanks to lockdowns again, juggling as a cook and as a maid as well :)

Haven't found time to look at this. I will squeeze in sometime over this weekend take this up and come back to you. Apologies for the delay in responding.