Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests(smoke): add _excludes and _runner #13707

Merged
merged 3 commits into from
Mar 1, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ async function begin() {
console.log('\n✨ Be sure to have recently run this: yarn build-all');
}
const {runLighthouse} = await import(runnerPath);
runLighthouse.runnerName = argv.runner;

// Find test definition file and filter by requestedTestIds.
let testDefnPath = argv.testsPath || coreTestDefnsPath;
Expand Down
14 changes: 14 additions & 0 deletions lighthouse-cli/test/smokehouse/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,18 @@ However, if an array literal is used as the expectation, an extra condition is e

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.

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.

**Examples**:
| Actual | Expected | Result |
| -- | -- | -- |
| `[{url: 'http://badssl.com'}, {url: 'http://example.com'}]` | `{1: {url: 'http://example.com'}}` | ✅ PASS |
| `[{timeInMs: 5}, {timeInMs: 15}]` | `{length: 2}` | ✅ PASS |
| `[{timeInMs: 5}, {timeInMs: 15}]` | `{_includes: [{timeInMs: 5}]}` | ✅ PASS |
| `[{timeInMs: 5}, {timeInMs: 15}]` | `{_includes: [{timeInMs: 5}, {timeInMs: 5}]}` | ❌ FAIL |
| `[{timeInMs: 5}, {timeInMs: 15}]` | `{_includes: [{timeInMs: 5}], _excludes: [{timeInMs: 5}]}` | ✅ PASS |
| `[{timeInMs: 5}, {timeInMs: 15}]` | `{_includes: [{timeInMs: 5}], _excludes: [{timeInMs: 15}]}` | ❌ FAIL |
| `[{timeInMs: 5}, {timeInMs: 15}]` | `{_includes: [{timeInMs: 5}], _excludes: [{}]}` | ❌ FAIL |
| `[{timeInMs: 5}, {timeInMs: 15}]` | `[{timeInMs: 5}]` | ❌ FAIL |

### Special environment checks
Expand Down Expand Up @@ -104,6 +109,15 @@ If an expectation requires a minimum version of Chromium, use `_minChromiumMiles
},
```

All pruning checks:

- `_minChromiumMilestone`
- `_maxChromiumMilestone`
- `_legacyOnly`
- `_fraggleRockOnly`
- `_skipInBundled`
- `_runner` (set to same value provided to CLI --runner flag, ex: `'devtools'`)

## Pipeline

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).
Expand Down
88 changes: 61 additions & 27 deletions lighthouse-cli/test/smokehouse/report-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ function findDifference(path, actual, expected) {
};
}

let inclExclCopy;

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

if (key === '_includes') {
inclExclCopy = [...actual];

if (!Array.isArray(expectedValue)) throw new Error('Array subset must be array');
if (!Array.isArray(actual)) {
return {
Expand All @@ -121,12 +125,12 @@ function findDifference(path, actual, expected) {
};
}

const actualCopy = [...actual];
for (const expectedEntry of expectedValue) {
const matchingIndex =
actualCopy.findIndex(actualEntry => !findDifference(keyPath, actualEntry, expectedEntry));
inclExclCopy.findIndex(actualEntry =>
!findDifference(keyPath, actualEntry, expectedEntry));
if (matchingIndex !== -1) {
actualCopy.splice(matchingIndex, 1);
inclExclCopy.splice(matchingIndex, 1);
continue;
}

Expand All @@ -140,6 +144,33 @@ function findDifference(path, actual, expected) {
continue;
}

if (key === '_excludes') {
// Re-use state from `_includes` check, if there was one.
/** @type {any[]} */
const arrToCheckAgainst = inclExclCopy || actual;

if (!Array.isArray(expectedValue)) throw new Error('Array subset must be array');
if (!Array.isArray(actual)) continue;

const expectedExclusions = expectedValue;
for (const expectedExclusion of expectedExclusions) {
const matchingIndex = arrToCheckAgainst.findIndex(actualEntry =>
!findDifference(keyPath, actualEntry, expectedExclusion));
if (matchingIndex !== -1) {
return {
path,
actual: arrToCheckAgainst[matchingIndex],
expected: {
message: 'Expected to not find matching entry via _excludes',
expectedExclusion,
},
};
}
}

continue;
}

const actualValue = actual[key];
const subDifference = findDifference(keyPath, actualValue, expectedValue);

Expand Down Expand Up @@ -187,7 +218,7 @@ function makeComparison(name, actualResult, expectedResult) {
* @param {LocalConsole} localConsole
* @param {LH.Result} lhr
* @param {Smokehouse.ExpectedRunnerResult} expected
* @param {{isBundled?: boolean}=} reportOptions
* @param {{runner?: string, isBundled?: boolean}=} reportOptions
*/
function pruneExpectations(localConsole, lhr, expected, reportOptions) {
const isFraggleRock = lhr.configSettings.channel === 'fraggle-rock-cli';
Expand Down Expand Up @@ -217,8 +248,20 @@ function pruneExpectations(localConsole, lhr, expected, reportOptions) {
* @param {*} obj
*/
function pruneRecursively(obj) {
for (const key of Object.keys(obj)) {
const value = obj[key];
/**
* @param {string} key
*/
const remove = (key) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drive by simplification

if (Array.isArray(obj)) {
obj.splice(Number(key), 1);
} else {
delete obj[key];
}
};

// Because we may be deleting keys, we should iterate the keys backwards
// otherwise arrays with multiple pruning checks will skip elements.
for (const [key, value] of Object.entries(obj).reverse()) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drive by bug fix

if (!value || typeof value !== 'object') {
continue;
}
Expand All @@ -229,42 +272,32 @@ function pruneExpectations(localConsole, lhr, expected, reportOptions) {
JSON.stringify(value, null, 2),
`Actual Chromium version: ${getChromeVersion()}`,
].join(' '));
if (Array.isArray(obj)) {
obj.splice(Number(key), 1);
} else {
delete obj[key];
}
remove(key);
} else if (value._legacyOnly && isFraggleRock) {
localConsole.log([
`[${key}] marked legacy only but run is Fraggle Rock, pruning expectation:`,
JSON.stringify(value, null, 2),
].join(' '));
if (Array.isArray(obj)) {
obj.splice(Number(key), 1);
} else {
delete obj[key];
}
remove(key);
} else if (value._fraggleRockOnly && !isFraggleRock) {
localConsole.log([
`[${key}] marked Fraggle Rock only but run is legacy, pruning expectation:`,
JSON.stringify(value, null, 2),
`Actual channel: ${lhr.configSettings.channel}`,
].join(' '));
if (Array.isArray(obj)) {
obj.splice(Number(key), 1);
} else {
delete obj[key];
}
remove(key);
} else if (value._skipInBundled && !isBundled) {
localConsole.log([
`[${key}] marked as skip in bundled and runner is bundled, pruning expectation:`,
JSON.stringify(value, null, 2),
].join(' '));
if (Array.isArray(obj)) {
obj.splice(Number(key), 1);
} else {
delete obj[key];
}
remove(key);
} else if (value._runner && reportOptions?.runner !== value._runner) {
localConsole.log([
`[${key}] is only for runner ${value._runner}, pruning expectation:`,
JSON.stringify(value, null, 2),
].join(' '));
remove(key);
} else {
pruneRecursively(value);
}
Expand All @@ -275,6 +308,7 @@ function pruneExpectations(localConsole, lhr, expected, reportOptions) {
delete obj._skipInBundled;
delete obj._minChromiumMilestone;
delete obj._maxChromiumMilestone;
delete obj._runner;
}

const cloned = cloneDeep(expected);
Expand Down Expand Up @@ -420,7 +454,7 @@ function reportAssertion(localConsole, assertion) {
* summary. Returns count of passed and failed tests.
* @param {{lhr: LH.Result, artifacts: LH.Artifacts, networkRequests?: string[]}} actual
* @param {Smokehouse.ExpectedRunnerResult} expected
* @param {{isDebug?: boolean, isBundled?: boolean}=} reportOptions
* @param {{runner?: string, isDebug?: boolean, isBundled?: boolean}=} reportOptions
* @return {{passed: number, failed: number, log: string}}
*/
function getAssertionReport(actual, expected, reportOptions = {}) {
Expand Down
7 changes: 5 additions & 2 deletions lighthouse-cli/test/smokehouse/smokehouse.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ async function runSmokehouse(smokeTestDefns, smokehouseOptions) {
useFraggleRock,
jobs = DEFAULT_CONCURRENT_RUNS,
retries = DEFAULT_RETRIES,
lighthouseRunner = cliLighthouseRunner,
lighthouseRunner = Object.assign(cliLighthouseRunner, {runnerName: 'cli'}),
takeNetworkRequestUrls,
} = smokehouseOptions;
assertPositiveInteger('jobs', jobs);
Expand Down Expand Up @@ -159,7 +159,10 @@ async function runSmokeTest(smokeTestDefn, testOptions) {
}

// Assert result.
report = getAssertionReport(result, expectations, {isDebug});
report = getAssertionReport(result, expectations, {
runner: lighthouseRunner.runnerName,
isDebug,
});

runs.push({
...result,
Expand Down
2 changes: 1 addition & 1 deletion types/smokehouse.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ declare global {
{expectations: Smokehouse.ExpectedRunnerResult | Array<Smokehouse.ExpectedRunnerResult>}

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

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