From a12ddb07f4b23d440d86007af97e36216a00be06 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Fri, 12 Sep 2025 16:24:29 -0300 Subject: [PATCH 1/2] Revert ":fire: Hot Fix: remove FileList tests" This reverts commit def8714f53561dc010398bf78a1bc257c3d71127. --- .../FileList/AbstractFileListTestCase.php | 55 +++++++ tests/Core/Files/FileList/AddFileTest.php | 138 ++++++++++++++++++ tests/Core/Files/FileList/ConstructTest.php | 122 ++++++++++++++++ tests/Core/Files/FileList/FilterDouble.php | 31 ++++ tests/Core/Files/FileList/Fixtures/file1.php | 3 + tests/Core/Files/FileList/Fixtures/file2.php | 3 + 6 files changed, 352 insertions(+) create mode 100644 tests/Core/Files/FileList/AbstractFileListTestCase.php create mode 100644 tests/Core/Files/FileList/AddFileTest.php create mode 100644 tests/Core/Files/FileList/ConstructTest.php create mode 100644 tests/Core/Files/FileList/FilterDouble.php create mode 100644 tests/Core/Files/FileList/Fixtures/file1.php create mode 100644 tests/Core/Files/FileList/Fixtures/file2.php diff --git a/tests/Core/Files/FileList/AbstractFileListTestCase.php b/tests/Core/Files/FileList/AbstractFileListTestCase.php new file mode 100644 index 0000000000..abced07317 --- /dev/null +++ b/tests/Core/Files/FileList/AbstractFileListTestCase.php @@ -0,0 +1,55 @@ +filter = __DIR__.'/FilterDouble.php'; + self::$ruleset = new Ruleset(self::$config); + } + + }//end initializeConfigAndRuleset() + + +}//end class diff --git a/tests/Core/Files/FileList/AddFileTest.php b/tests/Core/Files/FileList/AddFileTest.php new file mode 100644 index 0000000000..4346fbbf95 --- /dev/null +++ b/tests/Core/Files/FileList/AddFileTest.php @@ -0,0 +1,138 @@ +files = []; + $this->fileList = new FileList(self::$config, self::$ruleset); + + }//end initializeFileList() + + + /** + * Test adding a file to the list. + * + * @param string $fileName The name of the file to add. + * @param object|null $fileObject An optional file object to add instead of creating a new one. + * + * @dataProvider dataAddFile + * + * @return void + */ + public function testAddFile($fileName, $fileObject=null) + { + $this->assertCount(0, $this->fileList); + + $this->fileList->addFile($fileName, $fileObject); + + $fileListArray = iterator_to_array($this->fileList); + + $this->assertCount(1, $this->fileList, 'File count mismatch'); + $this->assertArrayHasKey($fileName, $fileListArray, 'File not found in list'); + + if (isset($fileObject) === true) { + $this->assertSame($fileObject, $fileListArray[$fileName], 'File object mismatch'); + } else { + $this->assertInstanceOf( + 'PHP_CodeSniffer\Files\File', + $fileListArray[$fileName], + 'File object not found in list' + ); + } + + }//end testAddFile() + + + /** + * Data provider for testAddFile. + * + * @return array> + */ + public static function dataAddFile() + { + self::initializeConfigAndRuleset(); + + return [ + 'Regular file' => [ + 'fileName' => 'test1.php', + ], + 'STDIN' => [ + 'fileName' => 'STDIN', + ], + 'Regular file with file object' => [ + 'fileName' => 'test1.php', + 'fileObject' => new File('test1.php', self::$ruleset, self::$config), + ], + ]; + + }//end dataAddFile() + + + /** + * Test that it is not possible to add the same file twice. + * + * @return void + */ + public function testAddFileShouldNotAddTheSameFileTwice() + { + $file1 = 'test1.php'; + $file2 = 'test2.php'; + $expectedFiles = [ + $file1, + $file2, + ]; + + // Add $file1 once. + $this->fileList->addFile($file1); + $this->assertCount(1, $this->fileList); + $this->assertSame([$file1], array_keys(iterator_to_array($this->fileList))); + + // Try to add $file1 again. Should be ignored. + $this->fileList->addFile($file1); + $this->assertCount(1, $this->fileList); + $this->assertSame([$file1], array_keys(iterator_to_array($this->fileList))); + + // Add $file2. Should be added. + $this->fileList->addFile($file2); + $this->assertCount(2, $this->fileList); + $this->assertSame($expectedFiles, array_keys(iterator_to_array($this->fileList))); + + }//end testAddFileShouldNotAddTheSameFileTwice() + + +}//end class diff --git a/tests/Core/Files/FileList/ConstructTest.php b/tests/Core/Files/FileList/ConstructTest.php new file mode 100644 index 0000000000..f40a400993 --- /dev/null +++ b/tests/Core/Files/FileList/ConstructTest.php @@ -0,0 +1,122 @@ + $files List of file paths in the Config class. + * @param array $expectedFiles List of expected file paths in the FileList. + * + * @dataProvider dataConstruct + * + * @return void + */ + public function testConstruct($files, $expectedFiles) + { + self::$config->files = $files; + + $fileList = new FileList(self::$config, self::$ruleset); + + $this->assertSame(self::$config, $fileList->config, 'Config object mismatch'); + $this->assertSame(self::$ruleset, $fileList->ruleset, 'Ruleset object mismatch'); + + $this->assertCount(count($expectedFiles), $fileList, 'File count mismatch'); + + $i = 0; + + // Sort the values to make the tests stable as different OSes will read directories + // in a different order and the order is not relevant for these tests. Just the values. + $fileListArray = iterator_to_array($fileList); + ksort($fileListArray); + + foreach ($fileListArray as $filePath => $fileObject) { + $this->assertSame( + $expectedFiles[$i], + $filePath, + sprintf('File path mismatch: expected "%s", got "%s"', $expectedFiles[$i], $filePath) + ); + $this->assertInstanceOf( + 'PHP_CodeSniffer\Files\File', + $fileObject, + sprintf('File object for "%s" is not an instance of PHP_CodeSniffer\Files\File', $filePath) + ); + $i++; + } + + }//end testConstruct() + + + /** + * Data provider for testConstruct. + * + * @return array>> + */ + public static function dataConstruct() + { + $fixturesDir = __DIR__.DIRECTORY_SEPARATOR.'Fixtures'.DIRECTORY_SEPARATOR; + + return [ + 'No files' => [ + 'files' => [], + 'expectedFiles' => [], + ], + 'Two files' => [ + 'files' => [ + 'file1.php', + 'file2.php', + ], + 'expectedFiles' => [ + 'file1.php', + 'file2.php', + ], + ], + 'A directory' => [ + 'files' => [$fixturesDir], + 'expectedFiles' => [ + $fixturesDir.'file1.php', + $fixturesDir.'file2.php', + ], + ], + 'Same file twice' => [ + 'files' => [ + 'file1.php', + 'file1.php', + ], + 'expectedFiles' => [ + 'file1.php', + ], + ], + 'File and then directory containing that file' => [ + 'files' => [ + $fixturesDir.'file1.php', + $fixturesDir, + ], + 'expectedFiles' => [ + $fixturesDir.'file1.php', + $fixturesDir.'file2.php', + ], + ], + ]; + + }//end dataConstruct() + + +}//end class diff --git a/tests/Core/Files/FileList/FilterDouble.php b/tests/Core/Files/FileList/FilterDouble.php new file mode 100644 index 0000000000..99952ef63c --- /dev/null +++ b/tests/Core/Files/FileList/FilterDouble.php @@ -0,0 +1,31 @@ + Date: Fri, 12 Sep 2025 16:45:42 -0300 Subject: [PATCH 2/2] Files/FileList: fix tests to stop interfering with external standards tests The AddFileTest.php tests introduced an issue that 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, and the tests for external standards were not included. To fix this problem, I opted to change the test to create an instance of the `File` class (which requires an instance of the `Config` class) in the test itself instead of doing that in the data provider. --- tests/Core/Files/FileList/AddFileTest.php | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/tests/Core/Files/FileList/AddFileTest.php b/tests/Core/Files/FileList/AddFileTest.php index 4346fbbf95..2166e643f9 100644 --- a/tests/Core/Files/FileList/AddFileTest.php +++ b/tests/Core/Files/FileList/AddFileTest.php @@ -47,15 +47,21 @@ protected function initializeFileList() /** * Test adding a file to the list. * - * @param string $fileName The name of the file to add. - * @param object|null $fileObject An optional file object to add instead of creating a new one. + * @param string $fileName The name of the file to add. + * @param bool $passFileObject Whether to pass a File object to addFile() or not. * * @dataProvider dataAddFile * * @return void */ - public function testAddFile($fileName, $fileObject=null) + public function testAddFile($fileName, $passFileObject=false) { + $fileObject = null; + + if ($passFileObject === true) { + $fileObject = new File($fileName, self::$ruleset, self::$config); + } + $this->assertCount(0, $this->fileList); $this->fileList->addFile($fileName, $fileObject); @@ -65,7 +71,7 @@ public function testAddFile($fileName, $fileObject=null) $this->assertCount(1, $this->fileList, 'File count mismatch'); $this->assertArrayHasKey($fileName, $fileListArray, 'File not found in list'); - if (isset($fileObject) === true) { + if ($fileObject instanceof File) { $this->assertSame($fileObject, $fileListArray[$fileName], 'File object mismatch'); } else { $this->assertInstanceOf( @@ -81,12 +87,10 @@ public function testAddFile($fileName, $fileObject=null) /** * Data provider for testAddFile. * - * @return array> + * @return array> */ public static function dataAddFile() { - self::initializeConfigAndRuleset(); - return [ 'Regular file' => [ 'fileName' => 'test1.php', @@ -95,8 +99,8 @@ public static function dataAddFile() 'fileName' => 'STDIN', ], 'Regular file with file object' => [ - 'fileName' => 'test1.php', - 'fileObject' => new File('test1.php', self::$ruleset, self::$config), + 'fileName' => 'test1.php', + 'passFileObject' => true, ], ];