tests(smokehouse): add flag for test sharding #13047
Conversation
brendankenny
requested review from
connorjclark
and removed request for
a team
It's being used to count things and we humans start at 1, and starting at zero is only necessary to help computers squeeze performance out of adding offsets to memory locations so... 1 sgtm :) |
| # 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 |
There was a problem hiding this comment.
this is unfortunately really annoying. You can get the length of the entire matrix with ${{ strategy.job-index }}, but that's 4 here, not 2 (since it's chrome-channel × smoke-test-shard).
There are also ways you can do fancy things with | jq '. | length', but you need a reference to the whole array first, so you need it to be the output of another job (can't do it with env because they aren't available inside matrix...), at which point it all becomes much more verbose than just having a 2 here.
So... :/
| * 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. |
There was a problem hiding this comment.
here and elsewhere: can you break up with spaces, hard to read. 1 ≤ shardNumber ≤ shardTotal
brendankenny
force-pushed
the
smokeshard
branch
from
c7ebc25 to
c55b22b
Compare
|
maybe time to divide up the other smoke tests :) |
Adds an easier way to split up smoke tests across multiple runners with a
--shard=n/dflag, which splits the requested tests up intodshards and runs thenth shard.This stacks with the existing test selection and
--invert-matchfunctionality, so e.g.yarn smoke seo --shard=1/2will runseo-passing seo-failing(out of all theseotestsseo-passing seo-failing seo-status-403 seo-tap-targets), and similarlyyarn smoke --invert-match forms --shard=1/4will run the first fourth of all the smoke tests excludingforms.Both FR and the smoke bundle now run almost all of the smoke tests so are taking longer and we may want to split them up, but the
--invert-matchtrick won't work to split them up into multiple runners because they need--invert-matchto disallow certain tests. We may also eventually want to split up the smoke tests into more than two jobs, which--invert-matchalso wouldn't handle.I got the flag naming from
playwright'sshard(similar flags exist elsewhere but this was the most recent inspiration). Also the reason for using 1 as the lowest shard number instead of 0...not sure how other people feel about that :)