Skip to content

Commit 0d88dd5

Browse files
authoredMar 1, 2022
tests(smoke): add _excludes and _runner (#13707)
1 parent 5c558fb commit 0d88dd5

File tree

5 files changed

+82
-30
lines changed

5 files changed

+82
-30
lines changed
 

‎lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js

+1
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ async function begin() {
186186
console.log('\n✨ Be sure to have recently run this: yarn build-all');
187187
}
188188
const {runLighthouse} = await import(runnerPath);
189+
runLighthouse.runnerName = argv.runner;
189190

190191
// Find test definition file and filter by requestedTestIds.
191192
let testDefnPath = argv.testsPath || coreTestDefnsPath;

‎lighthouse-cli/test/smokehouse/readme.md

+14
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,18 @@ However, if an array literal is used as the expectation, an extra condition is e
6666

6767
Arrays can be checked against a subset of elements using the special `_includes` property. The value of `_includes` _must_ be an array. Each assertion in `_includes` will remove the matching item from consideration for the rest.
6868

69+
Arrays can be asserted to not match any elements using the special `_excludes` property. The value of `_excludes` _must_ be an array. If an `_includes` check is defined before an `_excludes` check, only the element not matched under the previous will be considered.
70+
6971
**Examples**:
7072
| Actual | Expected | Result |
7173
| -- | -- | -- |
7274
| `[{url: 'http://badssl.com'}, {url: 'http://example.com'}]` | `{1: {url: 'http://example.com'}}` | ✅ PASS |
7375
| `[{timeInMs: 5}, {timeInMs: 15}]` | `{length: 2}` | ✅ PASS |
7476
| `[{timeInMs: 5}, {timeInMs: 15}]` | `{_includes: [{timeInMs: 5}]}` | ✅ PASS |
7577
| `[{timeInMs: 5}, {timeInMs: 15}]` | `{_includes: [{timeInMs: 5}, {timeInMs: 5}]}` | ❌ FAIL |
78+
| `[{timeInMs: 5}, {timeInMs: 15}]` | `{_includes: [{timeInMs: 5}], _excludes: [{timeInMs: 5}]}` | ✅ PASS |
79+
| `[{timeInMs: 5}, {timeInMs: 15}]` | `{_includes: [{timeInMs: 5}], _excludes: [{timeInMs: 15}]}` | ❌ FAIL |
80+
| `[{timeInMs: 5}, {timeInMs: 15}]` | `{_includes: [{timeInMs: 5}], _excludes: [{}]}` | ❌ FAIL |
7681
| `[{timeInMs: 5}, {timeInMs: 15}]` | `[{timeInMs: 5}]` | ❌ FAIL |
7782

7883
### Special environment checks
@@ -104,6 +109,15 @@ If an expectation requires a minimum version of Chromium, use `_minChromiumMiles
104109
},
105110
```
106111
112+
All pruning checks:
113+
114+
- `_minChromiumMilestone`
115+
- `_maxChromiumMilestone`
116+
- `_legacyOnly`
117+
- `_fraggleRockOnly`
118+
- `_skipInBundled`
119+
- `_runner` (set to same value provided to CLI --runner flag, ex: `'devtools'`)
120+
107121
## Pipeline
108122
109123
The different frontends launch smokehouse with a set of tests to run. Smokehouse then coordinates the tests using a particular method of running Lighthouse (CLI, as a bundle, etc).

‎lighthouse-cli/test/smokehouse/report-assert.js

+61-27
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ function findDifference(path, actual, expected) {
103103
};
104104
}
105105

106+
let inclExclCopy;
107+
106108
// We only care that all expected's own properties are on actual (and not the other way around).
107109
// Note an expected `undefined` can match an actual that is either `undefined` or not defined.
108110
for (const key of Object.keys(expected)) {
@@ -112,6 +114,8 @@ function findDifference(path, actual, expected) {
112114
const expectedValue = expected[key];
113115

114116
if (key === '_includes') {
117+
inclExclCopy = [...actual];
118+
115119
if (!Array.isArray(expectedValue)) throw new Error('Array subset must be array');
116120
if (!Array.isArray(actual)) {
117121
return {
@@ -121,12 +125,12 @@ function findDifference(path, actual, expected) {
121125
};
122126
}
123127

124-
const actualCopy = [...actual];
125128
for (const expectedEntry of expectedValue) {
126129
const matchingIndex =
127-
actualCopy.findIndex(actualEntry => !findDifference(keyPath, actualEntry, expectedEntry));
130+
inclExclCopy.findIndex(actualEntry =>
131+
!findDifference(keyPath, actualEntry, expectedEntry));
128132
if (matchingIndex !== -1) {
129-
actualCopy.splice(matchingIndex, 1);
133+
inclExclCopy.splice(matchingIndex, 1);
130134
continue;
131135
}
132136

@@ -140,6 +144,33 @@ function findDifference(path, actual, expected) {
140144
continue;
141145
}
142146

147+
if (key === '_excludes') {
148+
// Re-use state from `_includes` check, if there was one.
149+
/** @type {any[]} */
150+
const arrToCheckAgainst = inclExclCopy || actual;
151+
152+
if (!Array.isArray(expectedValue)) throw new Error('Array subset must be array');
153+
if (!Array.isArray(actual)) continue;
154+
155+
const expectedExclusions = expectedValue;
156+
for (const expectedExclusion of expectedExclusions) {
157+
const matchingIndex = arrToCheckAgainst.findIndex(actualEntry =>
158+
!findDifference(keyPath, actualEntry, expectedExclusion));
159+
if (matchingIndex !== -1) {
160+
return {
161+
path,
162+
actual: arrToCheckAgainst[matchingIndex],
163+
expected: {
164+
message: 'Expected to not find matching entry via _excludes',
165+
expectedExclusion,
166+
},
167+
};
168+
}
169+
}
170+
171+
continue;
172+
}
173+
143174
const actualValue = actual[key];
144175
const subDifference = findDifference(keyPath, actualValue, expectedValue);
145176

@@ -187,7 +218,7 @@ function makeComparison(name, actualResult, expectedResult) {
187218
* @param {LocalConsole} localConsole
188219
* @param {LH.Result} lhr
189220
* @param {Smokehouse.ExpectedRunnerResult} expected
190-
* @param {{isBundled?: boolean}=} reportOptions
221+
* @param {{runner?: string, isBundled?: boolean}=} reportOptions
191222
*/
192223
function pruneExpectations(localConsole, lhr, expected, reportOptions) {
193224
const isFraggleRock = lhr.configSettings.channel === 'fraggle-rock-cli';
@@ -217,8 +248,20 @@ function pruneExpectations(localConsole, lhr, expected, reportOptions) {
217248
* @param {*} obj
218249
*/
219250
function pruneRecursively(obj) {
220-
for (const key of Object.keys(obj)) {
221-
const value = obj[key];
251+
/**
252+
* @param {string} key
253+
*/
254+
const remove = (key) => {
255+
if (Array.isArray(obj)) {
256+
obj.splice(Number(key), 1);
257+
} else {
258+
delete obj[key];
259+
}
260+
};
261+
262+
// Because we may be deleting keys, we should iterate the keys backwards
263+
// otherwise arrays with multiple pruning checks will skip elements.
264+
for (const [key, value] of Object.entries(obj).reverse()) {
222265
if (!value || typeof value !== 'object') {
223266
continue;
224267
}
@@ -229,42 +272,32 @@ function pruneExpectations(localConsole, lhr, expected, reportOptions) {
229272
JSON.stringify(value, null, 2),
230273
`Actual Chromium version: ${getChromeVersion()}`,
231274
].join(' '));
232-
if (Array.isArray(obj)) {
233-
obj.splice(Number(key), 1);
234-
} else {
235-
delete obj[key];
236-
}
275+
remove(key);
237276
} else if (value._legacyOnly && isFraggleRock) {
238277
localConsole.log([
239278
`[${key}] marked legacy only but run is Fraggle Rock, pruning expectation:`,
240279
JSON.stringify(value, null, 2),
241280
].join(' '));
242-
if (Array.isArray(obj)) {
243-
obj.splice(Number(key), 1);
244-
} else {
245-
delete obj[key];
246-
}
281+
remove(key);
247282
} else if (value._fraggleRockOnly && !isFraggleRock) {
248283
localConsole.log([
249284
`[${key}] marked Fraggle Rock only but run is legacy, pruning expectation:`,
250285
JSON.stringify(value, null, 2),
251286
`Actual channel: ${lhr.configSettings.channel}`,
252287
].join(' '));
253-
if (Array.isArray(obj)) {
254-
obj.splice(Number(key), 1);
255-
} else {
256-
delete obj[key];
257-
}
288+
remove(key);
258289
} else if (value._skipInBundled && !isBundled) {
259290
localConsole.log([
260291
`[${key}] marked as skip in bundled and runner is bundled, pruning expectation:`,
261292
JSON.stringify(value, null, 2),
262293
].join(' '));
263-
if (Array.isArray(obj)) {
264-
obj.splice(Number(key), 1);
265-
} else {
266-
delete obj[key];
267-
}
294+
remove(key);
295+
} else if (value._runner && reportOptions?.runner !== value._runner) {
296+
localConsole.log([
297+
`[${key}] is only for runner ${value._runner}, pruning expectation:`,
298+
JSON.stringify(value, null, 2),
299+
].join(' '));
300+
remove(key);
268301
} else {
269302
pruneRecursively(value);
270303
}
@@ -275,6 +308,7 @@ function pruneExpectations(localConsole, lhr, expected, reportOptions) {
275308
delete obj._skipInBundled;
276309
delete obj._minChromiumMilestone;
277310
delete obj._maxChromiumMilestone;
311+
delete obj._runner;
278312
}
279313

280314
const cloned = cloneDeep(expected);
@@ -420,7 +454,7 @@ function reportAssertion(localConsole, assertion) {
420454
* summary. Returns count of passed and failed tests.
421455
* @param {{lhr: LH.Result, artifacts: LH.Artifacts, networkRequests?: string[]}} actual
422456
* @param {Smokehouse.ExpectedRunnerResult} expected
423-
* @param {{isDebug?: boolean, isBundled?: boolean}=} reportOptions
457+
* @param {{runner?: string, isDebug?: boolean, isBundled?: boolean}=} reportOptions
424458
* @return {{passed: number, failed: number, log: string}}
425459
*/
426460
function getAssertionReport(actual, expected, reportOptions = {}) {

‎lighthouse-cli/test/smokehouse/smokehouse.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ async function runSmokehouse(smokeTestDefns, smokehouseOptions) {
5757
useFraggleRock,
5858
jobs = DEFAULT_CONCURRENT_RUNS,
5959
retries = DEFAULT_RETRIES,
60-
lighthouseRunner = cliLighthouseRunner,
60+
lighthouseRunner = Object.assign(cliLighthouseRunner, {runnerName: 'cli'}),
6161
takeNetworkRequestUrls,
6262
} = smokehouseOptions;
6363
assertPositiveInteger('jobs', jobs);
@@ -159,7 +159,10 @@ async function runSmokeTest(smokeTestDefn, testOptions) {
159159
}
160160

161161
// Assert result.
162-
report = getAssertionReport(result, expectations, {isDebug});
162+
report = getAssertionReport(result, expectations, {
163+
runner: lighthouseRunner.runnerName,
164+
isDebug,
165+
});
163166

164167
runs.push({
165168
...result,

‎types/smokehouse.d.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ declare global {
5252
{expectations: Smokehouse.ExpectedRunnerResult | Array<Smokehouse.ExpectedRunnerResult>}
5353

5454
export type LighthouseRunner =
55-
(url: string, configJson?: Config.Json, runnerOptions?: {isDebug?: boolean; useFraggleRock?: boolean}) => Promise<{lhr: LHResult, artifacts: Artifacts, log: string}>;
55+
{runnerName?: string} & ((url: string, configJson?: Config.Json, runnerOptions?: {isDebug?: boolean; useFraggleRock?: boolean}) => Promise<{lhr: LHResult, artifacts: Artifacts, log: string}>);
5656

5757
export interface SmokehouseOptions {
5858
/** If true, performs extra logging from the test runs. */

0 commit comments

Comments
 (0)
Please sign in to comment.