Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Refactor travis-ci to use parallel jobs #2414

Merged
merged 7 commits into from
Jan 9, 2019

Conversation

erikjohnston
Copy link
Member

This should make it easier to see what's actually failed and what hasn't.

Currently this does an npm install for both npm run test and npm run lintwithexclusions, does that need to happen?

@erikjohnston
Copy link
Member Author

The same question applies to whether we need chrome for all the tests. We can also probably improve the end to end test job:

  1. It looks like it does both npm run test and matrix-react-end-to-end-tests when against develop (you can tell travis to only run particular jobs on particular branches)
  2. It looks like it ignores the dependencies in the install steps and compiles its own dependencies. We should split that out into a separate install script. This a) reduces time to run the test and b) means the install step outputs are automatically collapsed when viewing the job output making it easier to see the actual test output.

@erikjohnston erikjohnston requested a review from a team January 8, 2019 11:44
This reverts commit f9ecdcb.

Travis is unhappy about adding an NPM cache as it can't find
matrix-js-sdk for some reason
@erikjohnston
Copy link
Member Author

Oh, sad times. It doesn't like cache: npm for some reason:

$ npm install
npm WARN checkPermissions Missing write access to /home/travis/build/matrix-org/matrix-react-sdk/node_modules/matrix-js-sdk
npm ERR! path /home/travis/build/matrix-org/matrix-react-sdk/node_modules/matrix-js-sdk
npm ERR! code ENOENT
npm ERR! errno -2
npm ERR! syscall access
npm ERR! enoent ENOENT: no such file or directory, access '/home/travis/build/matrix-org/matrix-react-sdk/node_modules/matrix-js-sdk'
npm ERR! enoent This is related to npm not being able to find a file.
npm ERR! enoent 
npm ERR! A complete log of this run can be found in:
npm ERR!     /home/travis/.npm/_logs/2019-01-08T11_53_06_641Z-debug.log
The command "npm install" failed and exited with 254 during .

@bwindels bwindels self-assigned this Jan 9, 2019
@bwindels
Copy link
Contributor

bwindels commented Jan 9, 2019

Currently this does an npm install for both npm run test and npm run lintwithexclusions, does that need to happen?

npm run test needs a full install, I imagine npm run lintwithexclusions could get by with npm install --only=dev. I assume Travis runs these different steps on the same directory, in serial? In that case an subsequent npm install should be rather fast as everything is already installed, so I would not bother.

@bwindels
Copy link
Contributor

bwindels commented Jan 9, 2019

The same question applies to whether we need chrome for all the tests. We can also probably improve the end to end test job:

npm run test needs chrome. The end-to-end tests actually install their own copy of chrome (on each build, a major slow-down), because the one provided by Travis is not recent enough IIRC. Not ideal, but last time I checked I couldn't find a way to get a puppeteer-compatible chrome installed by Travis :(

@bwindels
Copy link
Contributor

bwindels commented Jan 9, 2019

It looks like it ignores the dependencies in the install steps and compiles its own dependencies. We should split that out into a separate install script. This a) reduces time to run the test and b) means the install step outputs are automatically collapsed when viewing the job output making it easier to see the actual test output.

The end-to-end tests need a built version of riot to run against, and the riot build needs all dependencies installed.

The dependencies for the end-to-end tests are installed as part of the install.sh script, previously invoked at .travis-test-riot.sh:38. This would indeed be good to have in a separate install step so it doesn't pollute the test output. Anything stopping us from doing this now?

.travis.yml Show resolved Hide resolved
.travis.yml Outdated
- ./scripts/travis/test-riot.sh
- name: Unit Tests
script:
- npm run test
Copy link
Contributor

Choose a reason for hiding this comment

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

test-riot.sh, run by the end-to-end tests, still contains npm run test as well, so they would be run twice like this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think test-riot.sh is running the riot-web unit tests?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's doing both. It's just not running the end-to-end tests right now because they are disabled for experimental. https://travis-ci.org/matrix-org/matrix-react-sdk/jobs/476805158 shows its also running the unit tests...

I committed some changes to split up the test-riot script for unit and e2e tests.

.travis.yml Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
.travis.yml Show resolved Hide resolved
@erikjohnston
Copy link
Member Author

I think you need to keep the npm install step tbh

@erikjohnston erikjohnston merged commit 0b4ce3c into experimental Jan 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants