tests: fix split of smoke tests across jobs #12323
Conversation
| run: xvfb-run --auto-servernum yarn unit:cicoverage | ||
| run: | | ||
| xvfb-run --auto-servernum yarn unit:cicoverage | ||
| yarn c8 report --reporter text-lcov > unit-coverage.lcov |
There was a problem hiding this comment.
unit coverage didn't matter for this since we don't pass extra flags to it, but it made sense to do both in the same way.
There was a problem hiding this comment.
much better than doing in package.json!
|
smoke stable/ToT back down to 5-8 minutes instead of e.g. 12-16 minutes in the last landed PR (https://github.com/GoogleChrome/lighthouse/runs/2234611603) |
package.json
Outdated
| "coverage": "yarn unit:cicoverage && c8 report --reporter html", | ||
| "smoke:cicoverage": "yarn c8 yarn smoke -j=1 --retries=2 && yarn c8 report --reporter text-lcov > smoke-coverage.lcov", | ||
| "smoke:cicoverage": "yarn c8 yarn smoke -j=1 --retries=2", |
There was a problem hiding this comment.
should we just remove the -j and --retries flags here? better defined in the yml anyhow
There was a problem hiding this comment.
I thought about that but then didn't do it because what's the point of having a cicoverage npm script just to call yarn c8 yarn smoke since it's not ci-specific and it only really works with the extra flags? Well...there is no point, so let's move those to the yml and delete this :)
|
@adamraine (or @connorjclark) yah? |
| "coverage": "yarn unit:cicoverage && c8 report --reporter html", | ||
| "smoke:cicoverage": "yarn c8 yarn smoke -j=1 --retries=2 && yarn c8 report --reporter text-lcov > smoke-coverage.lcov", | ||
| "coverage:smoke": "yarn smoke:cicoverage && c8 report --reporter html", | ||
| "coverage:smoke": "yarn c8 yarn smoke -j=1 && c8 report --reporter html", |
There was a problem hiding this comment.
nit: Any reason this can't be smoke:cicoverage to match unit:cicoverage?
There was a problem hiding this comment.
nit: Any reason this can't be
smoke:cicoverageto matchunit:cicoverage?
coverage:smoke is to match coverage...it's meant for if you want to run coverage locally, so it generates an html version of the coverage report for human consumption. I assume it's called coverage and not coverage:unit because there was only the one kind of coverage back when it was named 🤷
I noticed today that the CI smoke tests have all been running the full suite of tests instead of splitting them up.
The
--invert-match ${{ matrix.smoke-test-invert }} $SMOKE_GROUP_1flags were being added at the end of theyarn smoke:cicoveragecommand, but when smoke switched to smoke-with-coverage, the command they were being added to was just the report generation one,yarn c8 report --reporter text-lcov > smoke-coverage.lcov, which does nothing with those flags.Rather than making yet another npm script for generating the
text-lcov, since the codecov upload only takes place in the CI script now, it makes sense to only generate the file to upload to codecov there as well.