WordPress.Files.FileName.InvalidClassFileName for tests/test-sample.php

[danielbachhuber]

See https://travis-ci.org/wp-cli/sample-plugin/jobs/213413061

[jrfnl]

I don't see the problem. The sniff follows the rule as prescribed by core.

You can opt-out if you like: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/wiki/Customizable-sniff-properties#disregard-class-file-name-rules

[danielbachhuber]

@jrfnl PHPUnit test files are meant to be named test-*.php, but include a class extending WP_UnitTestCase.

[jrfnl]

@danielbachhuber Ah! Now I see what you mean.

[danielbachhuber]

PHPUnit test files are meant to be named test-*.php

Actually, this isn't a strict standard, but I think it's common enough that WPCS should accommodate it.

[jrfnl]

@danielbachhuber You have a point - could you please test PR #883 to see if that eases the pain ?

[danielbachhuber]

👍 WFM

[GaryJones]

I've got no objection to the fix, but I'm not sure where you're getting the test- prefix from.

• The prefix attribute isn't part of the PHPUnit 5.7 schema (and doesn't appear to have been in earlier versions of the config schema - I checked back to 4.0).
• The default for suffix is Test.php, which would make any prefix value of test- redundant.
• WP Core doesn't appear to use the test-*.php for it's test files.

[jrfnl]

@GaryJones FYI regarding the fix which has now been merged:

• The test- prefix is just used for the unit test files to auto document what we're testing.
• That prefix is at no point checked for by the sniff. All the sniff checks is that the file contains a class which is or extends one of the known unit test classes, if so, the whole class name check will be skipped.

[westonruter]

I'm not sure where you're getting the test- prefix from

It comes from WP-CLI, the wp plugin scaffold command. See https://github.com/wp-cli/wp-cli/blob/3f4670ea74c63d5db2eb5c2fd29930df9f31be2e/php/commands/scaffold.php#L455

[GaryJones]

Right, but @danielbachhuber must have decided to use that himself, or otherwise seen it around even though it may never have been supported with a prefix attribute - a de facto anti-pattern.

[danielbachhuber]

It predates me a while:

• https://github.com/wp-cli/wp-cli/pull/270
• https://unit-test.trac.wordpress.org/ticket/39

I think the pattern has been around long enough that it makes sense to support.

[GaryJones]

So Core doesn't use it, and the only plugins (and themes) that use it are those that were scaffolded via wp-cli. While that's not an insignificant number, there's likely to be some, as many or more (really, I have no idea) who have unit tests files created by other means.

The unique situation here though, is that most unit test files can be renamed without significant consequence, since PHPUnit is just looking in a specific directory for *Test.php or whatever the defined suffix attribute says; apart from any manual requires there might be, any prefixes can be dropped.

I'm fine with the exclusion such that files with test classes aren't named with the class- prefix.

A secondary check (part of the Extra ruleset) that warns (with an option to turn off) about the test class files starting with test- might help to fix this pattern that exists in plugins/themes for no good reason (and isn't followed by Core anyway).

[danielbachhuber]

the only plugins (and themes) that use it are those that were scaffolded via wp-cli.

Not necessarily. Here's 386k results in GitHub (unfortunately, not exact match): https://github.com/search?utf8=%E2%9C%93&q=%3Cdirectory+prefix%3D%22test-%22+suffix%3D%22.php%22%3E.%2Ftests%2F%3C%2Fdirectory%3E&type=Code

[GaryJones]

Using a smaller more exact text string, and limiting the file extension brings that number down to 60k: https://github.com/search?utf8=%E2%9C%93&q=prefix%3D%22test-%22+extension%3Axml+extension%3Axml.dist&type=Code

There's no easy way to see if those results all apply to WordPress plugins/themes, and of course not all WP plugins / themes are on GitHub. Not all of those 60k projects will be using PHPCS.