CI: switch to GitHub Actions#1260
Merged
Merged
Conversation
44bbe8f to
8eb0e1d
Compare
dingo-d
added a commit
to WPTT/WPThemeReview
that referenced
this pull request
Dec 20, 2020
Also, some ideas from PHPCompatibility PR PHPCompatibility/PHPCompatibility#1260 were implemented as well (composer dependencies and xml violations).
73434d6 to
e128f36
Compare
This commit:
* Adds a GH Actions workflow for the CI checks which were previously run on Travis in the `sniff` stage.
While these aren't 100% CS (= code style) checks, for the badge and workflow display, `CS` still seemed the most descriptive name.
* Removes that part of the `.travis.yml` configuration.
* Adds a "Build Status" badge in the Readme to use the results from this particular GH Actions runs.
Notes:
1. Builds will run on all pushes and on pull requests, with the exception of those just modifying files which are irrelevant to this workflow.
This commit:
* Adds a GH Actions workflow for the CI checks which were previously run on Travis in the `quicktest` stage.
* Removes that part of the `.travis.yml` configuration.
Notes:
1. Previously, this "stage" would run on all `push` events, except when merging to `master` or `develop`.
The current set-up still does so, with one exception: pushes to `develop` will now use this quicktest instead of the full test.
`develop` is a protected branch anyhow, so all merges will already have had the full test run in the pull request.
For merges to `master`, the full Test will still be used as that will generally mean we're getting ready to tag a release and some extra safety checking before a release is not a bad thing.
2. This also updates the "high" PHP version for the "quicktest" against PHPCS `dev-master` from PHP 7.4 to PHP 8.0 by using `latest` which translates to "latest stable PHP release".
e128f36 to
52fa5ce
Compare
This commit:
* Adds a GH Actions workflow for the CI checks which were previously run on Travis in the `test` and `coverage` stages.
* Removes the, now redundant, `.travis.yml` configuration.
* Updates the `.gitattributes` file.
* Updates the "Build Status" badge in the Readme to use the results from the GH `Test` Actions runs.
Notes:
1. After a lot of experimenting, I've split this into three (four) jobs.
- Linting against high/low PHP versions of each major first.
This removes the linting against interim version of PHP majors, as, to be fair, that shouldn't really be make much difference anyhow.
- Running the tests without code coverage.
- Running the tests with code coverage.
- And a final job to let Coveralls know that all parallel run coverage jobs have finished.
2. Each of these jobs has a `needs` dependency on the previous job to prevent it from starting if the previous job failed.
While not 100% necessary, this is just an efficiency tweak, being kind to the free service being offered as we know that if linting fails, the tests will fail etc.
3. The builds in the `test` and `coverage` jobs are essentially the same as previously run on Travis, though PHP 8.0 is now fully accounted for and `8.1` is set as `nightly`.
4. Previously, this "stage" would run on all `pull requests` events.
The current set-up still does so, with one addition: pushes to `master` (merges) will now also use this workflow instead of the quicktest.
This replaces the full run on tagging a release, meaning that thing will essentially be the same.
5. As there are a couple of jobs which are "allowed to fail" (`experimental` = true), the build status will unfortunately show as "failed", even though all non-experimental jobs have succeeded.
This is a known issue in GHA: https://github.com/actions/toolkit/issues/399
52fa5ce to
c4b4fe3
Compare
wimg
approved these changes
Dec 21, 2020
wimg
left a comment
Member
There was a problem hiding this comment.
Big step forward (or sideways, since Travis was working...) :-)
Member
Author
Well, it hasn't been for a while... Either way, for the record (already mentioned to @wimg off-GH): I discovered that the Coveralls upload for the PHP 5.4 code coverage test runs is not working at the moment. I'm looking into some options to fix this. Bear with me. |
Uploading the test coverage `clover.xml` file to Coveralls works with PHP Coveralls 2.4+, but not with PHP Coveralls 1.x, but version 2.x is only compatible with PHP 5.5+, while we are running some coverage builds on PHP 5.4. Luckily a "hot swap" of the PHP version during a job turns out to work just fine, so adding steps to do so, which allows us to only use PHP Coveralls 2.x for the executing the upload to Coveralls.
jrfnl
added a commit
to WPTT/WPThemeReview
that referenced
this pull request
Feb 19, 2021
* Add the sniff stage * Remove travis file No need to clutter checks * Setup xml lint * Remove XMLLINT_INDENT export * Add empty test.yaml file * Fix the diff? * Trying to fix the diff * Try to run on xenial * Add lint check and composer validation to workflow * Trying to add test stage * Try to exclude certain versions Only allow master phpcs and develop wpcs on one PHP version, and that should be allowed to fail * Refine exclusions We only need master - master and lowest - lowest for every PHP version, not all 4 possible combinations. * PHP exclusions cannot be in the list apparently * Adding php 5.4 and 5.5 * Fix typo * Add nightly to build * Fix the sniff workflow * Update suggestions for dealerdirect plugin So that it works with the composer v2 * Remove travis reference * Fix the minor grammar issues in the Contributing file Also change the theme review team repo to correct repo name. It's themes team now. * Fix the sniff stage accordding to the PR suggestions Also, some ideas from PHPCompatibility PR PHPCompatibility/PHPCompatibility#1260 were implemented as well (composer dependencies and xml violations). * Update test workflow with the suggestions from the PR * Trying to fix the invalid yaml issue * Change quotation marks in the condition * Specify the PHP 8 branch that can, and will, fail * Add continue-on-error directive * Fix the badges and the repo name * GH Actions: remove the ignores for markdown files ... as it would prevent PRs with only markdown changes from being merged due to required statuses. * 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. Ref: https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/ * GH Actions: remove GH token from setup PHP step ... as it shouldn't be needed. * GH Actions: add some inline documentation for some steps * GH Actions: add lint workflow As things were, the code was only ever linted against PHP 7.4, while in the original Travis workflow, the code was linted against every single supported PHP version. This adds a hybrid workflow in which the code is linted against the high/low of each PHP major within the range of supported PHP versions. * GH Actions: use Parallel-Lint for PHP linting Not a necessity, but allows for: 1. Faster linting due to the parallel option. 2. The ability to display linting errors in the PR via the cs2pr tooling. * GH Actions: fix up the matrix The minimum supported PHPCS version according to the `composer.json` file, is `3.3.1`, so that should be the minimum version to test against. Similarly, the minimum supported WPCS version according to the `composer.json` file is `2.2.0`. For PHP 7.4 and 8.0, the builds have to be specified explicitly as PHP 7.4 only has PHPCS runtime support as of PHPCS 3.5.0 and PHP 8.0, as of PHPCS 3.5.7. As the matrix builds only test high PHPCS with high WPCS and low PHPCS with low WPCS, for PHP 7.2, in the Travis config, explicit builds were specified to reverse that combination, i.e. high PHPCS with low WPCS and low PHPCS with high WPCS. This has now been implemented here as well. Includes adding some inline documentation to the matrix. Note: I've not added a build against PHP 8.1/nightly as the PHPUnit version we are forced to use via PHPCS (PHPUnit 7.x) is not compatible with PHP 8.1, so the build wouldn't pass no matter what. * GH actions: set the PHP ini values * GH actions: remove duplication of setting of PHPCS/WPCS versions * GH actions: handle Composer install on PHP 8 * Readme: fix up the badges ... based on latest example code generate on the Actions page. * Remove extra quotes and reduced spacing from 4 to 2 spaces Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CI: switch to GitHub Actions - step 1: sniff stage
This commit:
sniffstage.While these aren't 100% CS (= code style) checks, for the badge and workflow display,
CSstill seemed the most descriptive name..travis.ymlconfiguration.Notes:
CI: switch to GitHub Actions - step 2: quick test stage
This commit:
quickteststage..travis.ymlconfiguration.Notes:
pushevents, except when merging tomasterordevelop.The current set-up still does so, with one exception: pushes to
developwill now use this quicktest instead of the full test.developis a protected branch anyhow, so all merges will already have had the full test run in the pull request.For merges to
master, the full Test will still be used as that will generally mean we're getting ready to tag a release and some extra safety checking before a release is not a bad thing.dev-masterfrom PHP 7.4 to PHP 8.0 by usinglatestwhich translates to "latest stable PHP release".CI: switch to GitHub Actions - step 3: test and coverage stage
This commit:
testandcoveragestages..travis.ymlconfiguration..gitattributesfile.TestActions runs.Notes:
This removes the linting against interim version of PHP majors, as, to be fair, that shouldn't really be make much difference anyhow.
needsdependency on the previous job to prevent it from starting if the previous job failed.While not 100% necessary, this is just an efficiency tweak, being kind to the free service being offered as we know that if linting fails, the tests will fail etc.
testandcoveragejobs are essentially the same as previously run on Travis, though PHP 8.0 is now fully accounted for and8.1is set asnightly.pull requestsevents.The current set-up still does so, with one addition: pushes to
master(merges) will now also use this workflow instead of the quicktest.This replaces the full run on tagging a release, meaning that thing will essentially be the same.
experimental= true), the build status will unfortunately show as "failed", even though all non-experimental jobs have succeeded.This is a known issue in GHA: Please support something like "allow-failure" for a given job actions/runner#2347
To Do: