CI: move away from Travis-ci.org

[jrfnl]

Travis-ci.org is shutting down.

I know that in the past one could already manually move accounts over from .org to .com by a) signing up to .com, b) enabling the migration beta and then moving whichever repo you wanted to move.

It might be worth checking who within the Automattic organisation has the right access rights to the Travis account to do so and approach them about getting this repo moved (to prevent CI potentially having downtime when the move happens unexpectedly or when travis-ci.org is shut down without the account having been moved).

Alternatively, now might be a good time to move the CI to GitHub actions and away from Travis.

If the preference is to move to travis-ci.com, can someone from within Automattic please take ownership of this ticket and move that forward ?

If the preference is to move to GH actions, I can set that up and send in a PR.

[GaryJones]

I'm happy to go the GH route - it's one less service to have to connect with.

[jrfnl]

@GaryJones Shall I implement a "quick test" for pushes, similar to what we have for WPCS ?

In practice that would mean that for pushes to arbitrary branches and merges to develop, the tests will only be run against a limited matrix of high/low PHP/PHPCS/WPCS.

For pull requests and merges to master, the tests would still be run against a full matrix of all supported PHP versions in combination with various combinations of high/low PHPCS and WPCS.

[GaryJones]

Shall I implement a "quick test" for pushes, similar to what we have for WPCS ?

@jrfnl Yes please :-) The current 38 checks seems excessive for potentially work-in-progress pushes.

[jrfnl]

@GaryJones I'll action that after the initial PR has been merged.

Have you got an opinion about the other suggestions which I left in the PR ?

Some ideas for potential future iterations

• (Re-)enable a test run against PHP 8.1/nightly.
• Add a "Quick test" script to run the tests against a more limited matrix for pushes only.
• Implement use of the PHP Parallel Lint package for linting the PHP code and showing any errors in-line in a PR.
• Implement use of the PHPCS feature completeness check.
• Add a test run against PHPCS 4.x when it is closer to being released.
• Register with coveralls.io (or another service) and start recording and checking code coverage for builds.

[GaryJones]

(Re-)enable a test run against PHP 8.1/nightly.

Yes, but not urgent.

Implement use of the PHP Parallel Lint package for linting the PHP code and showing any errors in-line in a PR.

How much will that speed up the linting?
Do we have any intentionally broken PHP that this will flag that isn't already?
Overall, yes - nice to have, but not desperate.

Implement use of the PHPCS feature completeness check.

Sure - does that include the need for a set of docs files? Can it be informative and non-blocking?

Add a test run against PHPCS 4.x when it is closer to being released.

Yes please.

Register with coveralls.io (or another service) and start recording and checking code coverage for builds.

What sort of code coverage % do we currently have?
Can this be done within the GitHub Actions rather than using an external service to display the results?
Overall, yes :-)

[GaryJones]

Do we have any intentionally broken PHP that this will flag that isn't already?

Run unit test for the new/PR'd PHP 5.5 GH Action and 5.4 is giving out a:

PHP Parse error: syntax error, unexpected '*' in /home/runner/work/VIP-Coding-Standards/VIP-Coding-Standards/WordPressVIPMinimum/Sniffs/Performance/LowExpiryCacheTimeSniff.php(180) : eval()'d code on line 1

[jrfnl]

Let's keep this issue open until some of the other suggestions have also been addressed.

(Re-)enable a test run against PHP 8.1/nightly.

Yes, but not urgent.

Agreed. I'm thinking we should leave this till the summer (2021) when a) there will be an PHP 8.1 alpha/beta to test with locally and b) it will become clearer what PHPUnit versions will support PHP 8.1.

That is, unless a simple, working solution presents itself in the mean time.

One thing I've been thinking about, which is related to this, is to:

• either refactor the PHPCS unit test base in PHPCS 3.x to support a wider range of PHPUnit versions, but I'm not sure Greg would be open to this.
• or to provide an abstract base test class via PHPCSUtils which would work cross-version PHPUnit and would only require the use statement to be adjusted for external standards to switch over to it.

That last solution would also get rid of the hassle of having to use the GH source for testing against PHPCS 4.x, which no longer includes the test base classes in the distribution zip.
The only real difference would be that it may not include support for adding the 46 sniff test files generated 190 unique error codes; 0 were fixable (0%) line at the bottom of the test run.

Via that last solution, I would also want to offer more extended test abilities, like:

• testing that the correct error code is thrown where expected, so a change in a sniff changing the error code being thrown for a particular code snippet won't be "invisible" anymore from a testing perspective.
  Case in point - a change in PR IncludingFile: Add custom keywords to detect to reduce false positives #626 is doing exactly that and I caught it in the review, but it would have been easy to overlook.
• and testing (for select test cases), that the error message is as expected. This is particularly interesting for sniffs which do things with code snippets in the error message where we'd want to guard that functionality.
  Example: the WordPress.PHP.NoSilencedErrors sniff.

Implement use of the PHP Parallel Lint package for linting the PHP code and showing any errors in-line in a PR.

How much will that speed up the linting?

As this is not a huge code base, the speed up won't be significant, but the error messages shown would be much more descriptive (with syntax highlighted code snippet and all). So it is more a usability improvement.

Also, in the current script, linting is only run on PHP 7.4 in the "basic checks" workflow.
It might be a good idea to have a separate job before the tests (with a needs: lint for the tests) to check against a matrix of low/high PHP or to add linting as a step before the test run in the test job. After all: why waste time on running the tests when they would eventually fail due to a parse error anyway.

Linting just against PHP 7.4 will not catch everything in that regard (and there is no dependency for that job to pass before running the tests).

Do we have any intentionally broken PHP that this will flag that isn't already?
Overall, yes - nice to have, but not desperate.

Implement use of the PHPCS feature completeness check.

Sure - does that include the need for a set of docs files? Can it be informative and non-blocking?

The feature completeness check tool has two check levels: check for tests only or check for tests + docs.
In WPCS, for instance, we are only checking for tests, as not all sniffs have docs yet.
Also see: https://github.com/PHPCSStandards/PHPCSDevTools/#checking-whether-all-sniffs-in-a-phpcs-standard-are-feature-complete

A sniff being pulled without tests, IMO, should be blocked anyway.
Once all sniffs have docs, we can turn on the docs check to make sure that no new sniffs are being introduced without docs.

In all cases, it can be made non-blocking by using continue-on-error: true on the job step, though that kind of defeats the purpose.

Add a test run against PHPCS 4.x when it is closer to being released.

Yes please.

Also see my remark above about the tests which is loosely related to this.

Register with coveralls.io (or another service) and start recording and checking code coverage for builds.

What sort of code coverage % do we currently have?

It's one of the things I've been monitoring locally when I have been working on individual sniffs.

Can this be done within the GitHub Actions rather than using an external service to display the results?

Yes, we can use the text reporter instead of checkstyle to see the results in the action.

We'd need to investigate how we can make the build fail on a decrease in code coverage in that case though. That part is what external services do automatically (and with configurable settings).

Overall, yes :-)

[jrfnl]

Closing this issue as the immediate remaining actions have been pulled & merged.

For what still remains to be addressed at some point in the future, separate issues have been opened (#659, #660, #661).