Migrate Travis CI tasks to GitHub Actions #1040

eason9487 · 2021-10-06T09:30:26Z

Changes proposed in this Pull Request:

Somehow seems all repos under Woo were stopped CI tasks on Travis CI. This PR migrates CI tasks to GitHub Actions:

Run PHP unit tests
Run JavaScript unit tests
Check PHP coding standards
Lint JavaScript and CSS
Check bundle size
Add status badges to README

Additional notes

Since the WC_VERSION in install-wp-tests.sh doesn't take from a command param, the WC_VERSION of matrix values in .travis.yml have never been used when running PHP unit tests on Travis CI. Therefore, I didn't migrate the matrix of specified WC_VERSION such as WC_VERSION=5.2.

google-listings-and-ads/bin/install-wp-tests.sh

Line 23 in 56ee88b

WC_VERSION=trunk

google-listings-and-ads/.travis.yml

Lines 32 to 37 in 56ee88b

    
             - WC_LATEST=trunk 
        
           matrix: 
        
             - WP_VERSION="${WP_LATEST}" WC_VERSION="${WC_LATEST}" 
        
             - WP_VERSION="${WP_LATEST}" WC_VERSION=5.2 
        
             - WP_VERSION=5.7 WC_VERSION=5.2 
        
             - WP_VERSION=5.6 WC_VERSION=5.2

Screenshots:

Detailed test instructions:

Go to the Checks tab for details

Changelog entry

tomalec · 2021-10-06T09:42:15Z

WDYT of using the node version from .nvmrc? So we will not hardcode it in so many places, and have to manually change on every update?
actions/setup-node#32

.github/workflows/bundle-size.yml

eason9487 · 2021-10-06T12:37:39Z

WDYT of using the node version from .nvmrc? So we will not hardcode it in so many places, and have to manually change on every update?
actions/setup-node#32

Currently, there should be only one place that uses a hardcode node version:

google-listings-and-ads/.github/workflows/bundle-size.yml

Lines 20 to 24 in 56ee88b

    
           - name: Set up Node 
        
             uses: actions/setup-node@v2 
        
             with: 
        
               node-version: "12.21.0" 
        
               cache: "npm"

The reason is that I recalled several discussions about the node version used for building a bundle, and from slack I found that node 12.21.0 was mentioned a few times, so I specified it in the bundle-size action:

p1622120832186300-slack-C01E9LB4X61
p1622194602214700-slack-C01E9LB4X61

If that's not an issue anymore, then we could remove the actions/setup-node step in the bundle-size action, because in the runs-on: ubuntu-latest environment, the built-in node is already the LTS version.

tomalec · 2021-10-06T13:52:16Z

I'd rather make sure it's set to the exact same version as https://github.com/woocommerce/google-listings-and-ads/blob/develop/.nvmrc, which is the version that was used on Travis before, but more importantly, that's the version we use locally to build (if we use NVM). Which has the benefit of keeping all the parties at the same version, and a single point to set it up.

If the issue with building with lts would pop up, we should rather fix the issue, not downgrade, not to makes lag too far behind.

tomalec · 2021-10-06T13:57:18Z

Since the WC_VERSION in install-wp-tests.sh doesn't take from a command param, the WC_VERSION of matrix values in .travis.yml have never been used when running PHP unit tests on Travis CI. Therefore, I didn't migrate the matrix of specified WC_VERSION such as WC_VERSION=5.2.

I recently learned (naturally from reading the code, not from the docs 😬), that WC_VERSION is the only way to set the WC version for E2E test, so I think we would need them once we will run E2E against a range of versions. (For example to mitigate Dependency Extraction Plugin related bugs)

Edit: but adding E2E tests to CI could be a separate PR.

tomalec · 2021-10-06T14:08:42Z

.github/workflows/js-unit-tests.yml

+      - name: Get npm cache directory
+        uses: actions/cache@v2
+        with:
+          path: ~/.npm
+          key: ${{ runner.os }}-node-${{ hashFiles('package-lock.json') }}
+          restore-keys: |
+            ${{ runner.os }}-node-
+


💅 I wonder If we could somehow DRY it, as this bit repeats in many other workflows.

Doing a search about this led me to this issue as well as Composite Actions. I didn't delve deeply enough to see how that might play out with our actions.

Adjusted by 148dd5f, b86b5fe, and e7467f7. And it needs some workarounds due to lack of if syntax in composite actions. (ref: actions/runner#834)

tomalec · 2021-10-06T14:10:37Z

composer.json

@@ -23,7 +23,8 @@
 		"dealerdirect/phpcodesniffer-composer-installer": "^v0.7",
 		"phpunit/phpunit": "^7.5",
 		"wp-cli/i18n-command": "^2.2",
-		"wp-coding-standards/wpcs": "^2.3"
+		"wp-coding-standards/wpcs": "^2.3",
+		"yoast/phpunit-polyfills": "^1.0"


❓
Is this a dependency needed for GH Actions, or something that was missing before?

That's the problem stuck me for the longest before. I got error when running composer test-unit on my local and GHA. Not sure why it can run successfully on Travis CI.

> ./vendor/bin/phpunit Error: The PHPUnit Polyfills library is a requirement for running the WP test suite. If you are trying to run plugin/theme integration tests, make sure the PHPUnit Polyfills library (https://github.com/Yoast/PHPUnit-Polyfills) is available and either load the autoload file of this library in your own test bootstrap before calling the WP Core test bootstrap file; or set the absolute path to the PHPUnit Polyfills library in a "WP_TESTS_PHPUNIT_POLYFILLS_PATH" constant to allow the WP Core bootstrap to load the Polyfills. If you are trying to run the WP Core tests, make sure to set the "WP_RUN_CORE_TESTS" constant to 1 and run `composer install` before running the tests. Once the dependencies are installed, you can run the tests using the Composer-installed version of PHPUnit or using a PHPUnit phar file, but the dependencies do need to be installed whichever way the tests are run. Script ./vendor/bin/phpunit handling the test-unit event returned with error code 1

Finally, I ended up this problem by adding the yoast/phpunit-polyfills dependency.

eason9487 · 2021-10-07T12:03:27Z

I'd rather make sure it's set to the exact same version as https://github.com/woocommerce/google-listings-and-ads/blob/develop/.nvmrc, which is the version that was used on Travis before, but more importantly, that's the version we use locally to build (if we use NVM). Which has the benefit of keeping all the parties at the same version, and a single point to set it up.

If the issue with building with lts would pop up, we should rather fix the issue, not downgrade, not to makes lag too far behind.

Tested build bundle with LTS version and it works well. I removed the specified node version 12.21.0.

And since the built-in node version of runs-on: ubuntu-latest is the same as the lts/* version (in .nvmrc), I think we could just use it without installing the same version via shivammathur/setup-php@v2 again. If still have concern about it, we can easily add it later through the prepare-node action then.

eason9487 · 2021-10-07T12:07:42Z

I forgot to update the status badges within README before. Added by 9137422.

tomalec

LGTM :)

eason9487 added 6 commits October 6, 2021 15:06

Add the GitHub action to check PHP coding standards

66c4a14

Add the GitHub action to lint JavaScript and CSS

1f67fd1

Add yoast/phpunit-polyfills to require-dev in the composer.json

33e29bf

Add the GitHub action to run PHP unit tests

952e4c1

Add the GitHub action to run JavaScript unit tests

c411065

Add the GitHub action to check bundle size

56ee88b

eason9487 self-assigned this Oct 6, 2021

eason9487 mentioned this pull request Oct 6, 2021

Run Travis-CI on all branches, #922

Closed

eason9487 changed the title ~~Migrate Travis CI tasks to GitHub ActionsAdd/GitHub actions~~ Migrate Travis CI tasks to GitHub ActionsAdd Oct 6, 2021

eason9487 changed the title ~~Migrate Travis CI tasks to GitHub ActionsAdd~~ Migrate Travis CI tasks to GitHub Actions Oct 6, 2021

eason9487 requested a review from a team October 6, 2021 09:46

eason9487 marked this pull request as ready for review October 6, 2021 09:46

tomalec reviewed Oct 6, 2021

View reviewed changes

.github/workflows/bundle-size.yml Show resolved Hide resolved

tomalec reviewed Oct 6, 2021

View reviewed changes

eason9487 added 6 commits October 7, 2021 12:52

Run bundlewatch on push event with branches of trunk and develop

3a25855

Add status badges to README

9137422

Use composite action to set up node

148dd5f

Use composite action to set up PHP

b86b5fe

Apply composite actions to the bundle size check

e7467f7

Fix incorrect CI_COMMIT_SHA for pull request event

c467df5

eason9487 requested review from tomalec and JPry October 7, 2021 12:03

tomalec approved these changes Oct 7, 2021

View reviewed changes

Add missing filed to composite actions

13873b1

eason9487 merged commit df4fbb3 into develop Oct 8, 2021

eason9487 deleted the add/github-actions branch October 8, 2021 02:59

jconroy mentioned this pull request Oct 19, 2021

Remove travis config #1056

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Travis CI tasks to GitHub Actions #1040

Migrate Travis CI tasks to GitHub Actions #1040

eason9487 commented Oct 6, 2021 •

edited

Loading

tomalec commented Oct 6, 2021

eason9487 commented Oct 6, 2021

tomalec commented Oct 6, 2021

tomalec commented Oct 6, 2021 •

edited

Loading

tomalec Oct 6, 2021

JPry Oct 6, 2021

eason9487 Oct 7, 2021

tomalec Oct 6, 2021

eason9487 Oct 7, 2021

eason9487 commented Oct 7, 2021

eason9487 commented Oct 7, 2021

tomalec left a comment

	- WC_LATEST=trunk
	matrix:
	- WP_VERSION="${WP_LATEST}" WC_VERSION="${WC_LATEST}"
	- WP_VERSION="${WP_LATEST}" WC_VERSION=5.2
	- WP_VERSION=5.7 WC_VERSION=5.2
	- WP_VERSION=5.6 WC_VERSION=5.2

Migrate Travis CI tasks to GitHub Actions #1040

Migrate Travis CI tasks to GitHub Actions #1040

Conversation

eason9487 commented Oct 6, 2021 • edited Loading

Changes proposed in this Pull Request:

Additional notes

Screenshots:

Detailed test instructions:

Changelog entry

tomalec commented Oct 6, 2021

eason9487 commented Oct 6, 2021

tomalec commented Oct 6, 2021

tomalec commented Oct 6, 2021 • edited Loading

tomalec Oct 6, 2021

Choose a reason for hiding this comment

JPry Oct 6, 2021

Choose a reason for hiding this comment

eason9487 Oct 7, 2021

Choose a reason for hiding this comment

tomalec Oct 6, 2021

Choose a reason for hiding this comment

eason9487 Oct 7, 2021

Choose a reason for hiding this comment

eason9487 commented Oct 7, 2021

eason9487 commented Oct 7, 2021

tomalec left a comment

Choose a reason for hiding this comment

eason9487 commented Oct 6, 2021 •

edited

Loading

tomalec commented Oct 6, 2021 •

edited

Loading