Inherited groups should run only for groups selection

[krmahadevan]

Closes #2152

When a class is annotated with a “@test” and which
includes one or more groups, and when the test class
has one or more configuration methods, currently
TestNG inherits the groups information and ends
up running all the configurations even though
group filtering is not enabled.

Fixes #2152 .

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]

I suppose the previous tests were failing without the changes but I'm afraid of regressions for users too.

Could you check again if we can avoid or minimize the changes?

[krmahadevan]

@juherr

I suppose the previous tests were failing without the changes but I'm afraid of regressions for users too. Could you check again if we can avoid or minimize the changes?

I have tried my level best to keep the impact area to the minimum. The sleuth of tests that had to be changed, was only because they involved groups and now these tests need to be explicitly run in group mode to basically restore the validation that each of the tests are doing. I guess this should be ok.

[krmahadevan]

@juherr - I have fixed the review comments. Please take a look.

[juherr]

I agree with the change and I think it will be more logic after the fix.
But I'd prefer to let the final decision to @cbeust due to the regression risk.

@cbeust Please, could you check the issue #2152 and review the fix?

[krmahadevan]

Rebased off of master.. @cbeust - Please help provide your inputs.

[juherr]

Just an idea that will allow the merge without the need of @cbeust approbation:
-> having a configuration to keep the previous behavior (which will be removed in the next major release) and a specific highlight in the changelog for the potential regression (with the tips for having the previous behavior).

We should be safe enough then.

[krmahadevan]

@juherr

having a configuration to keep the previous behavior (which will be removed in the next major release) and a specific highlight in the changelog for the potential regression (with the tips for having the previous behavior).

Am not too excited about a configuration driven approach here for two reasons:

1. In my opinion this is a bug and is counter intuitive. So why fix a bug with a configuration ?
2. Its gonna require changes to the unit tests again, because now I will have to isolate the tests and have them sit separately in a configuration driven TestNG object. Not sure I have the bandwidth to go through that exercise.

I will wait to hear back from @cbeust on this.

[krmahadevan]

ping @cbeust

[mikalai-melnikau]

@krmahadevan Could we get published a new beta release (like 7.0.1-beta1) with this fix, please?