Skip to content

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Sep 5, 2025

Description

PR #1113 introduced a set of tests for the FileList class to accompany a bug fix.

However, it now looks like these tests break the ability to run sniff tests for external standards using the PHPCS native test framework. This is problematic and even more so as this will silently fail in CI, as PHPUnit 9 and lower exits with a 0 exit code when no tests are found.

For now, I'm removing the tests for the FileList class, while keeping the bugfix.

I've confirmed this fixes the issue by running the tests of two external standards which both use the PHPCS native test framework. Both were not running their tests with PHPCS 3.13.3 and the tests run correctly again with the FileList class tests removed.

Suggested changelog entry

Fixed: Tests for external standards using the PHPCS native test framework would no longer run.

Additional information

These tests will not be removed for the 4.x branch. In combination with the 4.x branch, tests for external standards run fine, so I suspect the problem may have something to do with the TestSuite mechanism (which has been removed for 4.x).

PR 1113 introduced a set of tests for the `FileList` class to accompany a bug fix.

However, it now looks like these tests break the ability to run sniff tests for external standards using the PHPCS native test framework.
This is problematic and even more so as this will silently fail in CI, as PHPUnit 9 and lower exits with a `0` exit code when no tests are found.

For now, I'm removing the tests for the `FileList` class, while keeping the bugfix.

I've confirmed this fixes the issue by running the tests of two external standards which both use the PHPCS native test framework. Both were not running their tests with PHPCS 3.13.3 and the tests run correctly again with the `FileList` class tests removed.
@jrfnl jrfnl added this to the 3.13.4 milestone Sep 5, 2025
@jrfnl jrfnl added the Type: bug label Sep 5, 2025
@jrfnl jrfnl merged commit d9b3e66 into master Sep 5, 2025
79 of 80 checks passed
@jrfnl jrfnl deleted the feature/remove-filelist-tests branch September 5, 2025 04:45
@rodrigoprimo
Copy link
Contributor

@jrfnl, I think I managed to understand why the FileList tests broke the ability to run sniff tests for external standards using the PHPCS native test framework.

This problem was happening because self::initializeConfigAndRuleset() was called in a data provider method in AddFileTest.php. This means that the Config instance created inside initializeConfigAndRuleset() using ConfigDouble was created before AllSniffs::suite() had a chance to get the installed standards from the CodeSniffer.conf configuration file. When AllSniffs::suite() runs, ConfigDouble already overrode the configData and configDataFile properties, and CodeSniffer.conf is never read.

I can prepare a PR to fix this. Here is the current way that I'm considering this should be addressed:

6e37c2c

This is the branch that I'm using:

https://github.com/PHPCSStandards/PHP_CodeSniffer/compare/master...rodrigoprimo:restore-file-list-tests?expand=1

I don't think I will be able to finish testing this approach before the end of my day today, but I can finish it on Monday and create the PR.

@jrfnl
Copy link
Member Author

jrfnl commented Sep 12, 2025

@rodrigoprimo Thanks for investigating. I look forward to seeing the PR.

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