Skip to content

Conversation

rodrigoprimo
Copy link
Collaborator

Calling AbstractSniffUnitTest::setUpPrerequisites() manually inside RestrictedClassesUnitTest::enhanceGroups() is not necessary, as this method already executes via its @before tag and results in the method running twice.

Originally, RestrictedClassesUnitTest::enhanceGroups() was named RestrictedClassesUnitTest::setUp() and called parent::setUp() (WordPress/Tests/DB/RestrictedClassesUnitTest.php#L34-L35). But later it was renamed, and then it probably didn't make sense to continue calling parent::setUp() (which was later renamed to setUpPrerequisites()): 00b895c.

I found this in the context of the work to prepare this codebase for PHPCS 4.0 as AbstractSniffUnitTest::setUpPrerequisites() was renamed in 4.0 (PHPCSStandards/PHP_CodeSniffer@92cf10f#diff-91ba2a2a923bbfdaa61dcd31051d783bd0e7ff99f979f6e1a2917cae7dde3eb2L58).

…sites()`

Calling `AbstractSniffUnitTest::setUpPrerequisites()` manually inside `RestrictedClassesUnitTest::enhanceGroups()` is not necessary as this method already executes via its `@before` tag and results in the method running twice.

Originally, `RestrictedClassesUnitTest::enhanceGroups()` was named `RestrictedClassesUnitTest::setUp()` and called `parent::setUp()` (https://github.com/WordPress/WordPress-Coding-Standards/blob/3b9ae92607295548df357e108fd27090cf89d1d7/WordPress/Tests/DB/RestrictedClassesUnitTest.php#L34-L35). But later it was renamed, and then it probably didn't make sense to continue calling `parent::setUp()` (which was later renamed to `setUpPrerequisites()`): WordPress@00b895c.

I found this in the context of the work to prepare this codebase for PHPCS 4.0 as `AbstractSniffUnitTest::setUpPrerequisites()` was renamed in 4.0 (PHPCSStandards/PHP_CodeSniffer@92cf10f#diff-91ba2a2a923bbfdaa61dcd31051d783bd0e7ff99f979f6e1a2917cae7dde3eb2L58).
Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find @rodrigoprimo and good explanation.

While this fix is valid as-is for the current codebase, I want to point out that this will need to be brought back again once support for PHPCS 3.x will be dropped.

Reasoning:

  • Drop PHPCS 3.x means => drop support for PHP 7.2 means => no need to support PHPUnit 4-7 anymore.
  • Once support for PHPUnit 4-7 has been dropped, we can remove the @before/@after annotations and start using the normal setUp()/tearDown() methods again (as the required return type void is no longer a problem), in which case, we will need to call parent::setUp() again.

Not sure if you already have some commits in your WIP branch for the follow ups (add cross-version PHPCS 3/4 support => drop support for PHP < 7.2 => drop support for PHPUnit < 8 => drop support for PHPCS 3.x => add support for PHPUnit 10+11). If so, please make sure you add it back in the appropriate follow-up commit.
Otherwise, we should maybe make a note of this somewhere ?

@jrfnl jrfnl added this to the 3.2.x milestone Aug 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants