Move from TravisCI to GitHub Actions #1965

desrosj · 2020-12-09T19:03:29Z

With recent changes to the TravisCI's service, the organization is moving over to GitHub Actions. This PR will convert all of the builds in Travis to jobs in workflows within GitHub Actions.

This converts the TravisCI configuration into 4 workflow files representing the 4 different stages of testing within Travis.

I've done my best to replicate the testing stages with all of their needed parts with a few exceptions.

There is a workflow_run event type that is supposed to allow you to configure a workflow to run based on the results of other workflows. I could not get that to work in my testing. The value of that would be setting the quick tests and unit tests to only run when the sniffs and ruleset checks complete successfully.

While it would be nice to set up this dependency, I think we could merge regardless in the interest of getting tests running again.

I also noticed that the nightly build was running with PHPCS 3.5.0, resulting in a failure. I updated this to use 3.5.7 instead.

dingo-d · 2020-12-09T20:21:23Z

I've started to work on something similar for wptt sniffs. Still need to tackle the tests. I've decided to split tests and sniff part into separate workflows: WPTT/WPThemeReview#261

The tricky part that is missing in GH actions is that there are still no allowed failures. Not sure how that will impact the tests.

jrfnl · 2020-12-10T03:27:09Z

The tricky part that is missing in GH actions is that there are still no allowed failures.

@dingo-d From what I've seen, there is (well, in a way), just not in the scripting part.

With Travis you had one status per complete run reporting and that status is added to required statusses to pass before a PR can be merged, which is why "allowed failures" is important.

With GH actions - you get individual reports in a PR on all the different build steps and instead, for those "builds" which are allowed to fail, you don't add a checkmark next to them in the "required statusses" for PRs to be merged.

Does that make sense ? or am I overlooking something ?

@desrosj One thing I don't know: while the PR has not been merged yet - how can we see what the current status would be and whether it works ?
I looked at the Actions tab here in the repo, but nothing is showing up yet.

dingo-d · 2020-12-10T08:16:47Z

how can we see what the current status would be and whether it works?

I use https://github.com/nektos/act/ top locally test the actions. Works ok with this instruction.

The actions need to be in the .github/workflows folder for them to run, and it looks like @desrosj put the test yaml file in .github/sniffs.yml/tests.yml 🤷🏼‍♂️ Could be a typo.

@jrfnl regarding the allowed failures, there is a continue-on-error, but I don't think that's the same. At least not from what I've read on the support question.

desrosj · 2020-12-11T01:05:40Z

Sorry, I ran out of time to finish this week. This is definitely near the top of my list though.

The directory is indeed a typo. I'll get it all wrapped up early next week, and we can get everything moved over!

dingo-d · 2020-12-12T11:19:35Z

I'm making some progress on WPThemeReview implementation: https://github.com/WPTT/WPThemeReview/pull/261/checks

There are a couple of things I need to do:

I plan on separating 'nightly' (8.1) into its own job that is allowed to fail. I may add it to the original, and do some checks in the run command for this once I play with it a bit

EDIT:

Managed to get 5.4 and 5.5 working and fixed the sniff stage 🎉 I'm not 100% sure I correctly matched the exclusions (would appreciate another set of eyes).

The tests are failing on PHP 8 and 8.1 as expected and on 7.4 with PHPCS dev-master and WPCS dev-develop, but I need to figure out how to make this excluded (allowed failures).

Feel free to check the code and reuse things if that will speed things up 🙂

- Update each file with full inline documentation. - Update each file to use consistent naming and approaches across workflow files. - Remove travis.yml file.

desrosj · 2020-12-17T19:18:25Z

Alright, I think this is good for a review! I updated the PR description with more details to call out.

One thing that I did not include here is an update to the README file with new badges. Those can be grabbed from the workflow pages. I am happy to do that in a follow up PR, or I can include them here.

.github/workflows/ruleset.yml

dingo-d · 2020-12-18T08:38:14Z

Wow, amazing job! I'll definitely use some of this in WPThemeReview 😄 I've left one question regarding matrix usage.

The only thing that I'm worried about is that we get a failed test (which is allowed to fail), but we don't really have any easy way to distinguish allowed vs not allowed failures. I hope GH fixes this quickly.

desrosj · 2020-12-18T14:12:49Z

The only thing that I'm worried about is that we get a failed test (which is allowed to fail), but we don't really have any easy way to distinguish allowed vs not allowed failures.

Hmm, we could add a prefix to the job names that are allowed to fail. Maybe something like changing PHP 7.4 on PHPCS 4.0.x-dev as 3.9.99 to (Dev) PHP 7.4 on PHPCS 4.0.x-dev as 3.9.99.

jrfnl

Hi @desrosj Thanks for taking on this task! Awesome job with a complex script.

I've left some suggestions and questions inline.

Additionally:

The .gitattributes file needs to be updated (removing .travis.yml)
The CI badges in the readme need to be updated. As there are now several workflows, this may need a think through of how we want to present the status.
A search should be done of the repo to see if there are other references to Travis (possibly in the contributing docs?) and if found, these should be removed/texts rewritten.

Other than that (not your concern):

The required statuses for the branch protection will need to be updated for this to be mergable.
This should only be done once the job names have been finalized.
All open PRs will need to be rebased once this PRs has been merged.

.github/workflows/quicktest.yml

jrfnl · 2020-12-19T00:14:35Z

.github/workflows/quicktest.yml

+      - name: Checkout repository
+        uses: actions/checkout@v2
+
+      - name: Get Composer cache directory


You may want to have a look at https://github.com/marketplace/actions/install-composer-dependencies which is an action which handles running the composer caching + install. May simplify the script a little more.

I think I have configured this properly and it seems to work, however it's not clear if there are any potential conflicts with running requiring a specific version of PHPCS in the step before and then the action running composer install after.

.github/workflows/quicktest.yml

.github/workflows/unit-tests.yml

The `korelstar/xmllint-problem-matcher` tool will allow to display the results from an xmllint action directly in the PR. Ref: https://github.com/staabm/annotate-pull-request-from-checkstyle

This was already implemented based on feedback for the QuickTest workflow, but not for the other workflows/jobs.

jrfnl · 2021-02-19T16:24:16Z

FYI: I did another review and found a number of things which would still need to be updated.

As this PR is a blocker for all other PR/progress in this repo, leaving it open any longer is not desirable.
With that in mind, @desrosj and me had a call this afternoon to talk through the review and the additional changes I was suggesting.

I'll be adding a number of additional commits to this PR based on our discussion to touch up and finalize this PR.

@dingo-d @GaryJones Please squash all commits on merge of this PR so as to keep the repo history semi-clean.

jrfnl

With the additional changes now pushed after the call/discussion with @desrosj, IMO this PR is ready to be merged.

* `Install PHPCS` -> `Set PHPCS version` In that particular step, we're not actually _installing_ PHPCS (`--no-update`), just updating the version being required. * `Check that all sniffs have unit tests` => `Check sniff feature completeness` The script run in this step checks feature completeness for PHPCS sniffs. The fact that the documentation check is currently disabled is irrelevant (and runs the risk of when it _does_ get enabled for the step name to become incorrect). * `Check the ... ruleset` => `Test the ... ruleset` This is a _test_ where we are testing that no unexpected errors/warnings are being thrown. Also fixes a typo.

Remove some documentation which isn't relevant (anymore) or which was in the wrong place. Add some documentation where necessary.

The name of the workflow is used in the badges displayed in the Readme. The name as was would make for an awkwardly long badge name.

PHPCS doesn't have named branches for the various releases. It has tags and the `master` branch, so `phpcs_version` is more descriptive.

jrfnl · 2021-02-19T16:35:03Z

@dingo-d @GaryJones I've just updated the protected branches. The status checks from Travis have been removed and the new ones from the GH Actions workflows have been added.

... as the `ignore_warnings_on_exit` does not seem to get respected when this is split into two commands.

... as the cs2pr combined with the `--runtime-set ignore_warnings_on_exit 1` appears to be buggy. To be revisited later.

.github/workflows/ruleset-checks-sniffs.yml

.github/workflows/unit-tests.yml

dingo-d

Left few small comments, otherwise looks great. Awesome work @desrosj and @jrfnl !

Co-authored-by: Denis Žoljom <dingo-d@users.noreply.github.com>

desrosj · 2021-02-19T19:07:47Z

Thanks @dingo-d! Addressed those, I think we're good to go now!

* upstream/develop: (106 commits) PHP/NoSilencedErrors: add `imagecreatefromwebp` PHP/NoSilencedErrors: add libxml_disable_entity_loader() GH Actions: turn display_errors on Bug report template: various improvements QA: import all used classes QA: use fully qualified global constant references QA: remove unnecessary assignment CS: get rid of "commented out code" warnings OneObjectStructurePerFile: move from `Extra` to `Core` GH Actions: don't use cs2pr in quicktest GH Actions: report CS violations in the PR GH Actions: get the tests running on PHP 8.1 (nightly) PHP 8.1 compatibility: fix deprecation notice [2] PHP 8.1 compatibility: fix deprecation notice [1] GH Actions: don't test against PHPCS 4.x (yet) Security/EscapeOutput: bug fix - allow for basename() being fully qualified 3.0: Move "is unit test" utility functions to dedicated trait (WordPress#1960) Move from TravisCI to GitHub Actions (WordPress#1965) Travis: add build against PHP 8.0 PHP/RestrictedPHPFunctions: update error message ...

Move the "sniffs" stage over to a GitHub Action workflow.

a5c7f81

desrosj self-assigned this Dec 9, 2020

desrosj added 15 commits December 13, 2020 10:39

Update Sniff workflow.

7badc59

Load libxml2-utils.

34cb7c9

Correct extension syntax.

44e14cc

Fix typo.

51a0628

Manually install xmllint.

116e189

First pass at a Ruleset stage workflow.

1f8c104

Pass the matrix defined PHP version.

4282a4f

Correct the ruleset name.

a775e0d

Set error reporting through the setup-php action.

f1741a8

Correct typo.

28c0d10

First pass at quick tests.

924fc14

Correct casing of environment variable.

dc804f9

Update the inline documentation for the sniff workflow.

036d9b4

Cleanup the ruleset workflow.

0d10e7c

Add full test suite file and other changes.

090e3c4

- Update each file with full inline documentation. - Update each file to use consistent naming and approaches across workflow files. - Remove travis.yml file.

desrosj marked this pull request as ready for review December 17, 2020 18:28

desrosj requested review from dingo-d and jrfnl December 17, 2020 19:16

dingo-d reviewed Dec 18, 2020

View reviewed changes

.github/workflows/ruleset.yml Outdated Show resolved Hide resolved

jrfnl requested changes Dec 19, 2020

View reviewed changes

jrfnl added 2 commits February 19, 2021 17:02

GH Actions: report XML lint violations in the PR

7b020f6

The `korelstar/xmllint-problem-matcher` tool will allow to display the results from an xmllint action directly in the PR. Ref: https://github.com/staabm/annotate-pull-request-from-checkstyle

GH Actions: use an action to handle the caching for Composer

c9b65e1

This was already implemented based on feedback for the QuickTest workflow, but not for the other workflows/jobs.

jrfnl approved these changes Feb 19, 2021

View reviewed changes

jrfnl added 7 commits February 19, 2021 17:29

GH Actions: split long line with multiple commands

815da61

GH Actions: minor inline documentation tweaks

e135bdb

Remove some documentation which isn't relevant (anymore) or which was in the wrong place. Add some documentation where necessary.

GH Actions: shorter name for the basic QA workflow

9dfb4e9

The name of the workflow is used in the badges displayed in the Readme. The name as was would make for an awkwardly long badge name.

GH Actions: rename phpcs_branch to phpcs_version

d6118b6

PHPCS doesn't have named branches for the various releases. It has tags and the `master` branch, so `phpcs_version` is more descriptive.

GH action: miscellaneous tweaks

050083f

Readme: replace the CI status badges

d53784e

jrfnl force-pushed the migrate-to-github-actions branch from 6870f14 to d53784e Compare February 19, 2021 16:29

GH Actions: try slightly different command syntax for CS check

a3b86fc

... as the `ignore_warnings_on_exit` does not seem to get respected when this is split into two commands.

jrfnl force-pushed the migrate-to-github-actions branch from e42613b to a3b86fc Compare February 19, 2021 16:42

GH Actions: undo "show CS violations in PR"

b33a8f0

... as the cs2pr combined with the `--runtime-set ignore_warnings_on_exit 1` appears to be buggy. To be revisited later.

dingo-d reviewed Feb 19, 2021

View reviewed changes

.github/workflows/ruleset-checks-sniffs.yml Outdated Show resolved Hide resolved

dingo-d reviewed Feb 19, 2021

View reviewed changes

.github/workflows/unit-tests.yml Outdated Show resolved Hide resolved

dingo-d reviewed Feb 19, 2021

View reviewed changes

.github/workflows/unit-tests.yml Outdated Show resolved Hide resolved

dingo-d requested changes Feb 19, 2021

View reviewed changes

desrosj and others added 3 commits February 19, 2021 14:05

Remove quotes around action name.

c2b6080

Co-authored-by: Denis Žoljom <dingo-d@users.noreply.github.com>

Remove extra space.

6872b1d

Co-authored-by: Denis Žoljom <dingo-d@users.noreply.github.com>

Remove quotes around action name.

42f39a0

Co-authored-by: Denis Žoljom <dingo-d@users.noreply.github.com>

dingo-d approved these changes Feb 19, 2021

View reviewed changes

dingo-d merged commit 862d970 into develop Feb 19, 2021

dingo-d deleted the migrate-to-github-actions branch February 19, 2021 19:32

jrfnl removed the Status: Review ready label Sep 23, 2022

KamilBaczkowski mentioned this pull request Oct 18, 2023

[THEME-3651] Update the PHPCS rules Placester/WordPress-Coding-Standards#2

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move from TravisCI to GitHub Actions #1965

Move from TravisCI to GitHub Actions #1965

desrosj commented Dec 9, 2020 •

edited

Loading

dingo-d commented Dec 9, 2020

jrfnl commented Dec 10, 2020

dingo-d commented Dec 10, 2020

desrosj commented Dec 11, 2020

dingo-d commented Dec 12, 2020 •

edited

Loading

desrosj commented Dec 17, 2020

dingo-d commented Dec 18, 2020

desrosj commented Dec 18, 2020

jrfnl left a comment

jrfnl Dec 19, 2020

desrosj Feb 17, 2021

jrfnl commented Feb 19, 2021

jrfnl left a comment

jrfnl commented Feb 19, 2021

dingo-d left a comment

desrosj commented Feb 19, 2021

Move from TravisCI to GitHub Actions #1965

Move from TravisCI to GitHub Actions #1965

Conversation

desrosj commented Dec 9, 2020 • edited Loading

dingo-d commented Dec 9, 2020

jrfnl commented Dec 10, 2020

dingo-d commented Dec 10, 2020

desrosj commented Dec 11, 2020

dingo-d commented Dec 12, 2020 • edited Loading

EDIT:

desrosj commented Dec 17, 2020

dingo-d commented Dec 18, 2020

desrosj commented Dec 18, 2020

jrfnl left a comment

Choose a reason for hiding this comment

jrfnl Dec 19, 2020

Choose a reason for hiding this comment

desrosj Feb 17, 2021

Choose a reason for hiding this comment

jrfnl commented Feb 19, 2021

jrfnl left a comment

Choose a reason for hiding this comment

jrfnl commented Feb 19, 2021

dingo-d left a comment

Choose a reason for hiding this comment

desrosj commented Feb 19, 2021

desrosj commented Dec 9, 2020 •

edited

Loading

dingo-d commented Dec 12, 2020 •

edited

Loading