tests(smoke): convert to single LH run per test #12818
Conversation
| */ | ||
| function updateTestDefnFormat(allTestDefns) { | ||
| const expandedTestDefns = allTestDefns.map(testDefn => { | ||
| if (!Array.isArray(testDefn.expectations)) { |
There was a problem hiding this comment.
nit: flip case to use positive condition
| // Include all tests that *include* requested id. | ||
| // e.g. a requested 'pwa' will match 'pwa-airhorner', 'pwa-caltrain', etc | ||
| const isRequested = requestedIds.some(requestedId => test.id.includes(requestedId)); | ||
| return invertMatch !== isRequested; |
There was a problem hiding this comment.
Since we're not trying to fit this on one line anymore, it could be more explicit.
| return invertMatch !== isRequested; | |
| if (invertMatch) isRequested = !isRequested; | |
| return isRequested; |
There was a problem hiding this comment.
Since we're not trying to fit this on one line anymore, it could be more explicit.
I like it
| if (result) bufferedConsole.write(result.log); | ||
|
|
||
| // If there's not an assertion report, not much detail to share but a failure. | ||
| if (report) bufferedConsole.write(report.log); |
There was a problem hiding this comment.
| if (report) bufferedConsole.write(report.log); |
There was a problem hiding this comment.
not sure why this suggestion? report and result too similar looking maybe?
(I was actually thinking of renaming them because they are a little hard to differentiate by name, but didn't want to add to the churn. If this is n=2 confusing, happy to rename now :)
edit: or is it just the comment should be lower/changed? This code was structured slightly differently before with the explicit failure block, but I retained the comment
There was a problem hiding this comment.
Wow I can't read haha. I thought L162 was a duplicate of L159.
part 1/2 of #11950. Wanted to get in these simplifications before FR started appearing in the smoke tests :)
As discussed in #11950, this flattens the smokehouse testDefn format so there's only a single
expectationsobject per smoke test (instead of an array ofexpectationsobjects). When both PRs have landed, this means e.g.yarn smoke dbwwill only do a single lighthouse run and assert its results, simplifying the process of re-running a single test, improving the test expectation files, and simplifying smokehouse itself quite a bit without all the nesting and logging only when a whole test id is complete, etc.This PR is only the changes to smokehouse itself, not the expectation files. We need to provide backwards compatibility for the old format because e.g. pubAds uses our smokehouse test format, so that makes for an easy split point in the change (if the backwards compatibility works for our test files, it'll work for them too :). The second PR will be much more bike sheddy, so stay tuned if that's your thing.
This PR:
'offline-0','offline-1','offline-2', etc for IDs of testDefns created from the array ofexpectations)yarn smoke lanternto still select all the lantern tests when wanting to runlantern-xhr,lantern-fetch, etc (and for now makes it so it still works withlantern-0,lantern-1, etc)smokehouse.js. We get to skip the middle step ofrunTestDefns->runTestDefn->runTestnow, and the grouped logging and etc is more straightforward.