testng-team / testng

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

TestNG 7.x DataProvider works in opposite to TestNG 6.x when retrying tests. #3041

Closed ubutar closed 6 months ago

ubutar commented 8 months ago

TestNG Version

7.9 (compared vs 6.8)

Expected behavior

TestNG DataProviders are frequently used as data generators, when feeding the same data to a test makes no sense due to application constraints, thus, it is (was) expected that DataProvider method is called for every retry.

In TestNG 6, when a test fails and is retried using IRetryAnalyser, dataProvider is called again, and failed tests are retried with new data (I'm not sure if it picks the data from newly generated dataset under same index). More often than not, it's the same data, except the cases described above, but it's being read and fed once again. Quite often, though, such dataproviders return single object.

In early TestNG 7, when Test method was retried, DataProvider was called every time, but with no effect - original data was fed. This was fixed in https://github.com/testng-team/testng/issues/2884 was discussed in https://github.com/testng-team/testng/issues/763 and in https://github.com/testng-team/testng/pull/2885 and of course on StackOverflow

Actual behavior

So, since TestNG 7.8, data is being generated once. Every retry is being fed the original data. Problem is, both make sense. Compared to v6, it's a regression and a pain when migrating to v7, for the mentioned case of "single object returned - but unique every time". For the sake of healthy test design, new behavior makes sense too - tests should fail once there is reason, and not be retried until they pass.

Is the issue reproducible on runner?

Test case sample

import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

import java.util.concurrent.atomic.AtomicInteger;

public class Test6_8 {

    private AtomicInteger integer = new AtomicInteger(0);

    @Test(dataProvider = "generator", retryAnalyzer = RetryListener.class)
    public void testableTest(Integer unique) {
        System.out.println("Test Input: "+unique);
        assert false;
    }

    @DataProvider
    public Object[][] generator() {
        System.out.println("DataProvider: "+integer.incrementAndGet());
        return new Object[][] {{integer.get()}};
    }

    public static class RetryListener implements IRetryAnalyzer {
        @Override
        public boolean retry(ITestResult iTestResult) {
            return iTestResult.getMethod().getCurrentInvocationCount() < 3;
        }
    }
}

Additionally, I've thrown in a project on GitHub: testng6vs7 for easy comparing

Suggestions

I guess since both make sense and are in different versions of TestNG, both should be present in coming version. My suggestion is doing one of the following:

It's not really clear to me personally what to do when there is NxM array of data is fed, and out of N tests, n1<N fail. Guess the best decision for the case of alwaysRun=true dataprovider would be execute it again, feed data from under the same index. And the rest should be responsibility of those who implement IRetryAnalyzer

ubutar commented 8 months ago

If this is too much to ask, how about adding getDataprovider() method and setData(Object ...) to ITestNGMethod so that we can implement this behaviour from listeners?..

juherr commented 8 months ago

Another option is a new behavior configuration like we often did. It is easier than an api modification.

juherr commented 8 months ago

The new features you propose sound great. You should propose them here: https://github.com/testng-team/testng/discussions/3037

ubutar commented 8 months ago

@juherr Did that. Though I'm confused as to whether call it an issue or a feature request. As said above, it's a regression issue; having it makes total sense. Having the opposite does too. @krmahadevan your thoughts?

ubutar commented 8 months ago

For history: the desired behavior exists up to 7.4.0. In 7.5.0 it has been broken, probably by one of many DataProvider related commits. Candidate for minor version fix, e.g. 7.9.1? :3

krmahadevan commented 8 months ago

@ubutar there have been some other merges as well which I feel shouldnt be part of a bugfix. So it will most likely be part of 7.10.0

On a side note would you be willing to help raise a pull request for this?