From 14081558c6b58e6c051e9ccfb52e46e764595f21 Mon Sep 17 00:00:00 2001 From: NanoSector Date: Fri, 6 Jun 2025 12:58:13 +0200 Subject: [PATCH 1/2] 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/2] 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 +}