[wp-trac] [WordPress Trac] #50401: Tests: Move tests into Github Actions

Tue Oct 13 19:27:27 UTC 2020

#50401: Tests: Move tests into Github Actions
-------------------------------------+-----------------------------
 Reporter:  whyisjake                |       Owner:  desrosj
     Type:  enhancement              |      Status:  assigned
 Priority:  normal                   |   Milestone:  Future Release
Component:  Build/Test Tools         |     Version:
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch needs-testing  |     Focuses:  performance
-------------------------------------+-----------------------------
Changes (by desrosj):

 * keywords:  has-patch => has-patch needs-testing
 * version:  5.5 =>

Comment:

 I've spent a chunk of time working through this, and I think I have a PR
 that can be considered. Before I dive in to what the PR contains, I want
 to document what the current CI processes are across the two services
 WordPress currently utilizes.

 **Appveyor**

 This service is used to test that NPM errors are not encountered on
 Windows based environments (context: #44276).

 The workflow:
 - Runs **only** on pushes to `master` branch.
 - Installs the NodeJS version specified in the config file (currently
 `10.13.0`).
 - Installs `npm` globally.
 - Runs `npm install` to install `package.json` dependencies.
 - Runs `npm run build` to ensure WordPress builds correctly.

 Build failures are reported to #core in Slack when a build fails, or when
 a build succeeds with a previous failure.

 **Travis CI**

 The Travis CI configuration has many more jobs within a build. Here are
 their summaries:

 - **E2E Tests:** Runs the E2E test suite using `npm run test:e2e`.
 - **PHP Linting:** Runs the code base through PHPCS rulesets to check that
 coding standards are followed.
   - The `phpcs.xml.dist` ruleset is run against the entire codebase with
 warnings suppressed (`-n`) using `composer lint:errors`.
   - The `phpcs.xml.dist` ruleset is then re-run, but only against the
 `test` directory without warnings suppressed (warnings are not allowed
 within the test suite) using `composer lint tests`.
 - **PHP Compatibility:** The `phpcompat.xml.dist` ruleset is run against
 the codebase using `composer compat`.
 - **JavaScript Linting & Testing:** JSHint linting and QUnit tests are run
 on the codebase using `npm run grunt travis:js`.
 - **PHPUnit Tests:** All tests below are run using `npm run test:php`
 within the WordPress Docker container unless otherwise noted.
   - **PHP 8.0:** The PHPUnit tests are run within the Docker container
 with `npm run test:php-composer`. This ensures the Composer installed
 version of PHPUnit (with the necessary modifications made to backport the
 required parts for PHP 8 support).
   - PHP 7.4
   - PHP 7.3
   - PHP 7.3 with memcached
   - PHP 7.2
   - PHP 7.1
   - PHP 5.6

 If any job within the build fails, the build is considered failed and the
 results are posted to #core in Slack. All failures and successful builds
 with a previous failure. Pull requests are ignored.

 **GitHub Actions**

 In GitHub actions, builds are called "workflows". Each workflow is
 comprised of jobs, and jobs are comprised of steps with actions (if you're
 not familiar with GitHub Actions yet, I recommend
 [https://docs.github.com/en/free-pro-team@latest/actions/learn-github-
 actions/introduction-to-github-actions#the-components-of-github-actions
 reading this quick breakdown]).

 I've set up 7 new workflow files to accomplish all of the current testing
 performed outlined above.

 - `verify-npm-windows.yml`: Replaces the NPM related testing on Windows
 currently handled by Appveyor.
 - `coding-standards.yml`: Runs the PHP coding standards checks done with
 PHPCS.
 - `end-to-end-tests.yml`: Runs the E2E test suite.
 - `javascript-tests.yml`: Runs JSHint linting and QUnit tests.
 - `php-compatibility-testing.yml`: Runs the PHP compatibility checks with
 PHPCS.
 - `test-wordpress.yml`: Runs the PHPUnit tests on various versions of PHP.
 - `welcome-new-contributors.yml`: Posts a comment welcoming new
 contributors with some helpful links and resources (when opening their
 first PR).

 Some notes:
 - The PHPUnit workflow is broken down into two steps. The first downloads
 the additional files needed (WordPress Importer, for example), installs
 NPM dependencies, and builds WordPress before zipping and uploading the
 result of these actions as an artifact. The artifact is then downloaded in
 the second step (which expands the matrix of PHP versions to test) before
 setting up Docker and running the PHPUnit tests. Theoretically, this
 should be faster (WordPress is built once vs. once for each version of
 PHP), the ZIP could be downloaded and inspected if there are weird issues
 with test results, and these artifacts could potentially be useful in some
 way for someone looking to test a WordPress at a specific commit (they
 could download the zip into a local environment and just run WordPress
 without having to run `npm install/npm run build`). This may be overkill
 though, and these artifacts could be deleted.
 - PHPCS is currently run from inside the Docker container on Travis. I
 believe that this was done because the version of PHP (and the environment
 in general) was unreliable in Travis. In Actions, there is a
 [https://github.com/shivammathur/setup-php setup-php action] that is more
 reliable, so I don't think we need to utilize the Docker containers for
 these scans. This will also not be a problem when we start backporting
 workflows because coding standards checks were not added to the test suite
 until WordPress 5.1, which ran the checks using PHP 7.2. This applies to
 both the coding standards and PHP compatibility workflows.
 - The PHP compatibility ruleset explicitly specifies `./src/` as the only
 directory to run the checks on. However, `lint-action` has `.` hard coded
 when invoking `phpcs`. I was able to fix this by specifying `/tests/` as
 an exclude pattern within the ruleset.
 - Unzipping files cannot be performed by an action. The file permissions
 and ownership did not match the runner's and will cause permission issues
 with Docker.

 There are a handful of items that need to be fixed before these can be
 merged:
 - The `PHP_FPM_UID` and `PHP_FPM_GID` environment variables need to be
 made dynamic. They are currently hard coded. I was having trouble figuring
 out the correct syntax to set those variables to the results of `id -u`
 and `id -g` ,respectively.
 - The Slack notifications will need to be configured to work similarly to
 how they do currently for Travis builds for failed workflows. There seem
 to be a lot of Slack actions in the GitHub Marketplace, so looking for a
 recommendation form someone that has set these up in another project. This
 will also require a Slack admin, which @helen should be.
 - The test reporter will need to be configured with the necessary API key
 added as a repository secret.
 - For some reason, failures are not always reported on the correct
 workflow. For example, if coding standards or compatibility checks fail,
 they frequently are reported on [https://github.com/desrosj/wordpress-
 develop/actions/runs/304876868 other random workflows run from the same
 commit].
 - Because the lint-action hard codes `.` as the directory to run PHPCS,
 re-running the ruleset and allowing warnings within just the `test`
 directory is not currently being done. There is an
 [https://github.com/wearerequired/lint-action/pull/46 open pull request on
 the action's repository to allow this], but I haven't figured out the best
 way around this. We could introduce a new PHPCS ruleset that uses the one
 defined within `phpcs.xml.dist` and adds an exclude pattern for `src`, but
 that's not really ideal.
 - There are some issues with some permission issues with actions and
 forks. I am not clear how these will translate once these workflows are
 merged. For example, the [https://github.com/wearerequired/lint-
 action#limitations lint-action] I am using states "The action doesn't have
 permission to create annotations for commits on forks and can therefore
 not display linting errors." But I'm not sure if the annotations will
 display within the context of the pull request itself. It's also not clear
 if these issues will prevent the Welcome workflow from working correctly
 when a PR is opened from a forked repository.

 Here are some other "would be nice" items that don't necessarily need to
 be solved right now:
 - The version of NodeJS/NPM that is installed is currently hard coded as
 12. This is fine for now, but this will cause a problem when the workflows
 are backported to older branches. Ideally, the version specified within
 `.nvmrc` should be read and installed instead. `nvm` is installed in the
 runner, so `nvm install` is an option, but using the `actions/setup-node`
 action seemed like the better path.
 - The idea behind Appveyor was to perform more regular testing for Windows
 based environments. The PHPUnit tests could easily be run on both Windows
 and Ubuntu by adding `windows-latest` to the `os` strategy. But there are
 a few issues that need to be solved first (totally outside the scope for
 this ticket, but just want to report all of my findings here for future
 reference):
   - The `bridge` network driver is not supported in Windows. Excluding
 that line from the `docker-compose.yml` file seems to work fine (`bridge`
 is the default on Linux machines, and Windows uses a different default),
 but that then causes an error attempting to pull the `mysql:5.7`
 container.

 After fixing the blockers outlined above and allowing some time for
 feedback (this is my first dive into GitHub actions, so I may very well be
 over complicating something somewhere), I think we could commit these
 workflows and keep Travis CI/Appveyor as the preferred testing services
 for a week or so in order to identify any issues.

 😅 I think I covered everything I needed to touch on! I'll also upload a
 patch of the changes here in case someone prefers to look at patches in
 Trac.

 The PR attached to the ticket will not run the workflows. TO see them in
 action, I have a PR against my fork that will show them running:
 https://github.com/desrosj/wordpress-develop/pull/2.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/50401#comment:9>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform