Skip to content

Enable phpcs for the tests folder #231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 6, 2023
Merged

Conversation

kevinfodness
Copy link
Member

Enables phpcs checks on files in the tests directory, because we should be enforcing coding standards everywhere, including when writing tests. Some notes on the change:

  • This is more for code formatting than it is for security/performance warnings, which will need to be selectively ignored in tests (e.g., when running a core function as part of a test fixture that phpcs will complain about, like flushing permalinks). However, this is easily done as an inline // phpcs:ignore within the test file.
  • This PR changes the naming convention of test classes back to class-* to make the phpcs tests pass, and updates the phpunit config to look for tests in all PHP files in the tests directory. PHPUnit is pretty smart about how it does this, and this change did not result in any difference in execution time for sample tests in this repo, even by looking at a more permissive set of files. See relevant discussion here on where the test- prefix came from and how it isn't an official PHPUnit standard, despite being in wide use in WP projects scaffolded using WP-CLI: WordPress.Files.FileName.InvalidClassFileName for tests/test-sample.php WordPress/WordPress-Coding-Standards#882

@kevinfodness kevinfodness changed the title Feature/phpcs tests Enable phpcs for the tests folder Jul 6, 2023
Copy link
Contributor

@anubisthejackle anubisthejackle left a comment

Choose a reason for hiding this comment

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

Looks good to me.

One thing we could do to help alleviate some of the pain of ignoring PHPCS issues in tests is put together a list of common safe exclusions for tests and add those to the .phpcs.xml file. I don't think that needs to happen today, but it might be something to consider as we find functions we're repeatedly ignoring.

@kevinfodness kevinfodness merged commit a5dac4d into develop Jul 6, 2023
@kevinfodness kevinfodness deleted the feature/phpcs-tests branch July 6, 2023 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants