Skip to content

fix: Check that the result of pcntl_waitpid is the PID of a managed process #1126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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:
Expand Down
60 changes: 60 additions & 0 deletions .github/workflows/end-to-end-tests.yml
Original file line number Diff line number Diff line change
@@ -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"
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,5 @@
composer.lock
phpstan.neon
/node_modules/
/tests/EndToEnd/Fixtures/*.fixed
/lib/
1 change: 1 addition & 0 deletions phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
<exclude-pattern>*/src/Standards/*/Tests/*\.(inc|css|js)$</exclude-pattern>
<exclude-pattern>*/tests/Core/*/*\.(inc|css|js)$</exclude-pattern>
<exclude-pattern>*/tests/Core/*/Fixtures/*\.php$</exclude-pattern>
<exclude-pattern>*/tests/EndToEnd/Fixtures/*\.inc$</exclude-pattern>

<arg name="basepath" value="."/>
<arg name="colors"/>
Expand Down
5 changes: 3 additions & 2 deletions src/Runner.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<int, string> $childProcs An array of child processes to wait for.
*
* @return bool
*/
Expand All @@ -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;
}

Expand Down
18 changes: 18 additions & 0 deletions tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

/**
* Class containing no style errors according to the end-to-end tests phpcs.xml.dist.
*
* @copyright 2025 PHPCSStandards and contributors
* @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
*/

namespace PHP_CodeSniffer\Tests\EndToEnd\Fixtures;

class ClassOneWithoutStyleError
{
private function foo()
{
return 'bar';
}
}
20 changes: 20 additions & 0 deletions tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

/**
* Class containing no style errors.
*
* @copyright 2025 PHPCSStandards and contributors
* @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
*/

namespace PHP_CodeSniffer\Tests\EndToEnd\Fixtures;

class ClassTwoWithoutStyleError
{
/**
* A property.
*
* @var string
*/
private $bar = 'baz';
}
22 changes: 22 additions & 0 deletions tests/EndToEnd/Fixtures/ClassWithStyleError.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

/**
* Class containing a simple style error that phpcbf can fix.
*
* @copyright 2025 PHPCSStandards and contributors
* @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
*/

namespace PHP_CodeSniffer\Tests\EndToEnd\Fixtures;

class ClassWithStyleError
{
/**
* The bracket for this function is misaligned and this can be automatically fixed by phpcbf.
*
* @return string
*/
private function foo() {
return 'bar';
}
}
13 changes: 13 additions & 0 deletions tests/EndToEnd/Fixtures/endtoend.xml.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version="1.0"?>
<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="EndToEndTests" xsi:noNamespaceSchemaLocation="phpcs.xsd">
<description>The coding standard for end to end tests.</description>

<rule ref="PSR12"/>

<file>.</file>

<arg name="basepath" value="."/>
<arg name="colors"/>
<arg name="parallel" value="75"/>
<arg value="p"/>
</ruleset>
38 changes: 38 additions & 0 deletions tests/EndToEnd/phpcbf_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#!/usr/bin/env bash

function tear_down() {
rm -r tests/EndToEnd/Fixtures/*.fixed
}

function test_phpcbf_is_working() {
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/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/endtoend.xml.dist tests/EndToEnd/Fixtures/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/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/endtoend.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc"')"
fi
}
28 changes: 28 additions & 0 deletions tests/EndToEnd/phpcs_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#!/usr/bin/env bash

function test_phpcs_is_working() {
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/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/endtoend.xml.dist tests/EndToEnd/Fixtures/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/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/endtoend.xml.dist tests/EndToEnd/Fixtures/ClassOneWithoutStyleError.inc tests/EndToEnd/Fixtures/ClassTwoWithoutStyleError.inc"')"
fi
}
Loading