WordPress-Coding-Standards icon indicating copy to clipboard operation
WordPress-Coding-Standards copied to clipboard

Sniff to check that test class names and test file names comply with PHPUnit naming conventions

Open jrfnl opened this issue 1 year ago • 6 comments

Is your feature request related to a problem?

To quote myself in Trac ticket https://core.trac.wordpress.org/ticket/62004:

As things are, none of the WordPress test classes comply with the naming conventions expected by PHPUnit and while they will still run on PHPUnit 9 (and 10 for that matter), this is no longer the case on PHPUnit 11 where any and all tests which do not comply with the naming conventions will no longer run.

For automatic recognition of the tests, test classes have to follow the following naming conventions:

  1. Class name [SomethingUnderTest]Test - take note of the Test suffix.
  2. The file name for the test class MUST match the class name EXACTLY in a case-sensitive manner. In other words, test file names have to comply largely with PSR4 file name regulations, though underscores in the names are still allowed.

With some config tweaks, the first rule could be bypassed, however, the second rule cannot be bypassed.

As all test classes will need to be touched/changed/split to address problem 4 anyway, I would strongly recommend for the new test classes to be made fully compatible with the PHPUnit naming conventions. This will prevent having to re-do this exercise yet again in the future if the bypass for the first rule would be made impossible.

Describe the solution you'd like

In other words, while it would not be possible at this point in time to introduce a sniff to check that test classes and test files comply with the PHPUnit naming conventions (as it would fail the CS build of WP Core hard), this will need to be addressed in the next year or so.

A sniff for this needs to be available by the time WordPress starts supporting PHPUnit 11 to prevent new issues being (re-)introduced into the codebase.

I still have a plan to create a PHPUnit related external PHPCS standard. That standard would be the best place for a sniff like this.

If that standard would be made available/public in time, we can add it as a new dependency and use a sniff from that standard (ideally). If the standard is not (yet) available, we will need to think of a WPCS native solution for the time being.

As for what should and shouldn't be flagged, let's quote myself again:

What this means in practice (example):

In the current situation, the test class Tests_Compat_ArrayKeyLast is in a file called tests/compat/arrayKeyLast.php. This class will need to be renamed to Compat_ArrayKeyLast_Test and the file will need to be renamed to tests/compat/Compat_ArrayKeyLast_Test.php.

Or, even better, we could choose to use namespaces in the tests (also see ticket #53010) and to rename the class to WordPress\Tests\Compat\ArrayKeyLastTest and the file to tests/compat/ArrayKeyLastTest.php.

Additional context (optional)

Related to:

  • #1995
  • https://core.trac.wordpress.org/ticket/53010
  • https://core.trac.wordpress.org/ticket/62004
  • [ ] I intend to create a pull request to implement this feature.

jrfnl avatar Sep 07 '24 02:09 jrfnl

I still have a plan to create a PHPUnit related external PHPCS standard. That standard would be the best place for a sniff like this.

I wound up writing a few PHPUnit-related sniffs while working on updating our stuff to work with PHPUnit 11 and 12. I thought I'd drop you a note about it in case you might find them useful. 🙂

  • A TestClassName sniff to check for naming, as described in this issue. I also had it require abstract test case subclasses to be named like "*TestCase" so it can have a better chance of identifying misnamed test classes.
  • A TestMethodCovers sniff to flag use of @covers and @uses on test methods, and migrate them to the class level. (We decided not to worry about splitting our test classes to cover individual methods; we only haphazardly use @covers in the first place.)
  • An Attributes sniff to match annotations against attributes to make sure they match. It can add any that are missing, and can be configured either to require both annotations and attributes or to remove annotations in favor of attributes.

The code is in https://github.com/Automattic/jetpack/tree/trunk/projects/packages/codesniffer/Jetpack/Sniffs/PHPUnit

In the package are also some utility classes for working with attrbutes, doc blocks, and resolving class names.

anomiex avatar Apr 24 '25 18:04 anomiex

@anomiex Nice! And yes, I have some sniffs like that locally as well for the PHPUnitQA standard. Would love to compare notes at some point.

The problem with the sniffs you link to, is that they are under a GPL license which is incompatible with the WPCS license, as well as being incompatible with the licenses used in the PHPCSStandards organisation, so unfortunately I can't ask you to contribute (parts of) these sniffs to PHPUnitQA if and when.

jrfnl avatar Apr 24 '25 23:04 jrfnl

I may be able to change that. The relevant code is (so far) 100% my own authorship, although my employer claims rights to it since I did it on their time. Before I ask them, let me make sure I have the details right.

Looks like everything in PHPCSStandards is using LGPL v3. I don't think WPCS's MIT license matters if we're talking about a PHPCSStandards/PHPUnitQA project. Is all that correct?

anomiex avatar Apr 25 '25 18:04 anomiex

@anomiex Well, there's a mix of licenses for historic reasons.

  • PHPCS itself is licensed under BSD-3.
  • The Composer installer + the XMLlint Validate package are licensed under MIT.
  • PHPCSUtils, PHPCSExtra, PHPCSDevTools and PHPCSDevCS are licensed under LGPL v3.

I agree, WPCS's license can probably be disregarded.

Of course, PHPUnitQA hasn't got a license yet, but the GPL license would not be acceptable. I'd be inclined to go for either BSD-3 (to stay in line with PHPCS itself for new external standards), or with LGPL v3 (to be in line with existing external standards in the org).

Maybe it would be an idea for us to put our heads together to get an initial version of PHPUnitQA set up ? Want to discuss it in a call ?

jrfnl avatar Apr 25 '25 19:04 jrfnl

I can do a call if you want, but it may be easier to open an issue or discussion over in the PHPCSStandards org and talk about it async. 🙂

anomiex avatar Apr 30 '25 16:04 anomiex

@anomiex Could you DM me in the A8C slack ? (as a guest I can't initiate DMs)

jrfnl avatar May 01 '25 05:05 jrfnl

I may be able to change that. The relevant code is (so far) 100% my own authorship, although my employer claims rights to it since I did it on their time.

They cleared it: https://github.com/Automattic/jetpack/pull/46328

anomiex avatar Dec 16 '25 20:12 anomiex