From 4e3e88cd5059f0560424fdeff6fbdc09907fd7cf Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 28 Mar 2021 03:07:00 +0200 Subject: [PATCH 1/9] GH Actions workflows: fix PHP set up step To disable code coverage, the `coverage` key should be set to `none`. `false` is not a valid (or supported) value, so the behaviour may be unpredictable. Ref: https://github.com/shivammathur/setup-php#disable-coverage --- .github/workflows/release.yml | 4 ++-- .github/workflows/test.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9fbb685..a172935 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -19,7 +19,7 @@ jobs: with: php-version: 5.4 extensions: exif, phar, openssl - coverage: false + coverage: none ini-values: phar.readonly=Off - name: Install Box from GitHub @@ -79,7 +79,7 @@ jobs: uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php }} - coverage: false + coverage: none - name: Run linter against codebase run: php ./parallel-lint.phar src/ diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d948369..badc207 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -61,7 +61,7 @@ jobs: with: php-version: 5.4 extensions: exif, phar, openssl - coverage: false + coverage: none ini-values: phar.readonly=Off - name: Install Box from GitHub @@ -121,7 +121,7 @@ jobs: uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php }} - coverage: false + coverage: none - name: Run linter against codebase run: php ./parallel-lint.phar src/ From 3a0f8ce34c8e0684e57682695748d22caeb73ebc Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 28 Mar 2021 03:19:02 +0200 Subject: [PATCH 2/9] GH Actions: run the tests against all supported PHP versions The original Travis script ran the following commands against *all* supported PHP versions, but didn't test the phar: ```yaml - composer test - ./parallel-lint --exclude vendor --exclude tests/examples --no-colors . - ./parallel-lint --exclude vendor --exclude tests/examples . ``` The current `test` job only ran the unit tests against PHP 5.4. It did not run the integration test (running Parallel Lint against its own code) via a direct call to the script anymore at all. The integration test was basically now _only_ being run for the phar and only in one flavour, though it did do that on all supported PHP versions. This commit: * Removes the job which only ran the unit tests against PHP 5.4. * Adds the running of the above three commands (unit tests and two versions of integrations tests runs via a direct call to the script), to the job which also runs the integration tests via the phar file. * Updates the command line parameters used for the phar run to be the same as those used for the direct script call integration tests. --- .github/workflows/test.yml | 48 ++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index badc207..8e84d04 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -28,26 +28,6 @@ jobs: - name: Run code sniffer run: vendor/bin/phpcs - test: - name: Run unit tests - runs-on: ubuntu-latest - - steps: - - name: Setup PHP - uses: shivammathur/setup-php@v2 - with: - php-version: 5.4 - extensions: json, tokenizer - - - name: Checkout code - uses: actions/checkout@v2 - - - name: Install Composer dependencies - uses: ramsey/composer-install@v1 - - - name: Run tests - run: composer test - bundle: name: Bundle binary runs-on: ubuntu-latest @@ -84,8 +64,8 @@ jobs: name: parallel-lint-phar path: ./parallel-lint.phar - verify-bundle: - name: Validate binary on PHP ${{ matrix.php }} + test: + name: Run tests on PHP ${{ matrix.php }} runs-on: ubuntu-latest continue-on-error: ${{ matrix.experimental == true }} needs: @@ -113,15 +93,27 @@ jobs: - name: Checkout code uses: actions/checkout@v2 - - uses: actions/download-artifact@v2 - with: - name: parallel-lint-phar - - name: Setup PHP uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php }} coverage: none - - name: Run linter against codebase - run: php ./parallel-lint.phar src/ + - name: Install Composer dependencies + uses: ramsey/composer-install@v1 + + - name: Run unit tests + run: composer test + + - name: 'Integration test 1 - linting own code, no colors' + run: ./parallel-lint --exclude vendor --exclude tests/examples --no-colors . + + - name: 'Integration test 2 - linting own code' + run: ./parallel-lint --exclude vendor --exclude tests/examples . + + - uses: actions/download-artifact@v2 + with: + name: parallel-lint-phar + + - name: Run linter against codebase using the phar + run: php ./parallel-lint.phar --exclude vendor --exclude tests/examples . From 18272dbb9d631293f434e6724a0b967e3ed815fe Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 28 Mar 2021 04:42:27 +0200 Subject: [PATCH 3/9] GH Actions: actually get the tests running on all PHP versions This fixes the problem originally identified by roelofr with the `Fatal error: Interface 'JsonSerializable' not found` error. The problem had nothing to do with the PHP setup, but everything with some inane setting of the Nette testing framework as can be seen in the transcript of older, failing trial runs done by roelofr while setting up the GH Actions workflows: ``` Run composer test > @php vendor/bin/tester -p php tests _____ ___ ___ _____ ___ ___ |_ _/ __)( __/_ _/ __)| _ ) |_| \___ /___) |_| \___ |_|_\ v2.3.5 Note: No php.ini is used. ``` The key here is the `Note: No php.ini is used.`. As the system `php.ini` was not being used, no extensions were loaded, which was causing the errors. This is a change which was introduced in Nette Tester 2.0.0. As of that version, you need to always tell the Nette testing framework explicitly which `php.ini` file to use, `-C` tells it to use the system-wide `php.ini`, with `-c` you can specify a path to a `php.ini` file and by default **_no `php.ini` is used_**. (_honestly, why did anyone ever think making that the default was a good idea ?!!?_) As the tests will be running on different versions of the Nette Framework, Nette 1.x for PHP 5.4 and 5.5 and Nette 2.x for PHP 5.6+ and Nette 1.x, does not yet support the `-C` option and breaks when it is used, I've added a second test script to the `composer.json` file with the correct command line options to run Nette on PHP 5.4/5.5 and added conditions to the GH Actions workflow to use the correct script depending on the PHP/Nette test framework version being used. Refs: * https://tester.nette.org/en/running-tests#toc-c-path * https://tester.nette.org/en/guide#toc-supported-php-versions * https://github.com/nette/tester/releases/tag/v2.0.0 --- .github/workflows/test.yml | 7 ++++++- composer.json | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8e84d04..fd7cac0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -102,7 +102,12 @@ jobs: - name: Install Composer dependencies uses: ramsey/composer-install@v1 - - name: Run unit tests + - name: 'Run unit tests PHP 5.4, 5.5' + if: ${{ matrix.php < 5.6 }} + run: composer testphp5 + + - name: 'Run unit tests PHP >= 5.6' + if: ${{ matrix.php >= 5.6 }} run: composer test - name: 'Integration test 1 - linting own code, no colors' diff --git a/composer.json b/composer.json index ae978fd..9916ca7 100644 --- a/composer.json +++ b/composer.json @@ -37,7 +37,8 @@ "parallel-lint" ], "scripts": { - "test": "@php vendor/bin/tester -p php tests" + "test": "@php vendor/bin/tester -C -p php tests", + "testphp5": "@php vendor/bin/tester -p php tests" }, "scripts-descriptions": { "test": "Run all tests!" From 0978833954e33de01b5f06cd3b1bcb09c06bcdcf Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 28 Mar 2021 03:19:33 +0200 Subject: [PATCH 4/9] GH Actions: allow for manually triggering a workflow Triggering a workflow for a branch manually is not supported by default in GH Actions, but has to be explicitly allowed. This is useful if, for instance, an external action script or composer dependency has broken. Once a fix is available, failing builds for open PRs can be retriggered manually instead of having to be re-pushed to retrigger the workflow. Ref: https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/ --- .github/workflows/release.yml | 2 ++ .github/workflows/test.yml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a172935..634d9d6 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -4,6 +4,8 @@ on: push: tags: - 'v*' + # Allow manually triggering the workflow. + workflow_dispatch: jobs: bundle: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index fd7cac0..fa1bc27 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,6 +6,8 @@ on: branches: - develop - master + # Allow manually triggering the workflow. + workflow_dispatch: jobs: lint: From 9ebde0898fdab3ab3c072f21522c3431b16fe0d6 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 28 Mar 2021 03:21:26 +0200 Subject: [PATCH 5/9] GH Actions: report CS violations in the PR The cs2pr tool will allow to display the results from an action run in checkstyle format in-line in the PR code view, which should improve usability of the workflow results. This implements this for the style check command. Includes switching the PHP version to PHP 7.4, as PHP 8.0 is not fully supported yet in PHP_CodeSniffer (full support expected in PHPCS 3.6.0). Ref: https://github.com/staabm/annotate-pull-request-from-checkstyle --- .github/workflows/test.yml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index fa1bc27..03a9ffc 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,9 +17,9 @@ jobs: - name: Setup PHP uses: shivammathur/setup-php@v2 with: - php-version: '8.0' - extensions: json + php-version: '7.4' coverage: none + tools: cs2pr - name: Checkout code uses: actions/checkout@v2 @@ -28,7 +28,11 @@ jobs: uses: ramsey/composer-install@v1 - name: Run code sniffer - run: vendor/bin/phpcs + continue-on-error: true + run: vendor/bin/phpcs --report-full --report-checkstyle=./phpcs-report.xml + + - name: Show PHPCS results in PR + run: cs2pr ./phpcs-report.xml bundle: name: Bundle binary From 2c1511bb81e14d512ee15a679a21f34d21d36c4f Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 28 Mar 2021 03:26:25 +0200 Subject: [PATCH 6/9] GH Actions: clean up * Remove the now redundant Travis script. * Remove the reference to the Travis script from `.gitattributes` and add the new `.github` folder. * Update the "Build status" badge in the README to show the result of the GH Actions run instead of Travis. Supersedes 54 Co-authored-by: Sam Reed --- .gitattributes | 2 +- .travis.yml | 32 -------------------------------- README.md | 2 +- 3 files changed, 2 insertions(+), 34 deletions(-) delete mode 100644 .travis.yml diff --git a/.gitattributes b/.gitattributes index 64d88f3..e3dd5b0 100644 --- a/.gitattributes +++ b/.gitattributes @@ -7,7 +7,7 @@ # /.gitattributes export-ignore /.gitignore export-ignore -/.travis.yml export-ignore +/.github/ export-ignore /appveyor.yml export-ignore /box.json export-ignore /doc export-ignore diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index af54d4e..0000000 --- a/.travis.yml +++ /dev/null @@ -1,32 +0,0 @@ -language: php - -matrix: - include: - - php: 5.4 - dist: trusty - - php: 5.5 - dist: trusty - - php: 5.6 - - php: 7.0 - - php: 7.1 - - php: 7.2 - - php: 7.3 - - php: 7.4 - env: SNIFF=1 - - php: 8.0 - - php: "nightly" - - fast_finish: true - - allow_failures: - # Allow failures for unstable builds. - - php: "nightly" - -install: - - composer install --no-interaction --prefer-source - -script: - - composer test - - ./parallel-lint --exclude vendor --exclude tests/examples --no-colors . - - ./parallel-lint --exclude vendor --exclude tests/examples . - - if [[ "$SNIFF" == "1" ]]; then ./vendor/bin/phpcs;fi diff --git a/README.md b/README.md index 68c99c1..0ba4f11 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # PHP Parallel Lint [![Downloads this Month](https://img.shields.io/packagist/dm/php-parallel-lint/php-parallel-lint.svg)](https://packagist.org/packages/php-parallel-lint/php-parallel-lint) -[![Build Status](https://travis-ci.org/php-parallel-lint/PHP-Parallel-Lint.svg?branch=master)](https://travis-ci.org/php-parallel-lint/PHP-Parallel-Lint) +[![Build Status](https://github.com/php-parallel-lint/PHP-Parallel-Lint/actions/workflows/test.yml/badge.svg)](https://github.com/php-parallel-lint/PHP-Parallel-Lint/actions/workflows/test.yml) [![License](https://poser.pugx.org/php-parallel-lint/php-parallel-lint/license.svg)](https://packagist.org/packages/php-parallel-lint/php-parallel-lint) This application checks syntax of PHP files in parallel. From 5ca9c0cb79c214c39f831bea291026c144b39a1d Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 28 Mar 2021 14:10:58 +0200 Subject: [PATCH 7/9] GH Actions: allow for testing on PHP 5.3 In anticipation of PR 51 being merged, I'm already adding an extra step to the `test` job. PHP_CodeSniffer has a minimum PHP requirement of PHP 5.4, which would block the `composer install` on PHP 5.3. By removing that `dev` dependency ahead of the `composer install`, the test workflow can also run on PHP 5.3. --- .github/workflows/test.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 03a9ffc..7c2ee2f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -105,6 +105,11 @@ jobs: php-version: ${{ matrix.php }} coverage: none + # Remove PHPCS as it has a minimum PHP requirements of PHP 5.4 and would block install on PHP 5.3. + - name: 'Composer: remove PHPCS' + if: ${{ matrix.php < 5.4 }} + run: composer remove --dev squizlabs/php_codesniffer --no-update + - name: Install Composer dependencies uses: ramsey/composer-install@v1 From e373045fa0050607772da53f98781bc72e6494b1 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 28 Mar 2021 14:21:21 +0200 Subject: [PATCH 8/9] GH Actions: fix phar creation The `phar` file should only contain the files of PHP Parallel Lint and any non-dev requirements. It should not include the `dev` requirements of this package. As things were, it did. Fixed now, by doing the `composer install` with the `--no-dev` option, both for the Test workflow as well as for the Release workflow. --- .github/workflows/release.yml | 2 ++ .github/workflows/test.yml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 634d9d6..e1e1cd8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -35,6 +35,8 @@ jobs: - name: Install Composer dependencies uses: ramsey/composer-install@v1 + with: + composer-options: "--no-dev" - name: Building binary... run: box build -v diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7c2ee2f..b92c455 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -61,6 +61,8 @@ jobs: - name: Install Composer dependencies uses: ramsey/composer-install@v1 + with: + composer-options: "--no-dev" - name: Building binary... run: box build -v From 194a080c8b794a5730611a321afac4b2eb884332 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 28 Mar 2021 15:14:45 +0200 Subject: [PATCH 9/9] GH Actions: switch execution order of unit vs integration tests ... and add `continue-on-error` to the first of the integration tests. If/when any of the tests fail, execution of the `Test` job will stop. Now, if there is a parse error in the code of any of the Parallel Lint files, with the test execution order as it was, that means the job would already fail on the running of the unit tests and stop there. However to identify the parse error, the integration tests are more useful, so with that in mind, those will now be run first. Secondly, if there is a parse error, the first integration test would fail and the second (with colours) would never get executed, while especially in the case of a parse error in the Parallel Lint code itself, it is useful to see the output of both these integration tests. So, with that in mind, I've set the first of the two to `continue-on-error`. As the second integration test would fail anyway, this will never negatively impact the workflow success/failure marking in the end, while it does allow us to see the output of both integration test steps. --- .github/workflows/test.yml | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b92c455..99633a0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -115,6 +115,13 @@ jobs: - name: Install Composer dependencies uses: ramsey/composer-install@v1 + - name: 'Integration test 1 - linting own code, no colors' + continue-on-error: true + run: ./parallel-lint --exclude vendor --exclude tests/examples --no-colors . + + - name: 'Integration test 2 - linting own code' + run: ./parallel-lint --exclude vendor --exclude tests/examples . + - name: 'Run unit tests PHP 5.4, 5.5' if: ${{ matrix.php < 5.6 }} run: composer testphp5 @@ -123,12 +130,6 @@ jobs: if: ${{ matrix.php >= 5.6 }} run: composer test - - name: 'Integration test 1 - linting own code, no colors' - run: ./parallel-lint --exclude vendor --exclude tests/examples --no-colors . - - - name: 'Integration test 2 - linting own code' - run: ./parallel-lint --exclude vendor --exclude tests/examples . - - uses: actions/download-artifact@v2 with: name: parallel-lint-phar