testng-team / testng

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

RetryAnalyzer is not set properly and consistent when using dataprovider #2267

Closed erickubenka closed 4 years ago

erickubenka commented 4 years ago

Hi folks,

I've encountered a little bug when using a custom RetryAnalyzer and a DataProvider or the Parameter-Annotation.

TestNG Version

7.1.0 - And current master-branch.

Description

I will provide the code that leads to the "error", but at first I want to describe it.

When using the data provider and a custom RetryAnalyzer that is added in a beforeInvocation-method in an implementation of the IInvokedMethodListener-interface, the RetryAnalyzer is not added to the internal / private map m_testMethodToRetryAnalyzer in the following case:

When you call a getRetryAnalyzer(testResult) before a setRetryAnalyzerClass(RetryIfFailed.class) this error will occur, because the computeIfAbsent-call will return the default DisabledRetryAnalyzer instead of the new one that was added in beforeInvocation.

Expected behavior

The call of getRetryAnalyzer(iTestResult) should not affect the setRetryAnalyzerClass when using a Data Provider.

Actual behavior

The call of getRetryAnalyzer(iTestResult) affects the setRetryAnalyzerClass when using a Data Provider.

Is the issue reproductible on runner?

Test case sample

.CustomerListener

public class CustomListener implements IInvokedMethodListener {

    @Override
    public void beforeInvocation(IInvokedMethod method, ITestResult testResult) {

        if (method.isTestMethod()) {

            final IRetryAnalyzer currentRetryAnalyzer = method.getTestMethod().getRetryAnalyzer(testResult);

            // only override default retry analyzer but never a retry analyzer that may was added by another method call.
            if (currentRetryAnalyzer == null || currentRetryAnalyzer instanceof DisabledRetryAnalyzer) {
                method.getTestMethod().setRetryAnalyzerClass(RetryIfFailed.class);
                // this method here should as well respect the parameterbased setup that will be used for getting the current retry analyzer
            }
        }
    }
}

.RetryIfFailed RetryAnalyzer

public class RetryIfFailed implements IRetryAnalyzer {

    @Override
    public boolean retry(ITestResult result) {

        return !result.isSuccess();
    }
}

.RetryTest

@Listeners(CustomListener.class)
public class TestNgRetryTest {

    private static int counter = 0;

    @DataProvider(name = "dp")
    public Object[][] dp() {
        return new Object[][]{
                {1},
        };
    }

    @Test(dataProvider = "dp")
    public void testWithRetryAndDataProvider(int testNumber) {

        counter++;

        System.out.println("Data Provider: " + testNumber);
        System.out.println("Counter: " + counter);

        if (counter < 3) {
            Assert.fail("Counter is lower than 3: " + counter);
        }
    }
}

So executing this example will lead to a NON-Retry of my test method testWithRetryAndDataProvider. But when removing the getRetryAnalyzer(testResult) of my CustomListener class the test will be retried, because the internal map is filled correctly.

If I should add any value to this issue, please let me know. Further I will create a pull request for the bug fix, if you appreciate it.

Best regards Eric

krmahadevan commented 4 years ago

@erickubenka - Thanks for adding good amount of context to the issue. Why not raise a Pull request for this and we will help you get it merged as well ?