Skip to content

Commit

Permalink
tests(smokehouse): add flag for test sharding (#13047)
Browse files Browse the repository at this point in the history
  • Loading branch information
brendankenny authored Sep 14, 2021
1 parent 80f0a92 commit d02747d
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 7 deletions.
13 changes: 7 additions & 6 deletions .github/workflows/smoke.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,16 @@ jobs:
strategy:
matrix:
chrome-channel: ['stable', 'ToT']
smoke-test-invert: [false, true]
smoke-test-shard: [1, 2]
# e.g. if set 1 fails, continue with set 2 anyway
fail-fast: false
runs-on: ubuntu-latest
env:
# The smokehouse tests run by job `test set 1`. `test set 2` will run the rest.
SMOKE_BATCH_1: a11y oopif pwa dbw redirects errors offline
# Job named e.g. "Chrome stable, test set 1".
name: Chrome ${{ matrix.chrome-channel }}, batch ${{ matrix.smoke-test-invert == false && '1' || '2' }}
# The total number of shards. Set dynamically when length of single matrix variable is
# computable. See https://github.community/t/get-length-of-strategy-matrix-or-get-all-matrix-options/18342
SHARD_TOTAL: 2
# Job named e.g. "Chrome stable, batch 1".
name: Chrome ${{ matrix.chrome-channel }}, batch ${{ matrix.smoke-test-shard }}

steps:
- name: git clone
Expand Down Expand Up @@ -52,7 +53,7 @@ jobs:
- run: sudo apt-get install xvfb
- name: Run smoke tests
run: |
xvfb-run --auto-servernum yarn c8 yarn smoke --debug -j=1 --retries=2 --invert-match ${{ matrix.smoke-test-invert }} $SMOKE_BATCH_1
xvfb-run --auto-servernum yarn c8 yarn smoke --debug -j=1 --retries=2 --shard=${{ matrix.smoke-test-shard }}/$SHARD_TOTAL
yarn c8 report --reporter text-lcov > smoke-coverage.lcov
- name: Upload test coverage to Codecov
Expand Down
55 changes: 54 additions & 1 deletion lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

/* eslint-disable no-console */

import {strict as assert} from 'assert';
import path from 'path';
import fs from 'fs';
import url from 'url';
Expand Down Expand Up @@ -79,6 +80,52 @@ function getDefinitionsToRun(allTestDefns, requestedIds, {invertMatch}) {
return smokes;
}

/**
* Parses the cli `shardArg` flag into `shardNumber/shardTotal`. Splits
* `testDefns` into `shardTotal` shards and returns the `shardNumber`th shard.
* Shards will differ in size by at most 1.
* Shard params must be 1 ≤ shardNumber ≤ shardTotal.
* @param {Array<Smokehouse.TestDfn>} testDefns
* @param {string=} shardArg
* @return {Array<Smokehouse.TestDfn>}
*/
function getShardedDefinitions(testDefns, shardArg) {
if (!shardArg) return testDefns;

// eslint-disable-next-line max-len
const errorMessage = `'shard' must be of the form 'n/d' and n and d must be positive integers with 1 ≤ n ≤ d. Got '${shardArg}'`;
const match = /^(?<shardNumber>\d+)\/(?<shardTotal>\d+)$/.exec(shardArg);
assert(match && match.groups, errorMessage);
const shardNumber = Number(match.groups.shardNumber);
const shardTotal = Number(match.groups.shardTotal);
assert(shardNumber > 0 && Number.isInteger(shardNumber), errorMessage);
assert(shardTotal > 0 && Number.isInteger(shardTotal));
assert(shardNumber <= shardTotal, errorMessage);

// Array is sharded with `Math.ceil(length / shardTotal)` shards first
// and then the remaining `Math.floor(length / shardTotal) shards.
// e.g. `[0, 1, 2, 3]` split into 3 shards is `[[0, 1], [2], [3]]`.
const baseSize = Math.floor(testDefns.length / shardTotal);
const biggerSize = baseSize + 1;
const biggerShardCount = testDefns.length % shardTotal;

// Since we don't have tests for this file, construct all shards so correct
// structure can be asserted.
const shards = [];
let index = 0;
for (let i = 0; i < shardTotal; i++) {
const shardSize = i < biggerShardCount ? biggerSize : baseSize;
shards.push(testDefns.slice(index, index + shardSize));
index += shardSize;
}
assert.equal(shards.length, shardTotal);
assert.deepEqual(shards.flat(), testDefns);

const shardDefns = shards[shardNumber - 1];
console.log(`In this shard (${shardArg}), running: ${shardDefns.map(d => d.id).join(' ')}\n`);
return shardDefns;
}

/**
* Prune the `networkRequests` from the test expectations when `takeNetworkRequestUrls`
* is not defined. Custom servers may not have this method available in-process.
Expand Down Expand Up @@ -163,6 +210,11 @@ async function begin() {
default: false,
describe: 'Run all available tests except the ones provided',
},
'shard': {
type: 'string',
// eslint-disable-next-line max-len
describe: 'A argument of the form "n/d", which divides the selected tests into d groups and runs the nth group. n and d must be positive integers with 1 ≤ n ≤ d.',
},
})
.wrap(y.terminalWidth())
.argv;
Expand All @@ -187,7 +239,8 @@ async function begin() {
const {default: rawTestDefns} = await import(url.pathToFileURL(testDefnPath).href);
const allTestDefns = updateTestDefnFormat(rawTestDefns);
const invertMatch = argv.invertMatch;
const testDefns = getDefinitionsToRun(allTestDefns, requestedTestIds, {invertMatch});
const requestedTestDefns = getDefinitionsToRun(allTestDefns, requestedTestIds, {invertMatch});
const testDefns = getShardedDefinitions(requestedTestDefns, argv.shard);

let smokehouseResult;
let server;
Expand Down

0 comments on commit d02747d

Please sign in to comment.