From be8be3dd386feb0003ad99f494e8a2ab8da2ffa7 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Mon, 1 Apr 2024 21:40:25 +0800 Subject: [PATCH 1/4] Run tests on Windows too (#146) --- .github/workflows/phpcs.yml | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/.github/workflows/phpcs.yml b/.github/workflows/phpcs.yml index e8a52ed..277d046 100644 --- a/.github/workflows/phpcs.yml +++ b/.github/workflows/phpcs.yml @@ -4,18 +4,26 @@ on: [push, pull_request] jobs: test: - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} strategy: fail-fast: false matrix: - include: - - php: 8.3 - - php: 8.2 - - php: 8.1 - - php: 8.0 - - php: 7.4 + php: + - 8.3 + - 8.2 + - 8.1 + - 8.0 + - 7.4 + os: + - ubuntu-latest + - windows-latest steps: + - name: Set git to use LF + run: | + git config --global core.autocrlf false + git config --global core.eol lf + - name: Check out repository code uses: actions/checkout@v4 @@ -55,7 +63,7 @@ jobs: run: ./vendor/bin/phpunit-coverage-check -t 80 clover.xml - name: Integration tests - if: ${{ !cancelled() }} + if: ${{ (!cancelled()) && (runner.os == 'ubuntu-latest') }} run: | # There is one failure (exit with error) vendor/bin/phpcs --standard=moodle moodle/Tests/fixtures/integration_test_ci.php | tee output.txt || [[ $? = 1 ]] From 5529ded73024653c0b47b4974657b67782b3fd11 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Mon, 1 Apr 2024 21:41:32 +0800 Subject: [PATCH 2/4] Add @group test (#146) --- moodle/Tests/Sniffs/Commenting/ValidTagsSniffTest.php | 10 +++++----- .../Sniffs/Commenting/fixtures/ValidTags/unit_test.php | 3 +++ .../Commenting/fixtures/ValidTags/unit_test.php.fixed | 3 +++ 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/moodle/Tests/Sniffs/Commenting/ValidTagsSniffTest.php b/moodle/Tests/Sniffs/Commenting/ValidTagsSniffTest.php index f61b86a..b4b3aac 100644 --- a/moodle/Tests/Sniffs/Commenting/ValidTagsSniffTest.php +++ b/moodle/Tests/Sniffs/Commenting/ValidTagsSniffTest.php @@ -55,11 +55,11 @@ public static function provider(): array { 'fixturePath' => 'lib/tests/example_test.php', 'fixtureSource' => 'unit_test', 'errors' => [ - 11 => 'Incorrect docblock tag "@returns". Should be "@return"', - 12 => 'Invalid docblock tag "@void"', - 55 => 'Invalid docblock tag "@small"', - 56 => 'Invalid docblock tag "@zzzing"', - 57 => 'Invalid docblock tag "@inheritdoc"', + 12 => 'Incorrect docblock tag "@returns". Should be "@return"', + 13 => 'Invalid docblock tag "@void"', + 58 => 'Invalid docblock tag "@small"', + 59 => 'Invalid docblock tag "@zzzing"', + 60 => 'Invalid docblock tag "@inheritdoc"', ], 'warnings' => [], ], diff --git a/moodle/Tests/Sniffs/Commenting/fixtures/ValidTags/unit_test.php b/moodle/Tests/Sniffs/Commenting/fixtures/ValidTags/unit_test.php index 01cd04d..f9ab4a5 100644 --- a/moodle/Tests/Sniffs/Commenting/fixtures/ValidTags/unit_test.php +++ b/moodle/Tests/Sniffs/Commenting/fixtures/ValidTags/unit_test.php @@ -2,6 +2,7 @@ /** * @covers \some_class + * @group some_group */ class some_test extends \advanced_testcase { /** @@ -42,6 +43,8 @@ public function also_all_valid_tags() { /** * Some more valid tags, because we are under tests area. + * + * @group some_group */ public function valid_inline_tags() { // @codeCoverageIgnoreStart diff --git a/moodle/Tests/Sniffs/Commenting/fixtures/ValidTags/unit_test.php.fixed b/moodle/Tests/Sniffs/Commenting/fixtures/ValidTags/unit_test.php.fixed index a9aac37..93d3062 100644 --- a/moodle/Tests/Sniffs/Commenting/fixtures/ValidTags/unit_test.php.fixed +++ b/moodle/Tests/Sniffs/Commenting/fixtures/ValidTags/unit_test.php.fixed @@ -2,6 +2,7 @@ /** * @covers \some_class + * @group some_group */ class some_test extends \advanced_testcase { /** @@ -41,6 +42,8 @@ class some_test extends \advanced_testcase { /** * Some more valid tags, because we are under tests area. + * + * @group some_group */ public function valid_inline_tags() { // @codeCoverageIgnoreStart From 61d331120bc9e131bcd08427674e80c9ea545350 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Mon, 1 Apr 2024 21:49:43 +0800 Subject: [PATCH 3/4] Docblocks util properties should be private (#146) --- moodle/Util/Docblocks.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/moodle/Util/Docblocks.php b/moodle/Util/Docblocks.php index 15c4f57..bd4fe0b 100644 --- a/moodle/Util/Docblocks.php +++ b/moodle/Util/Docblocks.php @@ -32,7 +32,7 @@ abstract class Docblocks * * @var array * @link http://manual.phpdoc.org/HTMLSmartyConverter/HandS/ */ - public static array $validTags = [ + private static array $validTags = [ // Behat tags. 'Given' => true, 'Then' => true, @@ -100,7 +100,7 @@ abstract class Docblocks * * @var string[] */ - public static array $invalidTagsToRemove = [ + private static array $invalidTagsToRemove = [ 'void', ]; @@ -109,7 +109,7 @@ abstract class Docblocks * * @var string[string] */ - public static array $renameTags = [ + private static array $renameTags = [ // Rename returns to return. 'returns' => 'return', ]; @@ -120,7 +120,7 @@ abstract class Docblocks * * @var array(string => array(string)) */ - public static array $pathRestrictedTags = [ + private static array $pathRestrictedTags = [ 'Given' => ['#.*/tests/behat/.*#'], 'Then' => ['#.*/tests/behat/.*#'], 'When' => ['#.*/tests/behat/.*#'], From 28453891f7ecc6ae57c0968bc0c40281497af522 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Mon, 1 Apr 2024 22:04:42 +0800 Subject: [PATCH 4/4] Standardise filepath separators to / (Fixes #145 #146) --- moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php | 5 ++-- moodle/Tests/MoodleCSBaseTestCase.php | 6 +++- moodle/Util/Docblocks.php | 2 +- moodle/Util/MoodleUtil.php | 29 +++++++++++++++----- 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php b/moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php index 5e81c37..05f7575 100644 --- a/moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php +++ b/moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php @@ -233,9 +233,10 @@ public function process(File $file, $pointer) { if ($bspos !== false) { // Only if there are level2 and down namespace. $relns = str_replace('\\', '/', substr(trim($namespace, ' \\'), $bspos + 1)); + $filename = MoodleUtil::getStandardisedFilename($file); // Calculate the relative path under tests directory. - $dirpos = strripos(trim(dirname($file->getFilename()), ' /') . '/', '/tests/'); - $reldir = str_replace('\\', '/', substr(trim(dirname($file->getFilename()), ' /'), $dirpos + 7)); + $dirpos = strripos(trim(dirname($filename), ' /') . '/', '/tests/'); + $reldir = str_replace('\\', '/', substr(trim(dirname($filename), ' /'), $dirpos + 7)); // Warning if the relative namespace does not match the relative directory. if ($reldir !== $relns) { diff --git a/moodle/Tests/MoodleCSBaseTestCase.php b/moodle/Tests/MoodleCSBaseTestCase.php index 88325eb..baaf1fc 100644 --- a/moodle/Tests/MoodleCSBaseTestCase.php +++ b/moodle/Tests/MoodleCSBaseTestCase.php @@ -221,9 +221,13 @@ protected function verifyCsResults() { // Let's process the fixture. try { if ($this->fixtureFileName !== null) { + $fixtureFilename = $this->fixtureFileName; + if (DIRECTORY_SEPARATOR !== '/') { + $fixtureFilename = str_replace('/', DIRECTORY_SEPARATOR, $fixtureFilename); + } $fixtureSource = file_get_contents($this->fixture); $fixtureContent = <<fixtureFileName} + phpcs_input_file: {$fixtureFilename} {$fixtureSource} EOF; $phpcsfile = new \PHP_CodeSniffer\Files\DummyFile($fixtureContent, $ruleset, $config); diff --git a/moodle/Util/Docblocks.php b/moodle/Util/Docblocks.php index bd4fe0b..6b86cb7 100644 --- a/moodle/Util/Docblocks.php +++ b/moodle/Util/Docblocks.php @@ -324,7 +324,7 @@ public static function isValidTag( $tag = ltrim($tokens[$tagPtr]['content'], '@'); if (array_key_exists($tag, self::$validTags)) { if (array_key_exists($tag, self::$pathRestrictedTags)) { - $file = $phpcsFile->getFilename(); + $file = MoodleUtil::getStandardisedFilename($phpcsFile); foreach (self::$pathRestrictedTags[$tag] as $path) { if (preg_match($path, $file)) { return true; diff --git a/moodle/Util/MoodleUtil.php b/moodle/Util/MoodleUtil.php index 045d3ed..4af8457 100644 --- a/moodle/Util/MoodleUtil.php +++ b/moodle/Util/MoodleUtil.php @@ -242,14 +242,17 @@ public static function getMoodleComponent(File $file, $selfPath = true): ?string } } + $filepath = MoodleUtil::getStandardisedFilename($file); // Let's find the first component that matches the file path. foreach ($components as $component => $componentPath) { // Only components with path. if (empty($componentPath)) { continue; } + // Look for component paths matching the file path. - if (strpos($file->path, $componentPath . '/') === 0) { + $componentPath = str_replace('\\', '/', $componentPath . DIRECTORY_SEPARATOR); + if (strpos($filepath, $componentPath) === 0) { // First match found should be the better one always. We are done. return $component; } @@ -450,14 +453,15 @@ public static function getMoodleRoot(?File $file = null, bool $selfPath = true): */ public static function isLangFile(File $phpcsFile): bool { + $filename = MoodleUtil::getStandardisedFilename($phpcsFile); // If the file is not under a /lang/[a-zA-Z0-9_-]+/ directory, nothing to check. // (note that we are using that regex because it's what PARAM_LANG does). - if (preg_match('~/lang/[a-zA-Z0-9_-]+/~', $phpcsFile->getFilename()) === 0) { + if (preg_match('~/lang/[a-zA-Z0-9_-]+/~', $filename) === 0) { return false; } // If the file is not a PHP file, nothing to check. - if (substr($phpcsFile->getFilename(), -4) !== '.php') { + if (substr($filename, -4) !== '.php') { return false; } @@ -476,28 +480,29 @@ public static function isLangFile(File $phpcsFile): bool */ public static function isUnitTest(File $phpcsFile): bool { + $filename = MoodleUtil::getStandardisedFilename($phpcsFile); // If the file isn't called, _test.php, nothing to check. if (stripos(basename($phpcsFile->getFilename()), '_test.php') === false) { return false; } // If the file isn't under tests directory, nothing to check. - if (stripos($phpcsFile->getFilename(), '/tests/') === false) { + if (stripos($filename, '/tests/') === false) { return false; } // If the file is in a fixture directory, ignore it. - if (stripos($phpcsFile->getFilename(), '/tests/fixtures/') !== false) { + if (stripos($filename, '/tests/fixtures/') !== false) { return false; } // If the file is in a generator directory, ignore it. - if (stripos($phpcsFile->getFilename(), '/tests/generator/') !== false) { + if (stripos($filename, '/tests/generator/') !== false) { return false; } // If the file is in a behat directory, ignore it. - if (stripos($phpcsFile->getFilename(), '/tests/behat/') !== false) { + if (stripos($filename, '/tests/behat/') !== false) { return false; } @@ -609,4 +614,14 @@ public static function getTokensOnLine( ARRAY_FILTER_USE_BOTH ); } + + /** + * Get the standardised filename for the file. + * + * @param File @phpcsFile + * @return string + */ + public static function getStandardisedFilename(File $phpcsFile): string { + return str_replace('\\', '/', $phpcsFile->getFilename()); + } }