Improve Travis CI build times by noisysocks · Pull Request #13103 · WordPress/gutenberg

noisysocks · 2018-12-27T00:49:28Z

Some experimenting with optimising our Travis CI configuration to make builds run faster.

I've:

Configured Travis CI to cache npm and nvm artefacts (~30 second speed up)
Configured Jest to use caching (~10 second speed up)
Split E2E tests into two groups that run in parallel (~6 minute "speed up")

I also cleaned up the Travis CI configuration file and gave each job a name so that it's clearer in the UI what job does what:

noisysocks · 2018-12-27T06:41:08Z

Follow up tasks:

./setup-local-env.sh runs npm run build:packages twice which is a lot of unnecessary extra work
npm run build:packages is very slow when running in a CI environment (lots of disk read/write!)
- Avoiding synchronous methods in /bin/packages/build.js might help
- Could potentially build packages in parallel
- Could look at having /bin/packages/build.js use caching so that we don't compile source files that haven't changed

youknowriad · 2018-12-27T08:27:33Z

Awesome work here, I'm not sure about caching node_modules as we often do rm -rf node_modules locally when switching branches but I'm ok with testing it.

noisysocks · 2018-12-28T03:06:23Z

Maybe adding a PHP 7.2 Docker job to go along with the existing PHP 5.6 Docker job?

We don't have a 5.6 Docker job. Our PHP jobs are:

Run PHP unit tests using the Docker environment. PHP version will be whatever the wordpress Dockerfile is set to use.
Run PHP unit tests without booting Docker. PHP version will be 5.6 which is installed automatically by Travis CI. Only done on master and not on every PR.
Run PHP unit tests without booting Docker. PHP version will be 5.3 which we install manually using phpbrew. Only done on master and not on every PR.
Run PHP unit tests without booting Docker. PHP version will be 5.2 which we install manually using phpbrew.

To be honest, I'm not sure why we need so many. Does anyone remember the motivation here? Could we have it so that only (1) runs on PRs? My preference would be to have as few jobs as possible so that there's less surface area for things to break. It's super frustrating having to restart builds all the time 🙂

- Cache npm and nvm artifacts - Use jest caching - Run E2E tests in parallel

This feels "more correct" and doesn't significantly affect build performance.

gziolo

We should double check how many Travis jobs can be executed at once. It looks like 7 still run at the same time which is good. I'm wondering how many PRs can be verified at once though. The total time for execution is probably going to be higher than before because all the setup work required for e2e tests even though individual jobs finish much earlier.

gziolo · 2019-01-02T08:57:01Z

.travis.yml

+        - ./bin/setup-local-env.sh
+      script:
+        - $( npm bin )/jest --config test/e2e/jest.config.json --listTests > ~/.jest-e2e-tests
+        - npm run test-e2e -- --ci --runInBand --cacheDirectory="$HOME/.jest-cache" --runTestsByPath $( awk 'NR % 2 == 0' < ~/.jest-e2e-tests )


There is no need to define --runInBand in here. It's already provided behind the scenes:

https://github.com/WordPress/gutenberg/blob/master/packages/scripts/scripts/test-e2e.js#L39-L45

👍 ab1faa1d7

gziolo · 2019-01-02T08:59:37Z

.travis.yml

      script:
-        - npm install || exit 1
-        - npm run ci || exit 1
+        - npm run lint


Just noting that those 3 commands were executed concurrently before. I didn't benchmark it but given that we use all 2 cores with unit tests it might have no impact at all :)

Yes, this is intentional. I checked and it's faster to run these commands sequentially. I also like that it reports lint errors back to the developer really quickly 🙂

Adding fast_finish: true to the build matrix might also be worthwhile

https://docs.travis-ci.com/user/customizing-the-build/#fast-finishing
"If some rows in the build matrix are allowed to fail, the build won’t be marked as finished until they have completed. To mark the build as finished as soon as possible"

It used to be in WP, but was removed due to a Travis CI bug with the feature:

https://core.trac.wordpress.org/changeset/35150

gziolo · 2019-01-02T09:03:39Z

.travis.yml

+    - name: JS unit tests
      env: WP_VERSION=latest
+      before_install:
+        - nvm install --latest-npm


Out of curiosity, why is it needed here? Does it override nvm install from line 26?

Yes defining before_install here overrides the previous before_install. We need --latest-npm for this job so that we're using the latest version of npm. We don't need --latest-npm in other jobs because ./bin/setup-local-env.sh will upgrade npm if necessary.

gziolo · 2019-01-02T09:07:00Z

.travis.yml

+      install:
+        - ./bin/setup-local-env.sh
+      script:
+        - $( npm bin )/jest --config test/e2e/jest.config.json --listTests > ~/.jest-e2e-tests


It looks like this logic to running only the subset of test files is something that could be included in wp-scripts test-e2e by design with a new CLI flag:
https://github.com/WordPress/gutenberg/blob/master/packages/scripts/scripts/test-e2e.js#L45

~~👍 ab1faa1d7~~ see below

We aren't able to use npm run test-e2e here because it uses concurrently which writes a bunch of extra logging to STDOUT. We want only the test path names to be written to STDOUT so that ~/.jest-e2e-tests contains only test path names. This lets us pipe ~/.jest-e2e-tests into the next command as an argument.

Yes, that's correct. Also npm run test-e2e would run pretest-e2e which executes npm run build. In effect, they would run twice.

I was thinking about changing the way the script test-e2e works when a new flag would be provided. It's a nice task for the future though.

ntwb · 2019-01-02T09:25:15Z

@gziolo Via https://make.wordpress.org/core/2017/05/18/increased-concurrent-jobs-on-travis-ci-builds/

https://travis-ci.org/WordPress now has 15 concurrent jobs instead of the default 5 available to all open-source projects.

This covers all projects under https://github.com/WordPress, so it will help with daily development of Gutenberg, build times for our official wordpress-develop build (especially during releases), and ensuring that builds for these two projects don’t interfere with each other.

An example, if the WordPress/wordpress-develop repo is running a Travis CI build that will allocate 13 CI jobs, leaving only 2 for Gutenberg here.

gziolo · 2019-01-02T10:17:48Z

An example, if the WordPress/wordpress-develop repo is running a Travis CI build that will allocate 13 CI jobs, leaving only 2 for Gutenberg here.

@ntwb thanks for the clarification. It looks like 5 vs 7 doesn't make any difference in such case. I would think we should optimize for the time to finish a single run on Travis. That means, I think we should give this PR a try and see how it goes.

@noisysocks do you want to apply any of the suggestions I listed before we proceed?

noisysocks · 2019-01-02T23:17:03Z

Thanks @gziolo! I applied or responded to your suggestions 🙂

gziolo

Thanks for your responses. In my opinion, this is good to go. We should iterate further with follow-up PRs.

noisysocks added the [Status] In Progress Tracking issues with work in progress label Dec 27, 2018

noisysocks force-pushed the try/optimize-ci branch 20 times, most recently from 3ae2a83 to 45eda12 Compare December 27, 2018 06:30

noisysocks changed the title ~~[WIP] Try to improve CI times~~ Try to improve CI times Dec 27, 2018

noisysocks changed the title ~~Try to improve CI times~~ Improve Travis CI build times Dec 27, 2018

noisysocks added [Type] Build Tooling Issues or PRs related to build tooling and removed [Status] In Progress Tracking issues with work in progress labels Dec 27, 2018

noisysocks requested a review from gziolo December 27, 2018 06:33

noisysocks requested review from a team and talldan December 27, 2018 06:41

noisysocks mentioned this pull request Dec 28, 2018

[WIP] Try caching compiled JavaScript when building packages #13124

Closed

noisysocks added 6 commits December 28, 2018 16:54

Optimize Travis CI integration

0b2f880

- Cache npm and nvm artifacts - Use jest caching - Run E2E tests in parallel

Use npm ci instead of npm install

976e8c2

This feels "more correct" and doesn't significantly affect build performance.

Remove unnecessary npm scripts

c37ba28

Configure E2E tests to use caching and to not update snapshots

95f9981

Tell jest to not bother with regular expressions

1bb8a1c

Oops, how'd this get here?

94ffdce

noisysocks force-pushed the try/optimize-ci branch from db2ddee to 94ffdce Compare December 28, 2018 05:54

gziolo reviewed Jan 2, 2019

View reviewed changes

Don't provide unnecessary --runInBand argument

af4100d

noisysocks force-pushed the try/optimize-ci branch from ab1faa1 to af4100d Compare January 3, 2019 00:09

gziolo added this to the 5.0 (Gutenberg) milestone Jan 3, 2019

gziolo approved these changes Jan 3, 2019

View reviewed changes

gziolo modified the milestones: 5.0 (Gutenberg), 4.9 Jan 3, 2019

youknowriad merged commit 74c3d39 into master Jan 4, 2019

youknowriad deleted the try/optimize-ci branch January 4, 2019 09:30

noisysocks mentioned this pull request Jan 24, 2019

Fix Travis CI not detecting uncommitted changes to package-lock.json #13462

Closed

youknowriad pushed a commit that referenced this pull request Mar 6, 2019

Improve Travis CI build times (#13103)

fc72ea5

youknowriad pushed a commit that referenced this pull request Mar 6, 2019

Improve Travis CI build times (#13103)

22563c3

yassine4999 mentioned this pull request Nov 30, 2023

[Snyk] Fix for 1 vulnerabilities yassine4999/gutenberg#29

Open

captainbarber99 mentioned this pull request Nov 30, 2023

[Snyk] Fix for 1 vulnerabilities captainbarber99/gutenberg#23

Open

naiba4 mentioned this pull request Dec 1, 2023

[Snyk] Fix for 1 vulnerabilities naiba4/gutenberg#26

Open

Verdinjoshua26 mentioned this pull request Dec 1, 2023

[Snyk] Fix for 1 vulnerabilities Verdinjoshua26/gutenberg#101

Open

jay-Anonymous mentioned this pull request May 14, 2024

[Snyk] Security upgrade jest from 27.5.1 to 29.0.0 jay-Anonymous/gutenberg#11

Open

yassine4999 mentioned this pull request May 14, 2024

[Snyk] Fix for 2 vulnerabilities yassine4999/gutenberg#47

Open

Conversation

noisysocks commented Dec 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

noisysocks commented Dec 27, 2018

Uh oh!

youknowriad commented Dec 27, 2018

Uh oh!

noisysocks commented Dec 28, 2018

Uh oh!

gziolo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

noisysocks Jan 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

noisysocks Jan 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

noisysocks Jan 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ntwb commented Jan 2, 2019

Uh oh!

gziolo commented Jan 2, 2019

Uh oh!

noisysocks commented Jan 2, 2019

Uh oh!

gziolo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

noisysocks commented Dec 27, 2018 •

edited

Loading

noisysocks Jan 2, 2019 •

edited

Loading

noisysocks Jan 2, 2019 •

edited

Loading

noisysocks Jan 3, 2019 •

edited

Loading