From 2e7518daef4b57f4caef273e5e6dd33f159cdc31 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 31 Dec 2023 00:18:15 +0100 Subject: [PATCH 1/4] Tests/Filters: new `AbstractFilterTestCase` ... to contain some basic utilities and logic for use in Filter related tests. Includes switching the `Filter/AcceptTest` to use the new abstract and to use the available utilities. --- tests/Core/Filters/AbstractFilterTestCase.php | 104 ++++++++++++++++++ tests/Core/Filters/Filter/AcceptTest.php | 42 +------ 2 files changed, 110 insertions(+), 36 deletions(-) create mode 100644 tests/Core/Filters/AbstractFilterTestCase.php diff --git a/tests/Core/Filters/AbstractFilterTestCase.php b/tests/Core/Filters/AbstractFilterTestCase.php new file mode 100644 index 0000000000..a745a21481 --- /dev/null +++ b/tests/Core/Filters/AbstractFilterTestCase.php @@ -0,0 +1,104 @@ + + * @copyright 2023 PHPCSStandards Contributors + * @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence + */ + +namespace PHP_CodeSniffer\Tests\Core\Filters; + +use PHP_CodeSniffer\Config; +use PHP_CodeSniffer\Filters\Filter; +use PHP_CodeSniffer\Ruleset; +use PHPUnit\Framework\TestCase; +use RecursiveIteratorIterator; + +/** + * Base functionality and utilities for testing Filter classes. + */ +abstract class AbstractFilterTestCase extends TestCase +{ + + /** + * The Config object. + * + * @var \PHP_CodeSniffer\Config + */ + protected static $config; + + /** + * The Ruleset object. + * + * @var \PHP_CodeSniffer\Ruleset + */ + protected static $ruleset; + + + /** + * Initialize the config and ruleset objects. + * + * @beforeClass + * + * @return void + */ + public static function initializeConfigAndRuleset() + { + self::$config = new Config(['--standard=PSR1', '--report-width=80']); + self::$ruleset = new Ruleset(self::$config); + + }//end initializeConfigAndRuleset() + + + /** + * Retrieve an array of files which were accepted by a filter. + * + * @param \PHP_CodeSniffer\Filters\Filter $filter The Filter object under test. + * + * @return array + */ + protected function getFilteredResultsAsArray(Filter $filter) + { + $iterator = new RecursiveIteratorIterator($filter); + $files = []; + foreach ($iterator as $file) { + $files[] = $file; + } + + return $files; + + }//end getFilteredResultsAsArray() + + + /** + * Translate Linux paths to Windows paths, when necessary. + * + * These type of tests should be able to run and pass on both *nix as well as Windows + * based dev systems. This method is a helper to allow for this. + * + * @param array $paths A single or multi-dimensional array containing + * file paths. + * + * @return array + */ + protected static function mapPathsToRuntimeOs(array $paths) + { + if (DIRECTORY_SEPARATOR !== '\\') { + return $paths; + } + + foreach ($paths as $key => $value) { + if (is_string($value) === true) { + $paths[$key] = strtr($value, '/', '\\\\'); + } else if (is_array($value) === true) { + $paths[$key] = self::mapPathsToRuntimeOs($value); + } + } + + return $paths; + + }//end mapPathsToRuntimeOs() + + +}//end class diff --git a/tests/Core/Filters/Filter/AcceptTest.php b/tests/Core/Filters/Filter/AcceptTest.php index 53d4ba6441..facf9f9535 100644 --- a/tests/Core/Filters/Filter/AcceptTest.php +++ b/tests/Core/Filters/Filter/AcceptTest.php @@ -13,30 +13,16 @@ use PHP_CodeSniffer\Config; use PHP_CodeSniffer\Filters\Filter; use PHP_CodeSniffer\Ruleset; -use PHPUnit\Framework\TestCase; +use PHP_CodeSniffer\Tests\Core\Filters\AbstractFilterTestCase; /** * Tests for the \PHP_CodeSniffer\Filters\Filter::accept method. * * @covers \PHP_CodeSniffer\Filters\Filter */ -class AcceptTest extends TestCase +class AcceptTest extends AbstractFilterTestCase { - /** - * The Config object. - * - * @var \PHP_CodeSniffer\Config - */ - protected static $config; - - /** - * The Ruleset object. - * - * @var \PHP_CodeSniffer\Ruleset - */ - protected static $ruleset; - /** * Initialize the config and ruleset objects based on the `AcceptTest.xml` ruleset file. @@ -66,16 +52,10 @@ public static function initializeConfigAndRuleset() */ public function testExcludePatterns($inputPaths, $expectedOutput) { - $fakeDI = new \RecursiveArrayIterator($inputPaths); - $filter = new Filter($fakeDI, '/', self::$config, self::$ruleset); - $iterator = new \RecursiveIteratorIterator($filter); - $files = []; - - foreach ($iterator as $file) { - $files[] = $file; - } + $fakeDI = new \RecursiveArrayIterator($inputPaths); + $filter = new Filter($fakeDI, '/', self::$config, self::$ruleset); - $this->assertEquals($expectedOutput, $files); + $this->assertEquals($expectedOutput, $this->getFilteredResultsAsArray($filter)); }//end testExcludePatterns() @@ -121,17 +101,7 @@ public function dataExcludePatterns() ]; // Allow these tests to work on Windows as well. - if (DIRECTORY_SEPARATOR === '\\') { - foreach ($testCases as $key => $case) { - foreach ($case as $nr => $param) { - foreach ($param as $file => $value) { - $testCases[$key][$nr][$file] = strtr($value, '/', '\\'); - } - } - } - } - - return $testCases; + return self::mapPathsToRuntimeOs($testCases); }//end dataExcludePatterns() From 43c85200a9211102cbf1616814cf2bcf09d009ea Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 29 Dec 2023 10:21:41 +0100 Subject: [PATCH 2/4] Tests/AcceptTest: use named data sets With non-named data sets, when a test fails, PHPUnit will display the number of the test which failed. With tests which have a _lot_ of data sets, this makes it _interesting_ (and time-consuming) to debug those, as one now has to figure out which of the data sets in the data provider corresponds to that number. Using named data sets makes debugging failing tests more straight forward as PHPUnit will display the data set name instead of the number. Using named data sets also documents what exactly each data set is testing. Includes making the data type in the docblocks more specific. --- tests/Core/Filters/Filter/AcceptTest.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/Core/Filters/Filter/AcceptTest.php b/tests/Core/Filters/Filter/AcceptTest.php index facf9f9535..e6fe2be8a2 100644 --- a/tests/Core/Filters/Filter/AcceptTest.php +++ b/tests/Core/Filters/Filter/AcceptTest.php @@ -43,8 +43,8 @@ public static function initializeConfigAndRuleset() /** * Test filtering a file list for excluded paths. * - * @param array $inputPaths List of file paths to be filtered. - * @param array $expectedOutput Expected filtering result. + * @param array $inputPaths List of file paths to be filtered. + * @param array $expectedOutput Expected filtering result. * * @dataProvider dataExcludePatterns * @@ -65,34 +65,34 @@ public function testExcludePatterns($inputPaths, $expectedOutput) * * @see testExcludePatterns * - * @return array + * @return array>> */ public function dataExcludePatterns() { $testCases = [ // Test top-level exclude patterns. - [ - [ + 'Non-sniff specific path based excludes from ruleset and command line are respected and don\'t filter out too much' => [ + 'inputPaths' => [ '/path/to/src/Main.php', '/path/to/src/Something/Main.php', '/path/to/src/Somethingelse/Main.php', '/path/to/src/SomethingelseEvenLonger/Main.php', '/path/to/src/Other/Main.php', ], - [ + 'expectedOutput' => [ '/path/to/src/Main.php', '/path/to/src/SomethingelseEvenLonger/Main.php', ], ], // Test ignoring standard/sniff specific exclude patterns. - [ - [ + 'Filter should not act on standard/sniff specific exclude patterns' => [ + 'inputPaths' => [ '/path/to/src/generic-project/Main.php', '/path/to/src/generic/Main.php', '/path/to/src/anything-generic/Main.php', ], - [ + 'expectedOutput' => [ '/path/to/src/generic-project/Main.php', '/path/to/src/generic/Main.php', '/path/to/src/anything-generic/Main.php', From d9b6f3cde6ff177c8f4496cc37ad8394e9e11b9a Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 29 Dec 2023 10:21:41 +0100 Subject: [PATCH 3/4] Tests/AcceptTest: minor tweaks * Import all used classes. * Make the data provider static. * Prevent a potential call to `stty` by setting the report width. --- tests/Core/Filters/Filter/AcceptTest.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/Core/Filters/Filter/AcceptTest.php b/tests/Core/Filters/Filter/AcceptTest.php index e6fe2be8a2..fa3c834f77 100644 --- a/tests/Core/Filters/Filter/AcceptTest.php +++ b/tests/Core/Filters/Filter/AcceptTest.php @@ -14,6 +14,7 @@ use PHP_CodeSniffer\Filters\Filter; use PHP_CodeSniffer\Ruleset; use PHP_CodeSniffer\Tests\Core\Filters\AbstractFilterTestCase; +use RecursiveArrayIterator; /** * Tests for the \PHP_CodeSniffer\Filters\Filter::accept method. @@ -34,7 +35,7 @@ class AcceptTest extends AbstractFilterTestCase public static function initializeConfigAndRuleset() { $standard = __DIR__.'/'.basename(__FILE__, '.php').'.xml'; - self::$config = new Config(["--standard=$standard", "--ignore=*/somethingelse/*"]); + self::$config = new Config(["--standard=$standard", '--ignore=*/somethingelse/*', '--report-width=80']); self::$ruleset = new Ruleset(self::$config); }//end initializeConfigAndRuleset() @@ -52,7 +53,7 @@ public static function initializeConfigAndRuleset() */ public function testExcludePatterns($inputPaths, $expectedOutput) { - $fakeDI = new \RecursiveArrayIterator($inputPaths); + $fakeDI = new RecursiveArrayIterator($inputPaths); $filter = new Filter($fakeDI, '/', self::$config, self::$ruleset); $this->assertEquals($expectedOutput, $this->getFilteredResultsAsArray($filter)); @@ -67,7 +68,7 @@ public function testExcludePatterns($inputPaths, $expectedOutput) * * @return array>> */ - public function dataExcludePatterns() + public static function dataExcludePatterns() { $testCases = [ // Test top-level exclude patterns. From bea39038937c9b5637bf84b1bb63d05a42f99bff Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 31 Dec 2023 07:38:46 +0100 Subject: [PATCH 4/4] Filters: add tests for `GitModified` and `GitStaged` filters This is an initial set of tests for the `GitModified` and `GitStaged` filters. The tests are largely the same for both filters as the logic under test is also largely the same. Still, having separate test classes will allow for differentiating the tests if the logic in the filters themselves would start to diverge over time or if the output of the individual git commands would give reason to add extra tests. Notes: * To allow for mocking the output of the "git modified"/"git staged" commands, the handling of the function call to `exec` has been moved to a separate method in both the `GitModified`, as well as the `GitStaged` class. * This commit adds three new helper methods to the `AbstractFilterTestCase`: - `getMockedClass()` to handle creating a mock object of the filter under test in a PHPUnit cross-version compatible manner. - `getBaseDir()` to get the project root directory for use as the `$basedir` for the filters. - `getFakeFileList()` to get, as the name says, a fake file list for use in the tests, which contains a range of different file/directory situations which filter may need to take into account. Note: `.` and `..` are not included as the `PHP_CodeSniffer\Files\FileList` (which is basically what we're faking here) does not include those either, so that would create unrealistic test scenarios. Also note that, aside from some non-existent files added to the list for testing purposes, the files in this list _do_ actually need to exist for the tests to be valid. --- src/Filters/GitModified.php | 26 +- src/Filters/GitStaged.php | 26 +- tests/Core/Filters/AbstractFilterTestCase.php | 125 ++++++++- tests/Core/Filters/GitModifiedTest.php | 260 ++++++++++++++++++ tests/Core/Filters/GitStagedTest.php | 260 ++++++++++++++++++ 5 files changed, 692 insertions(+), 5 deletions(-) create mode 100644 tests/Core/Filters/GitModifiedTest.php create mode 100644 tests/Core/Filters/GitStagedTest.php diff --git a/src/Filters/GitModified.php b/src/Filters/GitModified.php index 9bba997c24..b946577909 100644 --- a/src/Filters/GitModified.php +++ b/src/Filters/GitModified.php @@ -37,8 +37,7 @@ protected function getWhitelist() $modified = []; $cmd = 'git ls-files -o -m --exclude-standard -- '.escapeshellarg($this->basedir); - $output = []; - exec($cmd, $output); + $output = $this->exec($cmd); $basedir = $this->basedir; if (is_dir($basedir) === false) { @@ -63,4 +62,27 @@ protected function getWhitelist() }//end getWhitelist() + /** + * Execute an external command. + * + * {@internal This method is only needed to allow for mocking the return value + * to test the class logic.} + * + * @param string $cmd Command. + * + * @return array + */ + protected function exec($cmd) + { + $output = []; + $lastLine = exec($cmd, $output); + if ($lastLine === false) { + return []; + } + + return $output; + + }//end exec() + + }//end class diff --git a/src/Filters/GitStaged.php b/src/Filters/GitStaged.php index 1fdbb952ca..405387ff87 100644 --- a/src/Filters/GitStaged.php +++ b/src/Filters/GitStaged.php @@ -39,8 +39,7 @@ protected function getWhitelist() $modified = []; $cmd = 'git diff --cached --name-only -- '.escapeshellarg($this->basedir); - $output = []; - exec($cmd, $output); + $output = $this->exec($cmd); $basedir = $this->basedir; if (is_dir($basedir) === false) { @@ -65,4 +64,27 @@ protected function getWhitelist() }//end getWhitelist() + /** + * Execute an external command. + * + * {@internal This method is only needed to allow for mocking the return value + * to test the class logic.} + * + * @param string $cmd Command. + * + * @return array + */ + protected function exec($cmd) + { + $output = []; + $lastLine = exec($cmd, $output); + if ($lastLine === false) { + return []; + } + + return $output; + + }//end exec() + + }//end class diff --git a/tests/Core/Filters/AbstractFilterTestCase.php b/tests/Core/Filters/AbstractFilterTestCase.php index a745a21481..b867d7aaf8 100644 --- a/tests/Core/Filters/AbstractFilterTestCase.php +++ b/tests/Core/Filters/AbstractFilterTestCase.php @@ -45,12 +45,56 @@ abstract class AbstractFilterTestCase extends TestCase */ public static function initializeConfigAndRuleset() { - self::$config = new Config(['--standard=PSR1', '--report-width=80']); + self::$config = new Config(['--standard=PSR1', '--extensions=php,inc/php,js,css', '--report-width=80']); self::$ruleset = new Ruleset(self::$config); }//end initializeConfigAndRuleset() + /** + * Helper method to retrieve a mock object for a Filter class. + * + * The `setMethods()` method was silently deprecated in PHPUnit 9 and removed in PHPUnit 10. + * + * Note: direct access to the `getMockBuilder()` method is soft deprecated as of PHPUnit 10, + * and expected to be hard deprecated in PHPUnit 11 and removed in PHPUnit 12. + * Dealing with that is something for a later iteration of the test suite. + * + * @param string $className Fully qualified name of the class under test. + * @param array $constructorArgs Optional. Array of parameters to pass to the class constructor. + * @param array|null $methodsToMock Optional. The methods to mock in the class under test. + * Needed for PHPUnit cross-version support as PHPUnit 4.x does + * not have a `setMethodsExcept()` method yet. + * If not passed, no methods will be replaced. + * + * @return \PHPUnit\Framework\MockObject\MockObject + */ + protected function getMockedClass($className, array $constructorArgs=[], $methodsToMock=null) + { + $mockedObj = $this->getMockBuilder($className); + + if (\method_exists($mockedObj, 'onlyMethods') === true) { + // PHPUnit 8+. + if (is_array($methodsToMock) === true) { + return $mockedObj + ->setConstructorArgs($constructorArgs) + ->onlyMethods($methodsToMock) + ->getMock(); + } + + return $mockedObj->getMock() + ->setConstructorArgs($constructorArgs); + } + + // PHPUnit < 8. + return $mockedObj + ->setConstructorArgs($constructorArgs) + ->setMethods($methodsToMock) + ->getMock(); + + }//end getMockedClass() + + /** * Retrieve an array of files which were accepted by a filter. * @@ -71,6 +115,85 @@ protected function getFilteredResultsAsArray(Filter $filter) }//end getFilteredResultsAsArray() + /** + * Retrieve the basedir to use for tests using the `getFakeFileList()` method. + * + * @return string + */ + protected static function getBaseDir() + { + return dirname(dirname(dirname(__DIR__))); + + }//end getBaseDir() + + + /** + * Retrieve a file list containing a range of paths for testing purposes. + * + * This list **must** contain files which exist in this project (well, except for some which don't exist + * purely for testing purposes), as `realpath()` is used in the logic under test and `realpath()` will + * return `false` for any non-existent files, which will automatically filter them out before + * we get to the code under test. + * + * Note this list does not include `.` and `..` as \PHP_CodeSniffer\Files\FileList uses `SKIP_DOTS`. + * + * @return array + */ + protected static function getFakeFileList() + { + $basedir = self::getBaseDir(); + return [ + $basedir.'/.gitignore', + $basedir.'/.yamllint.yml', + $basedir.'/phpcs.xml', + $basedir.'/phpcs.xml.dist', + $basedir.'/autoload.php', + $basedir.'/bin', + $basedir.'/bin/phpcs', + $basedir.'/bin/phpcs.bat', + $basedir.'/scripts', + $basedir.'/scripts/build-phar.php', + $basedir.'/src', + $basedir.'/src/WillNotExist.php', + $basedir.'/src/WillNotExist.bak', + $basedir.'/src/WillNotExist.orig', + $basedir.'/src/Ruleset.php', + $basedir.'/src/Generators', + $basedir.'/src/Generators/Markdown.php', + $basedir.'/src/Standards', + $basedir.'/src/Standards/Generic', + $basedir.'/src/Standards/Generic/Docs', + $basedir.'/src/Standards/Generic/Docs/Classes', + $basedir.'/src/Standards/Generic/Docs/Classes/DuplicateClassNameStandard.xml', + $basedir.'/src/Standards/Generic/Sniffs', + $basedir.'/src/Standards/Generic/Sniffs/Classes', + $basedir.'/src/Standards/Generic/Sniffs/Classes/DuplicateClassNameSniff.php', + $basedir.'/src/Standards/Generic/Tests', + $basedir.'/src/Standards/Generic/Tests/Classes', + $basedir.'/src/Standards/Generic/Tests/Classes/DuplicateClassNameUnitTest.1.inc', + // Will rarely exist when running the tests. + $basedir.'/src/Standards/Generic/Tests/Classes/DuplicateClassNameUnitTest.1.inc.bak', + $basedir.'/src/Standards/Generic/Tests/Classes/DuplicateClassNameUnitTest.2.inc', + $basedir.'/src/Standards/Generic/Tests/Classes/DuplicateClassNameUnitTest.php', + $basedir.'/src/Standards/Squiz', + $basedir.'/src/Standards/Squiz/Docs', + $basedir.'/src/Standards/Squiz/Docs/WhiteSpace', + $basedir.'/src/Standards/Squiz/Docs/WhiteSpace/SemicolonSpacingStandard.xml', + $basedir.'/src/Standards/Squiz/Sniffs', + $basedir.'/src/Standards/Squiz/Sniffs/WhiteSpace', + $basedir.'/src/Standards/Squiz/Sniffs/WhiteSpace/OperatorSpacingSniff.php', + $basedir.'/src/Standards/Squiz/Tests', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.inc', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.inc.fixed', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.js', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.js.fixed', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.php', + ]; + + }//end getFakeFileList() + + /** * Translate Linux paths to Windows paths, when necessary. * diff --git a/tests/Core/Filters/GitModifiedTest.php b/tests/Core/Filters/GitModifiedTest.php new file mode 100644 index 0000000000..b3d11b631f --- /dev/null +++ b/tests/Core/Filters/GitModifiedTest.php @@ -0,0 +1,260 @@ + + * @copyright 2023 PHPCSStandards Contributors + * @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence + */ + +namespace PHP_CodeSniffer\Tests\Core\Filters; + +use PHP_CodeSniffer\Filters\GitModified; +use PHP_CodeSniffer\Tests\Core\Filters\AbstractFilterTestCase; +use RecursiveArrayIterator; +use ReflectionMethod; + +/** + * Tests for the \PHP_CodeSniffer\Filters\GitModified class. + * + * @covers \PHP_CodeSniffer\Filters\GitModified + */ +class GitModifiedTest extends AbstractFilterTestCase +{ + + + /** + * Test filtering a file list for excluded paths. + * + * @return void + */ + public function testFileNamePassesAsBasePathWillTranslateToDirname() + { + $rootFile = self::getBaseDir().'/autoload.php'; + + $fakeDI = new RecursiveArrayIterator(self::getFakeFileList()); + $constructorArgs = [ + $fakeDI, + $rootFile, + self::$config, + self::$ruleset, + ]; + $mockObj = $this->getMockedClass('PHP_CodeSniffer\Filters\GitModified', $constructorArgs, ['exec']); + + $mockObj->expects($this->once()) + ->method('exec') + ->willReturn(['autoload.php']); + + $this->assertEquals([$rootFile], $this->getFilteredResultsAsArray($mockObj)); + + }//end testFileNamePassesAsBasePathWillTranslateToDirname() + + + /** + * Test filtering a file list for excluded paths. + * + * @param array $inputPaths List of file paths to be filtered. + * @param array $outputGitModified Simulated "git modified" output. + * @param array $expectedOutput Expected filtering result. + * + * @dataProvider dataAcceptOnlyGitModified + * + * @return void + */ + public function testAcceptOnlyGitModified($inputPaths, $outputGitModified, $expectedOutput) + { + $fakeDI = new RecursiveArrayIterator($inputPaths); + $constructorArgs = [ + $fakeDI, + self::getBaseDir(), + self::$config, + self::$ruleset, + ]; + $mockObj = $this->getMockedClass('PHP_CodeSniffer\Filters\GitModified', $constructorArgs, ['exec']); + + $mockObj->expects($this->once()) + ->method('exec') + ->willReturn($outputGitModified); + + $this->assertEquals($expectedOutput, $this->getFilteredResultsAsArray($mockObj)); + + }//end testAcceptOnlyGitModified() + + + /** + * Data provider. + * + * @see testAcceptOnlyGitModified + * + * @return array>> + */ + public static function dataAcceptOnlyGitModified() + { + $basedir = self::getBaseDir(); + $fakeFileList = self::getFakeFileList(); + + $testCases = [ + 'no files marked as git modified' => [ + 'inputPaths' => $fakeFileList, + 'outputGitModified' => [], + 'expectedOutput' => [], + ], + + 'files marked as git modified which don\'t actually exist' => [ + 'inputPaths' => $fakeFileList, + 'outputGitModified' => [ + 'src/WillNotExist.php', + 'src/WillNotExist.bak', + 'src/WillNotExist.orig', + ], + 'expectedOutput' => [], + ], + + 'single file marked as git modified - file in root dir' => [ + 'inputPaths' => $fakeFileList, + 'outputGitModified' => ['autoload.php'], + 'expectedOutput' => [ + $basedir.'/autoload.php', + ], + ], + 'single file marked as git modified - file in sub dir' => [ + 'inputPaths' => $fakeFileList, + 'outputGitModified' => ['src/Standards/Generic/Sniffs/Classes/DuplicateClassNameSniff.php'], + 'expectedOutput' => [ + $basedir.'/src', + $basedir.'/src/Standards', + $basedir.'/src/Standards/Generic', + $basedir.'/src/Standards/Generic/Sniffs', + $basedir.'/src/Standards/Generic/Sniffs/Classes', + $basedir.'/src/Standards/Generic/Sniffs/Classes/DuplicateClassNameSniff.php', + ], + ], + + 'multiple files marked as git modified, none valid for scan' => [ + 'inputPaths' => $fakeFileList, + 'outputGitModified' => [ + '.gitignore', + 'phpcs.xml.dist', + 'src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.js.fixed', + ], + 'expectedOutput' => [ + $basedir.'/src', + $basedir.'/src/Standards', + $basedir.'/src/Standards/Squiz', + $basedir.'/src/Standards/Squiz/Tests', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace', + ], + ], + + 'multiple files marked as git modified, only one file valid for scan' => [ + 'inputPaths' => $fakeFileList, + 'outputGitModified' => [ + '.gitignore', + 'src/Standards/Generic/Docs/Classes/DuplicateClassNameStandard.xml', + 'src/Standards/Generic/Sniffs/Classes/DuplicateClassNameSniff.php', + ], + 'expectedOutput' => [ + $basedir.'/src', + $basedir.'/src/Standards', + $basedir.'/src/Standards/Generic', + $basedir.'/src/Standards/Generic/Docs', + $basedir.'/src/Standards/Generic/Docs/Classes', + $basedir.'/src/Standards/Generic/Sniffs', + $basedir.'/src/Standards/Generic/Sniffs/Classes', + $basedir.'/src/Standards/Generic/Sniffs/Classes/DuplicateClassNameSniff.php', + ], + ], + + 'multiple files marked as git modified, multiple files valid for scan' => [ + 'inputPaths' => $fakeFileList, + 'outputGitModified' => [ + '.yamllint.yml', + 'autoload.php', + 'src/Standards/Squiz/Sniffs/WhiteSpace/OperatorSpacingSniff.php', + 'src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.inc', + 'src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.inc.fixed', + 'src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.js', + 'src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.js.fixed', + 'src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.php', + ], + 'expectedOutput' => [ + $basedir.'/autoload.php', + $basedir.'/src', + $basedir.'/src/Standards', + $basedir.'/src/Standards/Squiz', + $basedir.'/src/Standards/Squiz/Sniffs', + $basedir.'/src/Standards/Squiz/Sniffs/WhiteSpace', + $basedir.'/src/Standards/Squiz/Sniffs/WhiteSpace/OperatorSpacingSniff.php', + $basedir.'/src/Standards/Squiz/Tests', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.inc', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.js', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.php', + ], + ], + ]; + + return $testCases; + + }//end dataAcceptOnlyGitModified() + + + /** + * Test filtering a file list for excluded paths. + * + * @param string $cmd Command to run. + * @param array $expected Expected return value. + * + * @dataProvider dataExecAlwaysReturnsArray + * + * @return void + */ + public function testExecAlwaysReturnsArray($cmd, $expected) + { + $fakeDI = new RecursiveArrayIterator(self::getFakeFileList()); + $filter = new GitModified($fakeDI, '/', self::$config, self::$ruleset); + + $reflMethod = new ReflectionMethod($filter, 'exec'); + $reflMethod->setAccessible(true); + $result = $reflMethod->invoke($filter, $cmd); + + $this->assertSame($expected, $result); + + }//end testExecAlwaysReturnsArray() + + + /** + * Data provider. + * + * @see testExecAlwaysReturnsArray + * + * {@internal Missing: test with a command which yields a `false` return value. + * JRF: I've not managed to find a command which does so, let alone one, which then + * doesn't have side-effects of uncatchable output while running the tests.} + * + * @return array>> + */ + public static function dataExecAlwaysReturnsArray() + { + return [ + 'valid command which won\'t have any output unless files in the bin dir have been modified' => [ + // Largely using the command used in the filter, but only checking the bin dir. + // This should prevent the test unexpectedly failing during local development (in most cases). + 'cmd' => 'git ls-files -o -m --exclude-standard -- '.escapeshellarg(self::getBaseDir().'/bin'), + 'expected' => [], + ], + 'valid command which will have output' => [ + 'cmd' => 'git ls-files --exclude-standard -- '.escapeshellarg(self::getBaseDir().'/bin'), + 'expected' => [ + 'bin/phpcbf', + 'bin/phpcbf.bat', + 'bin/phpcs', + 'bin/phpcs.bat', + ], + ], + ]; + + }//end dataExecAlwaysReturnsArray() + + +}//end class diff --git a/tests/Core/Filters/GitStagedTest.php b/tests/Core/Filters/GitStagedTest.php new file mode 100644 index 0000000000..ebf761c4fe --- /dev/null +++ b/tests/Core/Filters/GitStagedTest.php @@ -0,0 +1,260 @@ + + * @copyright 2023 PHPCSStandards Contributors + * @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence + */ + +namespace PHP_CodeSniffer\Tests\Core\Filters; + +use PHP_CodeSniffer\Filters\GitStaged; +use PHP_CodeSniffer\Tests\Core\Filters\AbstractFilterTestCase; +use RecursiveArrayIterator; +use ReflectionMethod; + +/** + * Tests for the \PHP_CodeSniffer\Filters\GitStaged class. + * + * @covers \PHP_CodeSniffer\Filters\GitStaged + */ +class GitStagedTest extends AbstractFilterTestCase +{ + + + /** + * Test filtering a file list for excluded paths. + * + * @return void + */ + public function testFileNamePassesAsBasePathWillTranslateToDirname() + { + $rootFile = self::getBaseDir().'/autoload.php'; + + $fakeDI = new RecursiveArrayIterator(self::getFakeFileList()); + $constructorArgs = [ + $fakeDI, + $rootFile, + self::$config, + self::$ruleset, + ]; + $mockObj = $this->getMockedClass('PHP_CodeSniffer\Filters\GitStaged', $constructorArgs, ['exec']); + + $mockObj->expects($this->once()) + ->method('exec') + ->willReturn(['autoload.php']); + + $this->assertEquals([$rootFile], $this->getFilteredResultsAsArray($mockObj)); + + }//end testFileNamePassesAsBasePathWillTranslateToDirname() + + + /** + * Test filtering a file list for excluded paths. + * + * @param array $inputPaths List of file paths to be filtered. + * @param array $outputGitStaged Simulated "git staged" output. + * @param array $expectedOutput Expected filtering result. + * + * @dataProvider dataAcceptOnlyGitStaged + * + * @return void + */ + public function testAcceptOnlyGitStaged($inputPaths, $outputGitStaged, $expectedOutput) + { + $fakeDI = new RecursiveArrayIterator($inputPaths); + $constructorArgs = [ + $fakeDI, + self::getBaseDir(), + self::$config, + self::$ruleset, + ]; + $mockObj = $this->getMockedClass('PHP_CodeSniffer\Filters\GitStaged', $constructorArgs, ['exec']); + + $mockObj->expects($this->once()) + ->method('exec') + ->willReturn($outputGitStaged); + + $this->assertEquals($expectedOutput, $this->getFilteredResultsAsArray($mockObj)); + + }//end testAcceptOnlyGitStaged() + + + /** + * Data provider. + * + * @see testAcceptOnlyGitStaged + * + * @return array>> + */ + public static function dataAcceptOnlyGitStaged() + { + $basedir = self::getBaseDir(); + $fakeFileList = self::getFakeFileList(); + + $testCases = [ + 'no files marked as git modified' => [ + 'inputPaths' => $fakeFileList, + 'outputGitStaged' => [], + 'expectedOutput' => [], + ], + + 'files marked as git modified which don\'t actually exist' => [ + 'inputPaths' => $fakeFileList, + 'outputGitStaged' => [ + 'src/WillNotExist.php', + 'src/WillNotExist.bak', + 'src/WillNotExist.orig', + ], + 'expectedOutput' => [], + ], + + 'single file marked as git modified - file in root dir' => [ + 'inputPaths' => $fakeFileList, + 'outputGitStaged' => ['autoload.php'], + 'expectedOutput' => [ + $basedir.'/autoload.php', + ], + ], + 'single file marked as git modified - file in sub dir' => [ + 'inputPaths' => $fakeFileList, + 'outputGitStaged' => ['src/Standards/Generic/Sniffs/Classes/DuplicateClassNameSniff.php'], + 'expectedOutput' => [ + $basedir.'/src', + $basedir.'/src/Standards', + $basedir.'/src/Standards/Generic', + $basedir.'/src/Standards/Generic/Sniffs', + $basedir.'/src/Standards/Generic/Sniffs/Classes', + $basedir.'/src/Standards/Generic/Sniffs/Classes/DuplicateClassNameSniff.php', + ], + ], + + 'multiple files marked as git modified, none valid for scan' => [ + 'inputPaths' => $fakeFileList, + 'outputGitStaged' => [ + '.gitignore', + 'phpcs.xml.dist', + 'src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.js.fixed', + ], + 'expectedOutput' => [ + $basedir.'/src', + $basedir.'/src/Standards', + $basedir.'/src/Standards/Squiz', + $basedir.'/src/Standards/Squiz/Tests', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace', + ], + ], + + 'multiple files marked as git modified, only one file valid for scan' => [ + 'inputPaths' => $fakeFileList, + 'outputGitStaged' => [ + '.gitignore', + 'src/Standards/Generic/Docs/Classes/DuplicateClassNameStandard.xml', + 'src/Standards/Generic/Sniffs/Classes/DuplicateClassNameSniff.php', + ], + 'expectedOutput' => [ + $basedir.'/src', + $basedir.'/src/Standards', + $basedir.'/src/Standards/Generic', + $basedir.'/src/Standards/Generic/Docs', + $basedir.'/src/Standards/Generic/Docs/Classes', + $basedir.'/src/Standards/Generic/Sniffs', + $basedir.'/src/Standards/Generic/Sniffs/Classes', + $basedir.'/src/Standards/Generic/Sniffs/Classes/DuplicateClassNameSniff.php', + ], + ], + + 'multiple files marked as git modified, multiple files valid for scan' => [ + 'inputPaths' => $fakeFileList, + 'outputGitStaged' => [ + '.yamllint.yml', + 'autoload.php', + 'src/Standards/Squiz/Sniffs/WhiteSpace/OperatorSpacingSniff.php', + 'src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.inc', + 'src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.inc.fixed', + 'src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.js', + 'src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.js.fixed', + 'src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.php', + ], + 'expectedOutput' => [ + $basedir.'/autoload.php', + $basedir.'/src', + $basedir.'/src/Standards', + $basedir.'/src/Standards/Squiz', + $basedir.'/src/Standards/Squiz/Sniffs', + $basedir.'/src/Standards/Squiz/Sniffs/WhiteSpace', + $basedir.'/src/Standards/Squiz/Sniffs/WhiteSpace/OperatorSpacingSniff.php', + $basedir.'/src/Standards/Squiz/Tests', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.inc', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.js', + $basedir.'/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.php', + ], + ], + ]; + + return $testCases; + + }//end dataAcceptOnlyGitStaged() + + + /** + * Test filtering a file list for excluded paths. + * + * @param string $cmd Command to run. + * @param array $expected Expected return value. + * + * @dataProvider dataExecAlwaysReturnsArray + * + * @return void + */ + public function testExecAlwaysReturnsArray($cmd, $expected) + { + $fakeDI = new RecursiveArrayIterator(self::getFakeFileList()); + $filter = new GitStaged($fakeDI, '/', self::$config, self::$ruleset); + + $reflMethod = new ReflectionMethod($filter, 'exec'); + $reflMethod->setAccessible(true); + $result = $reflMethod->invoke($filter, $cmd); + + $this->assertSame($expected, $result); + + }//end testExecAlwaysReturnsArray() + + + /** + * Data provider. + * + * @see testExecAlwaysReturnsArray + * + * {@internal Missing: test with a command which yields a `false` return value. + * JRF: I've not managed to find a command which does so, let alone one, which then + * doesn't have side-effects of uncatchable output while running the tests.} + * + * @return array>> + */ + public static function dataExecAlwaysReturnsArray() + { + return [ + 'valid command which won\'t have any output unless files in the bin dir have been modified & staged' => [ + // Largely using the command used in the filter, but only checking the bin dir. + // This should prevent the test unexpectedly failing during local development (in most cases). + 'cmd' => 'git diff --cached --name-only -- '.escapeshellarg(self::getBaseDir().'/bin'), + 'expected' => [], + ], + 'valid command which will have output' => [ + 'cmd' => 'git ls-files --exclude-standard -- '.escapeshellarg(self::getBaseDir().'/bin'), + 'expected' => [ + 'bin/phpcbf', + 'bin/phpcbf.bat', + 'bin/phpcs', + 'bin/phpcs.bat', + ], + ], + ]; + + }//end dataExecAlwaysReturnsArray() + + +}//end class