Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Streamline data providers & retrial of failed tests #2885

Merged
merged 1 commit into from
Mar 20, 2023

Conversation

krmahadevan
Copy link
Member

Closes #2884

When a data driven test fails, then ensure that
we don’t call the data provider once again when
retrying the test (we seem to invoke the data
provider multiple times, but ignore the value)

As part of fixing this, getting rid of the test for #2157.

#2157 - deals with null pointer exceptions arising Out of data provider that was re-invoked as a result Of us retrying a failed test.

But now since a data provider is NOT going to be
re-invoked the changes for #2157 become irrelevant.

Fixes #2884 .

Did you remember to?

  • Add test case(s)
  • Update CHANGES.txt
  • Auto applied styling via ./gradlew autostyleApply

We encourage pull requests that:

  • Add new features to TestNG (or)
  • Fix bugs in TestNG

If your pull request involves fixing SonarQube issues then we would suggest that you please discuss this with the
TestNG-dev before you spend time working on it.

Note: For more information on contribution guidelines please make sure you refer our Contributing section for detailed set of steps.

Closes testng-team#2884

When a data driven test fails, then ensure that 
we don’t call the data provider once again when 
retrying the test (we seem to invoke the data 
provider multiple times, but ignore the value)

As part of fixing this, getting rid of the test for
testng-team#2157.

testng-team#2157 - deals with null pointer exceptions arising
Out of data provider that was re-invoked as a result
Of us retrying a failed test.

But now since a data provider is NOT going to be 
re-invoked the changes for testng-team#2157 become irrelevant.
@juherr
Copy link
Member

juherr commented Mar 18, 2023

Are you sure that the removed sample is already covered by another sample?

@krmahadevan
Copy link
Member Author

Are you sure that the removed sample is already covered by another sample?

The removed sample covers the scenario wherein data providers were getting invoked for a failed test.

This PR prevents data providers from being invoked for failed tests ( it ensures only tests are retried and not their data providers ). So the removed sample is a failing test because it tests for something that no longer exists.

@juherr
Copy link
Member

juherr commented Mar 20, 2023

Ok. It was not clear the first time that the new and removed tests are similar and check the same behavior. Lgtm.

@krmahadevan krmahadevan merged commit ebd9dae into testng-team:master Mar 20, 2023
@krmahadevan krmahadevan deleted the issue_2884 branch March 20, 2023 05:33
@ubutar
Copy link

ubutar commented Jan 20, 2024

@krmahadevan

This is not an issue anymore in TestNG 7.8.0 (released version as of today). A data powered test that is being retried will get the same test data as the original and will continue to do so until it passes.

While it makes sense, in TestNg 6 it worked exactly the opposite way - dataprovider was called on each invocation attempt when retrying a test and the test was run with the resulting objects. Somewhere around 7.3 DataProvider was still called, but method was fed the original data. Now, in 7.8+ DataProvider is not called, as was stated here, and discussed in /763.

I suggest that both ways are preserved. Imagine when DataProvider is actually a generator and must provide unique data on each invocation, because, by test design, it's not possible to retry with same data, which will be stale/removed/incorrect by the time of retry. My suggestion is to have @DataProvider option "alwaysRun=true" to ensure dataprovider method actually runs and feeds data to @Test method no matter the history.

This is a pain when migrating from older Java 7 epoch tests to newest, tbh.

@krmahadevan
Copy link
Member Author

@ubutar - please file a separate issue with a sample and all additional context. That way its easier to track it to closure. Please dont forget to link this issue or PR in the new issue that you raise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discrepancies with DataProvider and Retry of failed tests
3 participants