tests: run smoke tests in parallel jobs #10993
Conversation
| @@ -38,19 +34,6 @@ jobs: | |||
| python -m pip install --upgrade pip | |||
| pip install protobuf==3.7.1 | |||
|
|
|||
| # Cache yarn deps. thx https://github.com/actions/cache/blob/master/examples.md#node---yarn | |||
There was a problem hiding this comment.
looking at the runs for https://github.com/GoogleChrome/lighthouse/pull/10988/checks?check_run_id=783384854 and running a bunch of them myself, it seems like caching only saves like 5 seconds out of ~30. Doesn't seem worth it.
|
|
||
| - run: yarn test-lantern | ||
| - run: yarn test-legacy-javascript | ||
| - run: yarn i18n:checks | ||
| - run: yarn dogfood-lhci | ||
|
|
||
| - name: Upload dist/ |
| # e.g. if smoke 0 failes, continue with smoke 1 anyway | ||
| fail-fast: false | ||
| env: | ||
| # The smokehouse tests run by job smoke 0. smoke-1 will run the rest. |
There was a problem hiding this comment.
whatcha thinking with ToTChrome in the mix too?
There was a problem hiding this comment.
whatcha thinking with ToTChrome in the mix too?
matrix:
smokeTestInvert: [false, true]
chrome: [Stable, ToT]
name: smoke ${{ strategy.job-index }} (Chrome ${{ matrix.chrome }})and then conditionally set CHROME_PATH? conditionals are awkward but that doesn't seem too bad
There was a problem hiding this comment.
I broke up the ToT and parallel job PRs for a reason. Things are getting complex :)
.github/workflows/ci.yml
Outdated
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| invert: [false, true] |
There was a problem hiding this comment.
out here i'd name this smokeTestInvert or something
| @@ -34,18 +34,19 @@ const runnerPaths = { | |||
| * Determine batches of smoketests to run, based on the `requestedIds`. | |||
| * @param {Array<Smokehouse.TestDfn>} allTestDefns | |||
| * @param {Array<string>} requestedIds | |||
| * @param {{invert: boolean}} options | |||
There was a problem hiding this comment.
grep's -v option is called --invert-match so... maybe that instead.
.github/workflows/ci.yml
Outdated
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| invert: [false, true] |
There was a problem hiding this comment.
also amusing would be
matrix:
smoketest: [a11y, errors, oopif, pwa, pwa2, pwa3, dbw, redirects, seo, offline, byte, perf, lantern, metrics, legacy-javascript, source-maps]
# ...
- name: Run smoke test
run yarn smoke -j=1 --retries=2 ${{ matrix.smoketest }}and launch all the worker jobs :)
But I'm pretty sure we compete with the rest of the org for workers?
| - name: yarn test-bundle | ||
| run: xvfb-run --auto-servernum yarn test-bundle | ||
| - name: yarn test-docs | ||
| run: xvfb-run --auto-servernum yarn test-docs | ||
|
|
||
| - run: yarn test-lantern | ||
| - run: yarn test-legacy-javascript |
There was a problem hiding this comment.
Some of these tests can take a long time. That's why I split up the "special" tests (legacy js, lantern, etc.) from the core unit test and the build job–with this PR all of these are still in the same job.
|
How long does it take? |
looking at https://github.com/GoogleChrome/lighthouse/actions?query=branch%3Aparsmoke (i think the time value is the end-to-end time?) looks like it was 9min and 10-20s.. and after splitting out basics.. it's a minute faster at 8m11s. |
It's end to end, but the parallel jobs don't always start at once. The slowest job was 7m 55s and we can improve that :) |
aye. just confirmed that, too. const data = await fetch('https://api.github.com/repos/GoogleChrome/lighthouse/actions/workflows/1093967/runs?conclusion=success&status=completed&per_page=100').then(r => r.json());
data
.workflow_runs
.filter(e => e.conclusion === "success")
.map(e => ((new Date(e.updated_at) - new Date(e.created_at)) / 1000 / 60).toLocaleString(undefined, {style: 'unit', unit: 'minute', unitDisplay: 'long'}))
|
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| smokeTestInvert: [false, true] |
There was a problem hiding this comment.
yaml keys are normally skewer-case.
🔠 🔢 🏌️ time :)
Counter proposal to #10988