Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move from Travis CI to GitHub Actions #635

Merged
merged 4 commits into from May 4, 2021
Merged

Conversation

htdat
Copy link
Member

@htdat htdat commented Apr 22, 2021

Fix #634

Summary

As mentioned in the title, the main goal of this PR is to move all tests/checks from Travis CI (aka .travis.yml file) to GitHub Actions.

I will not add new things (tests, checks, deploy actions) in this PR.

What we're running with Travis-CI

From here

Edit-Flow/.travis.yml

Lines 94 to 102 in 2e07a1b

script:
- npm run test-unit-php
- |
if [[ ${RUN_EXTRA} == "1" ]]; then
npm run test-e2e
bash bin/phpcs-diff.sh
npm run lint
npm run test-jest
fi

Travis-CI is running these tests/checks:

  1. PHP Unit tests
  2. E2E tests
  3. PHPCS Diff
  4. Lint (JS)
  5. Jest (JS Unit)

(1) is run for all matrix cases.
(2) to (5) are run for selected matrix cases, PHP 7.4 + WP 5.4 or latest (Note: the last commit to travis.yml was one year ago).

What this PR does

After many trial-and-error attempts, I decided to separate two tests into workflows in GitHub actions:

  • E2E and JS tests: (2), (4) and (5) above. Basically, these tests require npm.
  • PHP tests: (1) and (3) above - these tests require composer.

E2E and JS tests

Basically, I am following up this set up https://github.com/WordPress/gutenberg/blob/trunk/.github/workflows/end2end-test.yml
But with some differences:

PHP tests

The way I am setting up this file is similar to the WP Parsely plugin https://github.com/Parsely/wp-parsely/blob/d7a06bd696a7af3e31e49ee98f893ff8a1d07755/.github/workflows/integration-tests.yml

I don't follow the Dockerize way like E2E and JS tests mainly due to a similar reason as mentioned here woocommerce/woocommerce-blocks#3691 (comment)

Another note here is that I am using matrix.wp value to run tests for two latest WP versions 5.6 and 5.7.

Steps to Test

Well, I am not sure if there is any way to tests the test runs except looking at workflow files and action results:

Comment on lines +18 to +20
"dealerdirect/phpcodesniffer-composer-installer": "^0.4.1 || ^0.5 || ^0.6.2 || ^0.7",
"exussum12/coverage-checker": "^0.11.2",
"phpunit/phpunit": "^4 || ^5 || ^6 || ^7"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change and removing composer.json help handle different PHPUnit and PHP versions.

@mikeyarce
Copy link
Member

This is looking great, @htdat !

For the PHP tests, can we allow failures for PHP > 8.0 for now? Maybe something like continue-on-error: true?

What do you think about splitting this into two PRs - one for the PHP tests and one for the E2E tests? I feel like we'll be able to merge the PHP ones pretty soon but the E2E ones might take a bit longer.

@htdat
Copy link
Member Author

htdat commented Apr 23, 2021

@mikeyarce thanks for your review :D

For the PHP tests, can we allow failures for PHP > 8.0 for now? Maybe something like continue-on-error: true?

👍 My recent commit has addressed this b45c8f5

What do you think about splitting this into two PRs - one for the PHP tests and one for the E2E tests? I feel like we'll be able to merge the PHP ones pretty soon but the E2E ones might take a bit longer.

Yeah, I can for sure. However, IMHO, we might still want to keep those two tests in this PR.
The main purpose of this PR is to move away from Travis CI and ensure that all tests in Travis configuration can run in GitHub Actions with minimal changes.

In case, we need to upgrade composer and/or npm packages, add coverage, etc, we can do that in the next PRs. I've created a few issues for these: #636, #637, #638.

What do you think?

@pkevan
Copy link

pkevan commented Apr 23, 2021

I don't see this mentioned but what is the reason for moving? Will there be anything to consider when running these tests with Github actions?

Copy link
Member

@nielslange nielslange left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good overall. I just noticed two spots for which I left a comment.

NIT: I also noticed that e2e-and-js-tests.yml, php-tests.yml and composer.json are missing an empty line by the end of the file.

.github/workflows/php-tests.yml Show resolved Hide resolved
.github/workflows/php-tests.yml Outdated Show resolved Hide resolved
@htdat
Copy link
Member Author

htdat commented Apr 26, 2021

@pkevan - thanks for your great questions.

I don't see this mentioned but what is the reason for moving?

The main reason is that Travis-CI.org will be shut down soon.
And recently Travis CI tests have failed without any clear reason as I mentioned here #630 But maybe something may change and I could not figure it out.

Then @mikeyarce suggested moving to GitHub Actions.

When working and researching more with GitHub Actions, I notice that WordPress core and Gutenberg projects moved from Travis-CI.org to GitHub Actions too. With the fact that our current Travis-CI setup here based on Gutenberg setup (#562), I think this makes sense for us to do the same.

Will there be anything to consider when running these tests with Github actions?

I have not seen anything like that so far. Instead, it takes time to understand the differences between GitHub Actions and Travis, and follow the new way of Gutenberg in setting up testing environments (I mentioned details in E2E and JS tests section of this PR description).

@htdat
Copy link
Member Author

htdat commented Apr 26, 2021

NIT: I also noticed that e2e-and-js-tests.yml, php-tests.yml and composer.json are missing an empty line by the end of the file.

@nielslange - missing an empty line by the end of the file can cause any specific issue for .yml files? I know that might cause issues for other file types in some cases but I have done a bit of research and can not find the "why" for YAML files.
I do not mean we need to follow WP core closely but even its GitHub Actions config files like this also does not have any empty line by the end of the file.

@htdat
Copy link
Member Author

htdat commented Apr 26, 2021

I've added this line into PHP tests section in the PR description above.

Update on 2021.04.26: Another reason is that wp-env has not supported multisite yet WordPress/gutenberg#27961

Copy link
Member

@mikeyarce mikeyarce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mikeyarce mikeyarce merged commit fbb9cbf into master May 4, 2021
@htdat htdat deleted the travis-to-gh-actions branch May 6, 2021 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrating Travis CI to GitHub Actions
4 participants