From 14081558c6b58e6c051e9ccfb52e46e764595f21 Mon Sep 17 00:00:00 2001 From: NanoSector Date: Fri, 6 Jun 2025 12:58:13 +0200 Subject: [PATCH 1/7] fix: Check that the result of pcntl_waitpid is the PID of a managed process The --parallel option spawns processes with pcntl_fork and expects the result of pcntl_waitpid to be one of its spawned processes. It looks up the temporary output file of a process in the given $childProcs array without checking if the returned process ID is actually managed, which under some circumstances caused by external factors may result in an undefined array index error. Fixes #1112 --- src/Runner.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Runner.php b/src/Runner.php index d527ea575e..37c3c65c7a 100644 --- a/src/Runner.php +++ b/src/Runner.php @@ -764,7 +764,7 @@ public function processFile($file) * The reporting information returned by each child process is merged * into the main reporter class. * - * @param array $childProcs An array of child processes to wait for. + * @param array $childProcs An array of child processes to wait for. * * @return bool */ @@ -777,7 +777,8 @@ private function processChildProcs($childProcs) while (count($childProcs) > 0) { $pid = pcntl_waitpid(0, $status); - if ($pid <= 0) { + if ($pid <= 0 || isset($childProcs[$pid]) === false) { + // No child or a child with an unmanaged PID was returned. continue; } From 054315dfea23256d4d5bce4b4c9059404ad5e189 Mon Sep 17 00:00:00 2001 From: NanoSector Date: Mon, 9 Jun 2025 11:27:31 +0200 Subject: [PATCH 2/7] feat: Introduce bashunit test suite This introduces the bashunit test suite as requested by @jrfnl. This tests both a simple happy and a simple unhappy flow, as well as a test case for #1112. --- .github/workflows/test.yml | 13 ++++++- phpcs.xml.dist | 1 + tests/EndToEnd/Files/.gitignore | 1 + .../Files/ClassOneWithoutStyleError.inc | 18 +++++++++ .../Files/ClassTwoWithoutStyleError.inc | 20 ++++++++++ tests/EndToEnd/Files/ClassWithStyleError.inc | 22 +++++++++++ tests/EndToEnd/Files/phpcs.xml.dist | 13 +++++++ tests/EndToEnd/phpcbf_test.sh | 38 +++++++++++++++++++ tests/EndToEnd/phpcs_test.sh | 28 ++++++++++++++ 9 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 tests/EndToEnd/Files/.gitignore create mode 100644 tests/EndToEnd/Files/ClassOneWithoutStyleError.inc create mode 100644 tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc create mode 100644 tests/EndToEnd/Files/ClassWithStyleError.inc create mode 100644 tests/EndToEnd/Files/phpcs.xml.dist create mode 100644 tests/EndToEnd/phpcbf_test.sh create mode 100644 tests/EndToEnd/phpcs_test.sh diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 06364de3ef..f0d11195b3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -247,10 +247,21 @@ jobs: env: PHP_CODESNIFFER_CBF: '1' + - name: "Install bashunit" + if: ${{ matrix.custom_ini == false && matrix.os == 'ubuntu-latest' }} + run: | + curl -s https://bashunit.typeddevs.com/install.sh > install.sh + chmod +x install.sh + ./install.sh + + - name: "Run bashunit tests" + if: ${{ matrix.custom_ini == false && matrix.os == 'ubuntu-latest' }} + run: "./lib/bashunit -p tests/EndToEnd" + # Note: The code style check is run multiple times against every PHP version # as it also acts as an integration test. - name: 'PHPCS: check code style without cache, no parallel' - if: ${{ matrix.custom_ini == false }} + if: ${{ matrix.custom_ini == false && matrix.os == 'windows-latest' }} run: php "bin/phpcs" --no-cache --parallel=1 - name: Download the PHPCS phar diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 0fd2f38d0f..6285d269f1 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -11,6 +11,7 @@ */src/Standards/*/Tests/*\.(inc|css|js)$ */tests/Core/*/*\.(inc|css|js)$ */tests/Core/*/Fixtures/*\.php$ + */tests/EndToEnd/Files/*\.inc$ diff --git a/tests/EndToEnd/Files/.gitignore b/tests/EndToEnd/Files/.gitignore new file mode 100644 index 0000000000..862bc749ba --- /dev/null +++ b/tests/EndToEnd/Files/.gitignore @@ -0,0 +1 @@ +*.fixed \ No newline at end of file diff --git a/tests/EndToEnd/Files/ClassOneWithoutStyleError.inc b/tests/EndToEnd/Files/ClassOneWithoutStyleError.inc new file mode 100644 index 0000000000..5ae6ac8f94 --- /dev/null +++ b/tests/EndToEnd/Files/ClassOneWithoutStyleError.inc @@ -0,0 +1,18 @@ + + + The coding standard for end to end tests. + + + + . + + + + + + diff --git a/tests/EndToEnd/phpcbf_test.sh b/tests/EndToEnd/phpcbf_test.sh new file mode 100644 index 0000000000..74780570b9 --- /dev/null +++ b/tests/EndToEnd/phpcbf_test.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash + +function tear_down() { + rm -r tests/EndToEnd/Files/*.fixed +} + +function test_phpcbf_is_working() { + OUTPUT="$(bin/phpcbf --no-cache --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc)" + + assert_successful_code + assert_contains "No violations were found" "$OUTPUT" +} + +function test_phpcbf_is_working_in_parallel() { + OUTPUT="$(bin/phpcbf --no-cache --parallel=2 --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc)" + + assert_successful_code + assert_contains "No violations were found" "$OUTPUT" +} + +function test_phpcbf_returns_error_on_issues() { + OUTPUT="$(bin/phpcbf --no-colors --no-cache --suffix=.fixed --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassWithStyleError.inc)" + assert_exit_code 1 + + assert_contains "F 1 / 1 (100%)" "$OUTPUT" + assert_contains "A TOTAL OF 1 ERROR WERE FIXED IN 1 FILE" "$OUTPUT" +} + +function test_phpcbf_bug_1112() { + # See https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/1112 + if [[ "$(uname)" == "Darwin" ]]; then + # Perform some magic with `& fg` to prevent the processes from turning into a background job. + assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcbf --no-cache --parallel=2 --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc" & fg')" + else + # This is not needed on Linux / GitHub Actions + assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcbf --no-cache --parallel=2 --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc"')" + fi +} diff --git a/tests/EndToEnd/phpcs_test.sh b/tests/EndToEnd/phpcs_test.sh new file mode 100644 index 0000000000..ef4aef8e36 --- /dev/null +++ b/tests/EndToEnd/phpcs_test.sh @@ -0,0 +1,28 @@ +#!/usr/bin/env bash + +function test_phpcs_is_working() { + assert_successful_code "$(bin/phpcs --no-cache --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc)" +} + +function test_phpcs_is_working_in_parallel() { + assert_successful_code "$(bin/phpcs --no-cache --parallel=2 --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc)" +} + +function test_phpcs_returns_error_on_issues() { + OUTPUT="$(bin/phpcs --no-colors --no-cache --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassWithStyleError.inc)" + assert_exit_code 2 + + assert_contains "E 1 / 1 (100%)" "$OUTPUT" + assert_contains "FOUND 1 ERROR AFFECTING 1 LINE" "$OUTPUT" +} + +function test_phpcs_bug_1112() { + # See https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/1112 + if [[ "$(uname)" == "Darwin" ]]; then + # Perform some magic with `& fg` to prevent the processes from turning into a background job. + assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcs --no-cache --parallel=2 --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc" & fg')" + else + # This is not needed on Linux / GitHub Actions + assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcs --no-cache --parallel=2 --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc"')" + fi +} From 055f13591d26fd67c8427fda8ff546520082b0c4 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 6 Aug 2025 13:33:37 +0200 Subject: [PATCH 3/7] GH Actions: move bashunit tests to own workflow --- .github/workflows/end-to-end-tests.yml | 60 ++++++++++++++++++++++++++ .github/workflows/test.yml | 13 +----- 2 files changed, 61 insertions(+), 12 deletions(-) create mode 100644 .github/workflows/end-to-end-tests.yml diff --git a/.github/workflows/end-to-end-tests.yml b/.github/workflows/end-to-end-tests.yml new file mode 100644 index 0000000000..ff9efd3729 --- /dev/null +++ b/.github/workflows/end-to-end-tests.yml @@ -0,0 +1,60 @@ +name: E2E Tests + +on: + # Run on pushes to `master`/`4.0` and on all pull requests. + # Prevent the build from running when there are only irrelevant changes. + push: + branches: + - master + - 4.0 + tags: + - '**' + paths-ignore: + - '**.md' + pull_request: + # Allow manually triggering the workflow. + workflow_dispatch: + +jobs: + bash-tests: + # Cancels all previous runs of this particular job for the same branch that have not yet completed. + concurrency: + # The concurrency group contains the workflow name, job name, job index and the branch name. + group: ${{ github.workflow }}-${{ github.job }}-${{ strategy.job-index }}-${{ github.ref }} + cancel-in-progress: true + + runs-on: 'ubuntu-latest' + + strategy: + matrix: + php: ['5.4', '5.5', '5.6', '7.0', '7.1', '7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3', '8.4', '8.5'] + + # yamllint disable-line rule:line-length + name: "E2E PHP: ${{ matrix.php }}" + + continue-on-error: ${{ matrix.php == '8.5' }} + + steps: + - name: Prepare git to leave line endings alone + run: git config --global core.autocrlf input + + - name: Checkout code + uses: actions/checkout@v4 + + - name: Install PHP + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + ini-values: "error_reporting=-1, display_errors=On" + coverage: none + + - name: "Install bashunit" + shell: bash + run: | + curl -s https://bashunit.typeddevs.com/install.sh > install.sh + chmod +x install.sh + ./install.sh + + - name: "Run bashunit tests" + shell: bash + run: "./lib/bashunit -p tests/EndToEnd" diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f0d11195b3..06364de3ef 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -247,21 +247,10 @@ jobs: env: PHP_CODESNIFFER_CBF: '1' - - name: "Install bashunit" - if: ${{ matrix.custom_ini == false && matrix.os == 'ubuntu-latest' }} - run: | - curl -s https://bashunit.typeddevs.com/install.sh > install.sh - chmod +x install.sh - ./install.sh - - - name: "Run bashunit tests" - if: ${{ matrix.custom_ini == false && matrix.os == 'ubuntu-latest' }} - run: "./lib/bashunit -p tests/EndToEnd" - # Note: The code style check is run multiple times against every PHP version # as it also acts as an integration test. - name: 'PHPCS: check code style without cache, no parallel' - if: ${{ matrix.custom_ini == false && matrix.os == 'windows-latest' }} + if: ${{ matrix.custom_ini == false }} run: php "bin/phpcs" --no-cache --parallel=1 - name: Download the PHPCS phar From e23fdf965947a851e750a691dee1c524e5357e16 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 6 Aug 2025 17:20:53 +0200 Subject: [PATCH 4/7] E2E tests: rename fixtures directory ... to be in line with other test fixture directories. --- .gitignore | 1 + phpcs.xml.dist | 2 +- tests/EndToEnd/Files/.gitignore | 1 - .../ClassOneWithoutStyleError.inc | 2 +- .../ClassTwoWithoutStyleError.inc | 2 +- .../{Files => Fixtures}/ClassWithStyleError.inc | 2 +- tests/EndToEnd/{Files => Fixtures}/phpcs.xml.dist | 2 +- tests/EndToEnd/phpcbf_test.sh | 12 ++++++------ tests/EndToEnd/phpcs_test.sh | 10 +++++----- 9 files changed, 17 insertions(+), 17 deletions(-) delete mode 100644 tests/EndToEnd/Files/.gitignore rename tests/EndToEnd/{Files => Fixtures}/ClassOneWithoutStyleError.inc (87%) rename tests/EndToEnd/{Files => Fixtures}/ClassTwoWithoutStyleError.inc (86%) rename tests/EndToEnd/{Files => Fixtures}/ClassWithStyleError.inc (90%) rename tests/EndToEnd/{Files => Fixtures}/phpcs.xml.dist (83%) diff --git a/.gitignore b/.gitignore index 3b1c9e6084..769bd78e65 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ composer.lock phpstan.neon /node_modules/ +/tests/EndToEnd/Fixtures/*.fixed diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 6285d269f1..fdb9235f91 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -11,7 +11,7 @@ */src/Standards/*/Tests/*\.(inc|css|js)$ */tests/Core/*/*\.(inc|css|js)$ */tests/Core/*/Fixtures/*\.php$ - */tests/EndToEnd/Files/*\.inc$ + */tests/EndToEnd/Fixtures/*\.inc$ diff --git a/tests/EndToEnd/Files/.gitignore b/tests/EndToEnd/Files/.gitignore deleted file mode 100644 index 862bc749ba..0000000000 --- a/tests/EndToEnd/Files/.gitignore +++ /dev/null @@ -1 +0,0 @@ -*.fixed \ No newline at end of file diff --git a/tests/EndToEnd/Files/ClassOneWithoutStyleError.inc b/tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc similarity index 87% rename from tests/EndToEnd/Files/ClassOneWithoutStyleError.inc rename to tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc index 5ae6ac8f94..700fe7a7b0 100644 --- a/tests/EndToEnd/Files/ClassOneWithoutStyleError.inc +++ b/tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc @@ -7,7 +7,7 @@ * @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence */ -namespace PHP_CodeSniffer\Tests\EndToEnd\Files; +namespace PHP_CodeSniffer\Tests\EndToEnd\Fixtures; class ClassOneWithoutStyleError { diff --git a/tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc b/tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc similarity index 86% rename from tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc rename to tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc index 1fdc5b3b0b..551885b58c 100644 --- a/tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc +++ b/tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc @@ -7,7 +7,7 @@ * @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence */ -namespace PHP_CodeSniffer\Tests\EndToEnd\Files; +namespace PHP_CodeSniffer\Tests\EndToEnd\Fixtures; class ClassTwoWithoutStyleError { diff --git a/tests/EndToEnd/Files/ClassWithStyleError.inc b/tests/EndToEnd/Fixtures/ClassWithStyleError.inc similarity index 90% rename from tests/EndToEnd/Files/ClassWithStyleError.inc rename to tests/EndToEnd/Fixtures/ClassWithStyleError.inc index 4179ecdca1..7e7cc97c44 100644 --- a/tests/EndToEnd/Files/ClassWithStyleError.inc +++ b/tests/EndToEnd/Fixtures/ClassWithStyleError.inc @@ -7,7 +7,7 @@ * @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence */ -namespace PHP_CodeSniffer\Tests\EndToEnd\Files; +namespace PHP_CodeSniffer\Tests\EndToEnd\Fixtures; class ClassWithStyleError { diff --git a/tests/EndToEnd/Files/phpcs.xml.dist b/tests/EndToEnd/Fixtures/phpcs.xml.dist similarity index 83% rename from tests/EndToEnd/Files/phpcs.xml.dist rename to tests/EndToEnd/Fixtures/phpcs.xml.dist index 0a10053386..2cea01db64 100644 --- a/tests/EndToEnd/Files/phpcs.xml.dist +++ b/tests/EndToEnd/Fixtures/phpcs.xml.dist @@ -1,5 +1,5 @@ - + The coding standard for end to end tests. diff --git a/tests/EndToEnd/phpcbf_test.sh b/tests/EndToEnd/phpcbf_test.sh index 74780570b9..af6a646139 100644 --- a/tests/EndToEnd/phpcbf_test.sh +++ b/tests/EndToEnd/phpcbf_test.sh @@ -1,25 +1,25 @@ #!/usr/bin/env bash function tear_down() { - rm -r tests/EndToEnd/Files/*.fixed + rm -r tests/EndToEnd/Fixtures/*.fixed } function test_phpcbf_is_working() { - OUTPUT="$(bin/phpcbf --no-cache --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc)" + OUTPUT="$(bin/phpcbf --no-cache --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc)" assert_successful_code assert_contains "No violations were found" "$OUTPUT" } function test_phpcbf_is_working_in_parallel() { - OUTPUT="$(bin/phpcbf --no-cache --parallel=2 --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc)" + OUTPUT="$(bin/phpcbf --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc)" assert_successful_code assert_contains "No violations were found" "$OUTPUT" } function test_phpcbf_returns_error_on_issues() { - OUTPUT="$(bin/phpcbf --no-colors --no-cache --suffix=.fixed --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassWithStyleError.inc)" + OUTPUT="$(bin/phpcbf --no-colors --no-cache --suffix=.fixed --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassWithStyleError.inc)" assert_exit_code 1 assert_contains "F 1 / 1 (100%)" "$OUTPUT" @@ -30,9 +30,9 @@ function test_phpcbf_bug_1112() { # See https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/1112 if [[ "$(uname)" == "Darwin" ]]; then # Perform some magic with `& fg` to prevent the processes from turning into a background job. - assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcbf --no-cache --parallel=2 --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc" & fg')" + assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcbf --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc" & fg')" else # This is not needed on Linux / GitHub Actions - assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcbf --no-cache --parallel=2 --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc"')" + assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcbf --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc"')" fi } diff --git a/tests/EndToEnd/phpcs_test.sh b/tests/EndToEnd/phpcs_test.sh index ef4aef8e36..997b8da782 100644 --- a/tests/EndToEnd/phpcs_test.sh +++ b/tests/EndToEnd/phpcs_test.sh @@ -1,15 +1,15 @@ #!/usr/bin/env bash function test_phpcs_is_working() { - assert_successful_code "$(bin/phpcs --no-cache --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc)" + assert_successful_code "$(bin/phpcs --no-cache --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc)" } function test_phpcs_is_working_in_parallel() { - assert_successful_code "$(bin/phpcs --no-cache --parallel=2 --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc)" + assert_successful_code "$(bin/phpcs --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc)" } function test_phpcs_returns_error_on_issues() { - OUTPUT="$(bin/phpcs --no-colors --no-cache --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassWithStyleError.inc)" + OUTPUT="$(bin/phpcs --no-colors --no-cache --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassWithStyleError.inc)" assert_exit_code 2 assert_contains "E 1 / 1 (100%)" "$OUTPUT" @@ -20,9 +20,9 @@ function test_phpcs_bug_1112() { # See https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/1112 if [[ "$(uname)" == "Darwin" ]]; then # Perform some magic with `& fg` to prevent the processes from turning into a background job. - assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcs --no-cache --parallel=2 --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc" & fg')" + assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcs --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc" & fg')" else # This is not needed on Linux / GitHub Actions - assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcs --no-cache --parallel=2 --standard=tests/EndToEnd/Files/phpcs.xml.dist tests/EndToEnd/Files/ClassOneWithoutStyleError.inc tests/EndToEnd/Files/ClassTwoWithoutStyleError.inc"')" + assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcs --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc"')" fi } From db4f766eb8bb86a7a51797bfb0b7bcfb7bad7a56 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 6 Aug 2025 17:30:21 +0200 Subject: [PATCH 5/7] E2E tests: rename PHPCS config fixture ... to prevent it from ever being confused with the real project ruleset. --- .../Fixtures/{phpcs.xml.dist => endtoend.xml.dist} | 0 tests/EndToEnd/phpcbf_test.sh | 10 +++++----- tests/EndToEnd/phpcs_test.sh | 10 +++++----- 3 files changed, 10 insertions(+), 10 deletions(-) rename tests/EndToEnd/Fixtures/{phpcs.xml.dist => endtoend.xml.dist} (100%) diff --git a/tests/EndToEnd/Fixtures/phpcs.xml.dist b/tests/EndToEnd/Fixtures/endtoend.xml.dist similarity index 100% rename from tests/EndToEnd/Fixtures/phpcs.xml.dist rename to tests/EndToEnd/Fixtures/endtoend.xml.dist diff --git a/tests/EndToEnd/phpcbf_test.sh b/tests/EndToEnd/phpcbf_test.sh index af6a646139..3886832342 100644 --- a/tests/EndToEnd/phpcbf_test.sh +++ b/tests/EndToEnd/phpcbf_test.sh @@ -5,21 +5,21 @@ function tear_down() { } function test_phpcbf_is_working() { - OUTPUT="$(bin/phpcbf --no-cache --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc)" + OUTPUT="$(bin/phpcbf --no-cache --standard=tests/EndToEnd/Fixtures/endtoend.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc)" assert_successful_code assert_contains "No violations were found" "$OUTPUT" } function test_phpcbf_is_working_in_parallel() { - OUTPUT="$(bin/phpcbf --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc)" + OUTPUT="$(bin/phpcbf --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/endtoend.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc)" assert_successful_code assert_contains "No violations were found" "$OUTPUT" } function test_phpcbf_returns_error_on_issues() { - OUTPUT="$(bin/phpcbf --no-colors --no-cache --suffix=.fixed --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassWithStyleError.inc)" + OUTPUT="$(bin/phpcbf --no-colors --no-cache --suffix=.fixed --standard=tests/EndToEnd/Fixtures/endtoend.xml.dist tests/EndToEnd/Fixtures/ClassWithStyleError.inc)" assert_exit_code 1 assert_contains "F 1 / 1 (100%)" "$OUTPUT" @@ -30,9 +30,9 @@ function test_phpcbf_bug_1112() { # See https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/1112 if [[ "$(uname)" == "Darwin" ]]; then # Perform some magic with `& fg` to prevent the processes from turning into a background job. - assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcbf --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc" & fg')" + assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcbf --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/endtoend.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc" & fg')" else # This is not needed on Linux / GitHub Actions - assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcbf --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc"')" + assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcbf --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/endtoend.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc"')" fi } diff --git a/tests/EndToEnd/phpcs_test.sh b/tests/EndToEnd/phpcs_test.sh index 997b8da782..3bdf41708d 100644 --- a/tests/EndToEnd/phpcs_test.sh +++ b/tests/EndToEnd/phpcs_test.sh @@ -1,15 +1,15 @@ #!/usr/bin/env bash function test_phpcs_is_working() { - assert_successful_code "$(bin/phpcs --no-cache --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc)" + assert_successful_code "$(bin/phpcs --no-cache --standard=tests/EndToEnd/Fixtures/endtoend.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc)" } function test_phpcs_is_working_in_parallel() { - assert_successful_code "$(bin/phpcs --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc)" + assert_successful_code "$(bin/phpcs --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/endtoend.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc)" } function test_phpcs_returns_error_on_issues() { - OUTPUT="$(bin/phpcs --no-colors --no-cache --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassWithStyleError.inc)" + OUTPUT="$(bin/phpcs --no-colors --no-cache --standard=tests/EndToEnd/Fixtures/endtoend.xml.dist tests/EndToEnd/Fixtures/ClassWithStyleError.inc)" assert_exit_code 2 assert_contains "E 1 / 1 (100%)" "$OUTPUT" @@ -20,9 +20,9 @@ function test_phpcs_bug_1112() { # See https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/1112 if [[ "$(uname)" == "Darwin" ]]; then # Perform some magic with `& fg` to prevent the processes from turning into a background job. - assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcs --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc" & fg')" + assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcs --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/endtoend.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc" & fg')" else # This is not needed on Linux / GitHub Actions - assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcs --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/phpcs.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc"')" + assert_successful_code "$(bash -ic 'bash --init-file <(echo "echo \"Subprocess\"") -c "bin/phpcs --no-cache --parallel=2 --standard=tests/EndToEnd/Fixtures/endtoend.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc"')" fi } From e1e56e3c0aefa144972676e33ff7de8d81d8bbcf Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 6 Aug 2025 17:36:42 +0200 Subject: [PATCH 6/7] CONTRIBUTING: add minimal info about bashunit tests --- .github/CONTRIBUTING.md | 25 ++++++++++++++++++++++--- .gitignore | 1 + 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index e9af9149ca..b6a7bc4ea2 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -18,7 +18,8 @@ Thank you for your interest in contributing to PHP_CodeSniffer! * [Finding Something to Work on](#finding-something-to-work-on) * [Getting Started](#getting-started) * [While Working on a Patch](#while-working-on-a-patch) - * [Writing Tests](#writing-tests) + * [Writing Unit/Integration Tests](#writing-unitintegration-tests) + * [Writing End-to-End Tests](#writing-end-to-end-tests) * [Submitting Your Pull Request](#submitting-your-pull-request) * [Licensing](#licensing) @@ -268,7 +269,7 @@ To help you with this, a number of convenience scripts are available: N.B.: You can ignore any skipped tests as these are for external tools. -### Writing Tests +### Writing Unit/Integration Tests Tests for the PHP_CodeSniffer engine can be found in the `tests/Core` directory. Tests for individual sniffs can be found in the `src/Standards/[StandardName]/Tests/[Category]/` directory. @@ -376,7 +377,7 @@ To run the tests specific to the use of `PHP_CODESNIFFER_CBF === true`: vendor/bin/phpunit --group CBF --exclude-group nothing ``` -#### Other notes about writing tests +#### Other notes about writing unit/integration tests * The `Config` class uses a number of static properties and can have a performance impact on the tests too. To get round both these issues, use the `ConfigDouble` class instead. @@ -387,6 +388,24 @@ To run the tests specific to the use of `PHP_CODESNIFFER_CBF === true`: * When using data providers, define them immediately below the corresponding test method. * When a test method has only one data provider, it is considered best practice to closely couple the test and data provider methods via their names. I.e. the data provider's name should match the test method name, replacing the "test" prefix with "data". For example, the data provider for a method named `testSomething()` should be `dataSomething()`. + +### Writing End-to-End Tests + +Bash-based end-to-end tests can be written using the [Bashunit](https://bashunit.typeddevs.com/) test tooling. + +To install bashunit, follow the [installation guide](https://bashunit.typeddevs.com/installation). + +You can then run the bashunit tests on Linux/Mac/WSL, like so: +```bash +./lib/bashunit -p tests/EndToEnd +``` + +> Note: these tests will not run in the Windows native CMD shell. When on Windows, either use WSL or use the "git bash" shell. + +When writing end-to-end tests, please use fixtures for the "files under scan" to make the tests stable. +These fixtures can be placed in the `tests/EndToEnd/Fixtures` subdirectory. + + ### Submitting Your Pull Request Some guidelines for submitting pull requests (PRs) and improving the chance that your PR will be merged: diff --git a/.gitignore b/.gitignore index 769bd78e65..3152b53c61 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,4 @@ composer.lock phpstan.neon /node_modules/ /tests/EndToEnd/Fixtures/*.fixed +/lib/ From 98b197449e068a0cf89fe6557399a7554cf3209d Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 6 Aug 2025 13:33:57 +0200 Subject: [PATCH 7/7] Runner: fix type annotation --- src/Runner.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Runner.php b/src/Runner.php index 37c3c65c7a..7b299c7f84 100644 --- a/src/Runner.php +++ b/src/Runner.php @@ -764,7 +764,7 @@ public function processFile($file) * The reporting information returned by each child process is merged * into the main reporter class. * - * @param array $childProcs An array of child processes to wait for. + * @param array $childProcs An array of child processes to wait for. * * @return bool */