From bed42db81566e9e25f62fffb974a62304aaef724 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 17 Nov 2024 18:56:15 +0100 Subject: [PATCH] Tests: set up mechanism to allow for testing CBF specific code See the description of how this works in the `CONTRIBUTING` file for more information. Includes adjustments to the GH Actions workflows to ensure the CBF specific tests are: * Always run for the `quicktest` and "normal" test runs. * Run for code coverage and that the code coverage reports are send in a way that they can be merged correctly. Includes adding a "requires CS mode" condition to a few tests which would otherwise fail in CBF mode. --- .github/CONTRIBUTING.md | 30 ++++++++++++ .github/workflows/quicktest.yml | 5 ++ .github/workflows/test.yml | 32 ++++++++++++- phpunit.xml.dist | 10 ++++ tests/Core/Generators/GeneratorTest.php | 4 ++ tests/Core/Ruleset/ExplainTest.php | 4 ++ .../Ruleset/ShowSniffDeprecationsTest.php | 48 +++++++++++++++++-- tests/bootstrap.php | 21 +++++++- 8 files changed, 147 insertions(+), 7 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 27b1aa42ba..8dd294c50c 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -346,6 +346,36 @@ However, there are a few places which include OS-specific conditions, most notab Tests which cover code which have Windows specific conditions should be marked with a `@group Windows` annotation to allow for running those tests separately/selectively in CI. +#### Tests covering code which has CS/CBF specific behaviour + +There are a few places in PHPCS where code uses a global `PHP_CODESNIFFER_CBF` constant to determine what to do. +This makes testing this code more complicated. + +Tests which will only work correctly when `PHP_CODESNIFFER_CBF === false` should get the following test skip condition at the top of the test method: +```php +if (PHP_CODESNIFFER_CBF === true) { + $this->markTestSkipped('This test needs CS mode to run'); +} +``` + +Tests which are specifically intended to cover code run when `PHP_CODESNIFFER_CBF === true` should: +1. Be annotated with `@group CBF`. +2. Have a test skip condition at the top of the test method like so: + ```php + if (PHP_CODESNIFFER_CBF === false) { + $this->markTestSkipped('This test needs CBF mode to run'); + } + ``` + +By default, the tests are run with the `PHP_CODESNIFFER_CBF` constant set to `false` and tests in the `@group CBF` will not be run. + +To run the tests specific to the use of `PHP_CODESNIFFER_CBF === true`: +1. Set `` in a `phpunit.xml` file or set the ENV variable on an OS-level. +2. Run the tests like so: + ```bash + vendor/bin/phpunit --group CBF --exclude-group nothing + ``` + ### Submitting Your Pull Request diff --git a/.github/workflows/quicktest.yml b/.github/workflows/quicktest.yml index ed24bec539..5edde2a41a 100644 --- a/.github/workflows/quicktest.yml +++ b/.github/workflows/quicktest.yml @@ -76,6 +76,11 @@ jobs: if: ${{ matrix.os == 'windows-latest' }} run: php "vendor/bin/phpunit" tests/AllTests.php --group Windows --no-coverage + - name: 'PHPUnit: run select tests in CBF mode' + run: php "vendor/bin/phpunit" tests/AllTests.php --group CBF --exclude-group nothing --no-coverage + env: + PHP_CODESNIFFER_CBF: '1' + # Note: The code style check is run as an integration test. - name: 'PHPCS: check code style without cache, no parallel' run: php "bin/phpcs" --no-cache --parallel=1 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0113b7e8a7..e84fad9571 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -183,6 +183,11 @@ jobs: if: ${{ matrix.skip_tests != true }} run: php "vendor/bin/phpunit" tests/AllTests.php --no-coverage + - name: 'PHPUnit: run select tests in CBF mode' + run: php "vendor/bin/phpunit" tests/AllTests.php --group CBF --exclude-group nothing --no-coverage + env: + PHP_CODESNIFFER_CBF: '1' + - name: 'PHPCS: check code style without cache, no parallel' if: ${{ matrix.custom_ini == false && matrix.php != '7.4' }} run: php "bin/phpcs" --no-cache --parallel=1 @@ -311,6 +316,22 @@ jobs: if: ${{ matrix.os != 'windows-latest' && steps.phpunit_version.outputs.VERSION >= '9.3' }} run: php "vendor/bin/phpunit" tests/AllTests.php --coverage-cache ./build/phpunit-cache + - name: "Run select tests in CBF mode with code coverage (PHPUnit < 9.3)" + if: ${{ matrix.os != 'windows-latest' && steps.phpunit_version.outputs.VERSION < '9.3' }} + run: > + php "vendor/bin/phpunit" tests/AllTests.php + --group CBF --exclude-group nothing --coverage-clover build/logs/clover-cbf.xml + env: + PHP_CODESNIFFER_CBF: '1' + + - name: "Run select tests in CBF mode with code coverage (PHPUnit 9.3+)" + if: ${{ matrix.os != 'windows-latest' && steps.phpunit_version.outputs.VERSION >= '9.3' }} + run: > + php "vendor/bin/phpunit" tests/AllTests.php --coverage-cache ./build/phpunit-cache + --group CBF --exclude-group nothing --coverage-clover build/logs/clover-cbf.xml + env: + PHP_CODESNIFFER_CBF: '1' + - name: "Run the unit tests which may have different outcomes on Windows with code coverage (PHPUnit < 9.3)" if: ${{ matrix.os == 'windows-latest' && steps.phpunit_version.outputs.VERSION < '9.3' }} run: php "vendor/bin/phpunit" tests/AllTests.php --group Windows @@ -319,7 +340,7 @@ jobs: if: ${{ matrix.os == 'windows-latest' && steps.phpunit_version.outputs.VERSION >= '9.3' }} run: php "vendor/bin/phpunit" tests/AllTests.php --group Windows --coverage-cache ./build/phpunit-cache - - name: Upload coverage results to Coveralls + - name: "Upload coverage results to Coveralls (normal run)" if: ${{ success() }} uses: coverallsapp/github-action@v2 with: @@ -328,6 +349,15 @@ jobs: flag-name: os-${{ matrix.os }}-php-${{ matrix.php }}-custom-ini-${{ matrix.custom_ini }} parallel: true + - name: "Upload coverage results to Coveralls (CBF run)" + if: ${{ matrix.os != 'windows-latest' && success() }} + uses: coverallsapp/github-action@v2 + with: + format: clover + file: build/logs/clover-cbf.xml + flag-name: cbf-os-${{ matrix.os }}-ubuntu-latest-php-${{ matrix.php }}-custom-ini-${{ matrix.custom_ini }} + parallel: true + coveralls-finish: needs: coverage if: always() && needs.coverage.result == 'success' diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 1402588f20..cf2b588c86 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -18,6 +18,12 @@ + + + CBF + + + ./src @@ -32,4 +38,8 @@ + + + + diff --git a/tests/Core/Generators/GeneratorTest.php b/tests/Core/Generators/GeneratorTest.php index 30536518f6..441c499874 100644 --- a/tests/Core/Generators/GeneratorTest.php +++ b/tests/Core/Generators/GeneratorTest.php @@ -201,6 +201,10 @@ public static function dataGeneratingDocs() */ public function testGeneratorWillShowEachStandardSeparately() { + if (PHP_CODESNIFFER_CBF === true) { + $this->markTestSkipped('This test needs CS mode to run'); + } + $standard = __DIR__.'/OneDocTest.xml'; $_SERVER['argv'] = [ 'phpcs', diff --git a/tests/Core/Ruleset/ExplainTest.php b/tests/Core/Ruleset/ExplainTest.php index 48df24f473..4ec5b48317 100644 --- a/tests/Core/Ruleset/ExplainTest.php +++ b/tests/Core/Ruleset/ExplainTest.php @@ -217,6 +217,10 @@ public function testExplainWithDeprecatedSniffs() */ public function testExplainWillExplainEachStandardSeparately() { + if (PHP_CODESNIFFER_CBF === true) { + $this->markTestSkipped('This test needs CS mode to run'); + } + $standard = __DIR__.'/ExplainSingleSniffTest.xml'; $_SERVER['argv'] = [ 'phpcs', diff --git a/tests/Core/Ruleset/ShowSniffDeprecationsTest.php b/tests/Core/Ruleset/ShowSniffDeprecationsTest.php index c31f7b4a83..2a36c34c65 100644 --- a/tests/Core/Ruleset/ShowSniffDeprecationsTest.php +++ b/tests/Core/Ruleset/ShowSniffDeprecationsTest.php @@ -67,7 +67,7 @@ public static function dataHasSniffDeprecations() /** - * Test that the listing with deprecated sniffs will not show when specific command-line options are being used. + * Test that the listing with deprecated sniffs will not show when specific command-line options are being used [1]. * * @param string $standard The standard to use for the test. * @param array $additionalArgs Optional. Additional arguments to pass. @@ -102,24 +102,62 @@ public function testDeprecatedSniffsListDoesNotShow($standard, $additionalArgs=[ public static function dataDeprecatedSniffsListDoesNotShow() { return [ - 'Standard not using deprecated sniffs: PSR1' => [ + 'Standard not using deprecated sniffs: PSR1' => [ 'standard' => 'PSR1', ], - 'Standard using deprecated sniffs; explain mode' => [ + 'Standard using deprecated sniffs; explain mode' => [ 'standard' => __DIR__.'/ShowSniffDeprecationsTest.xml', 'additionalArgs' => ['-e'], ], - 'Standard using deprecated sniffs; quiet mode' => [ + 'Standard using deprecated sniffs; quiet mode' => [ 'standard' => __DIR__.'/ShowSniffDeprecationsTest.xml', 'additionalArgs' => ['-q'], ], + ]; + + }//end dataDeprecatedSniffsListDoesNotShow() + + + /** + * Test that the listing with deprecated sniffs will not show when specific command-line options are being used [2]. + * + * {@internal Separate test method for the same thing as this test will only work in CS mode.} + * + * @param string $standard The standard to use for the test. + * @param array $additionalArgs Optional. Additional arguments to pass. + * + * @dataProvider dataDeprecatedSniffsListDoesNotShowNeedsCsMode + * + * @return void + */ + public function testDeprecatedSniffsListDoesNotShowNeedsCsMode($standard, $additionalArgs=[]) + { + if (PHP_CODESNIFFER_CBF === true) { + $this->markTestSkipped('This test needs CS mode to run'); + } + + $this->testDeprecatedSniffsListDoesNotShow($standard, $additionalArgs); + + }//end testDeprecatedSniffsListDoesNotShowNeedsCsMode() + + + /** + * Data provider. + * + * @see testDeprecatedSniffsListDoesNotShowNeedsCsMode() + * + * @return array>> + */ + public static function dataDeprecatedSniffsListDoesNotShowNeedsCsMode() + { + return [ 'Standard using deprecated sniffs; documentation is requested' => [ 'standard' => __DIR__.'/ShowSniffDeprecationsTest.xml', 'additionalArgs' => ['--generator=text'], ], ]; - }//end dataDeprecatedSniffsListDoesNotShow() + }//end dataDeprecatedSniffsListDoesNotShowNeedsCsMode() /** diff --git a/tests/bootstrap.php b/tests/bootstrap.php index cf4936daa8..e8ebdbbdd0 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -11,8 +11,27 @@ define('PHP_CODESNIFFER_IN_TESTS', true); } +/* + * Determine whether the test suite should be run in CBF mode. + * + * Use `` in a `phpunit.xml` file + * or set the ENV variable at an OS-level to enable CBF mode. + * + * To run the CBF specific tests, use the following command: + * vendor/bin/phpunit --group CBF --exclude-group nothing + * + * If the ENV variable has not been set, or is set to "false", the tests will run in CS mode. + */ + if (defined('PHP_CODESNIFFER_CBF') === false) { - define('PHP_CODESNIFFER_CBF', false); + $cbfMode = getenv('PHP_CODESNIFFER_CBF'); + if ($cbfMode === '1') { + define('PHP_CODESNIFFER_CBF', true); + echo 'Note: Tests are running in "CBF" mode'.PHP_EOL.PHP_EOL; + } else { + define('PHP_CODESNIFFER_CBF', false); + echo 'Note: Tests are running in "CS" mode'.PHP_EOL.PHP_EOL; + } } if (defined('PHP_CODESNIFFER_VERBOSITY') === false) {