Handle exceptions from data provider

[krmahadevan]

Closes #2157

Fixes #2157 .

Did you remember to?

• Add test case(s)
• Update CHANGES.txt

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.

[juherr]

What will be the state of the test result?

If I remember well, there is a rule where the run is supposed to fail (TestNGException) if any exception occurs in a configuration method if the dev must fix things.
Because it is not the same signification than a skipped test in the test plan for a functional reason.

Here, it's a bit more complicated because the test is run twice and fails its third invocation.

Thought?

[krmahadevan]

@juherr - Here the data provider is throwing an exception which basically means that we will never get to running an iteration for the test for that particular set of data because we won't even get to that level. So the state would be as before (test had already failed, a retry was attempted with a call to the data provider, and the data provider now throws an exception. So TestNG can't do anything here, but just remember the last state of the test method, which would be failed).

Not sure if I answered the question.

[juherr]

What will be the state of https://github.com/cbeust/testng/pull/2159/files#diff-de2f62247bed27cf8d8738dec1c610faR42 ? Skipped or Failed (maybe assert it)?

The behavior should be the same than an exception in a before method.

[krmahadevan]

Skipped or Failed (maybe assert it)?

Will add it.

The behavior should be the same than an exception in a before method.

The exception is not from a before method. its being thrown by the data provider and TestNG is aborting with that.

[juherr]

Right, but both before methods and data providers are part of the test configuration (and not the test itself). That's why I think the behavior should be the same for both.

According to the documentation, data providers are part of the parameters (https://testng.org/doc/documentation-main.html#parameters-dataproviders).
So, IMO, a failure in the data provider is like a missing parameter in testng.xml.
I don't remember what is the behavior of a missing parameter. skip test, failed test or run exception?

[krmahadevan]

Right, but both before methods and data providers are part of the test configuration (and not the test itself). That's why I think the behavior should be the same for both.

That kind of sounds weird. Imagine this scenario, a test method has a configuration method and is also data driven. Now if the data provider fails, the test should fail, not get skipped because the data provider is part of the test and is not like a configuration method. A configuration method establishes the pre-requistes, while a data provider satisfies the data requirements alone. So if the data requirements are not fulfilled a test should fail, whereas if a configuration method fails, the entry criteria fails and so a test gets skipped.

So, IMO, a failure in the data provider is like a missing parameter in testng.xml.

Functionally that is correct, but its not try with respect to the code flow. An exception in a data provider is not equivalent to a missing parameter. They are different.

[juherr]

Ok for the difference between configuration methods and data providers.

But why do you think a data provider exception and a missing parameter are different?
@Parameters is just a way to configure a specific data provider (IMO it should share most of the code).
In fact, I don't have any preference for the behavior but just keep the same for the global feature.

[krmahadevan]

But why do you think a data provider exception and a missing parameter are different?

Functionally they look the same i.e., in both the cases the data needed for test method is not available. But I think they are different in the fact that a data provider exception has an additional capability of signaling a problem (an attempt was made to construct a data set for the test but some issues were encountered), while the same is not true with a parameter driven test.

@parameters is just a way to configure a specific data provider (IMO it should share most of the code).

Sure. But I don't think the code flow is that right now. and there's no way in which i can have a missing parameter flow throw an exception. That is only possible from within a data provider.

[krmahadevan]

@juherr - I have added an assertion in the test that ensures that the test result is failed. Please take a look. I think that should be good enough for this changeset because this is a very corner case.