Tests/Ruleset: introduce an abstract base TestCase and start using it

[jrfnl]

Description

Tests/Ruleset: introduce an abstract base TestCase

Introduce an abstract base TestCase with some helper methods specifically for tests testing aspects of the Ruleset class.

Tests/Ruleset: start using the new AbstractRulesetTestCase

Start using the new AbstractRulesetTestCase in pre-existing Ruleset tests.

Suggested changelog entry

N/A

Related issues/external references

This is a preliminary PR for a series of PRs expanding the tests for the Ruleset class.

[rodrigoprimo]

LGTM!

I just have one question that is not a blocker. Instead of creating an abstract base class for the ruleset tests, have you considered creating a more generic abstract base class that can be used by other tests as well?

I imagine that having those PHPUnit cross-version compatible methods will be useful for other tests. For example, there are already a few places in the PHPCS code base that check whether to use expectException() or setExpectedException():

https://github.com/search?q=repo%3APHPCSStandards%2FPHP_CodeSniffer%20method_exists(%24this%2C%20%27expectException%27)&type=code

There is even AbstractMethodUnitTest::expectRunTimeException():

https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/866f7cf551e49a410581cd50e4e1c44e2e4db2f0/tests/Core/AbstractMethodUnitTest.php#L207C21-L207C43

[jrfnl]

I just have one question that is not a blocker. Instead of creating an abstract base class for the ruleset tests, have you considered creating a more generic abstract base class that can be used by other tests as well?

I imagine that having those PHPUnit cross-version compatible methods will be useful for other tests. For example, there are already a few places in the PHPCS code base that check whether to use expectException() or setExpectedException():

Good question. Originally (while working on this series) this base test case contained some more Ruleset specific methods, which is why I introduced it as a Ruleset specific test case.

On that note, I just noticed the RULESET_CLASS constant declaration, which is a left-over of that earlier iteration with more Ruleset specific methods. I've just removed that, as it is no longer used in the current version of the class.

I did consider introducing a more generic TestCase for the Core tests (and am still considering it), but part of the usefulness of that class would be moot once we get to PHPCS 4.0 as support for PHPUnit < 8 has been dropped and the toggle for expecting exceptions is no longer needed.

As most of that work to drop support for PHPUnit < 8 is already in the 4.x branch, it would also make merging a PR which introduced a generic TestCase to existing tests more difficult for the cherrypick to the 4.x branch and would also make future cherrypicks for Core tests to the 4.x branch more difficult.

All in all, I decided that introducing this TestCase (as it currently is) now would be helpful for the task at hand (Ruleset tests) and to evaluate whether it should be moved to be a more generic TestCase at a later point in time. Most likely when working on the 4.x branch recreation.

Does that make sense ?

[rodrigoprimo]

Does that make sense ?

That makes sense. Thanks for the additional details!