diff --git a/cli/test/smokehouse/readme.md b/cli/test/smokehouse/readme.md index 3fb5e45fbde6..7170a55b2c86 100644 --- a/cli/test/smokehouse/readme.md +++ b/cli/test/smokehouse/readme.md @@ -64,9 +64,11 @@ Individual elements of an array can be asserted by using numeric properties in a However, if an array literal is used as the expectation, an extra condition is enforced that the actual array _must_ have the same length as the provided expected array. -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 and objects 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. +Arrays and objects 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. + +If an object is checked using `_includes` or `_excludes`, it will be checked against the `Object.entries` array. **Examples**: | Actual | Expected | Result | @@ -79,6 +81,8 @@ Arrays can be asserted to not match any elements using the special `_excludes` p | `[{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 | +| `{'foo': 1}` | `{_includes: [['foo', 1]]}` | ✅ PASS | +| `{'foo': 1, 'bar': 2}` | `{_includes: [['foo', 1]], _excludes: [['bar', 2]]}` | ❌ FAIL | ### Special environment checks diff --git a/cli/test/smokehouse/report-assert-test.js b/cli/test/smokehouse/report-assert-test.js index a0e9a3b2cd12..e84fa541ed67 100644 --- a/cli/test/smokehouse/report-assert-test.js +++ b/cli/test/smokehouse/report-assert-test.js @@ -133,6 +133,14 @@ describe('findDiffersences', () => { expected: {prices: {_includes: [/\d/, /\d/, /\d/, /\d/, /\d/, /\d/]}}, diffs: null, }, + '_includes (object)': { + actual: {'0-alpha': 1, '1-beta': 2, '3-gamma': 3}, + expected: {_includes: [ + ['0-alpha', '<2'], + [/[0-9]-beta/, 2], + ]}, + diffs: null, + }, '_excludes (1)': { actual: {prices: [0, 1, 2, 3, 4, 5]}, @@ -147,6 +155,16 @@ describe('findDiffersences', () => { message: 'Expected to not find matching entry via _excludes', }}], }, + '_excludes (object)': { + actual: {'0-alpha': 1, '1-beta': 2, '3-gamma': 3}, + expected: {_excludes: [ + [/[0-9]-beta/, 2], + ]}, + diffs: [{path: '', actual: ['1-beta', 2], expected: { + expectedExclusion: [/[0-9]-beta/, 2], + message: 'Expected to not find matching entry via _excludes', + }}], + }, '_includes and _excludes (1)': { actual: {prices: [0, 1, 2, 3, 4, 5]}, @@ -170,6 +188,22 @@ describe('findDiffersences', () => { message: 'Expected to not find matching entry via _excludes', }}], }, + '_includes and _excludes (object)': { + actual: {'0-alpha': 1, '1-beta': 2, '3-gamma': 3}, + expected: { + _includes: [ + ['0-alpha', '<2'], + ], + _excludes: [ + [/[0-9]-alpha/, 1], + [/[0-9]-beta/, 2], + ], + }, + diffs: [{path: '', actual: ['1-beta', 2], expected: { + expectedExclusion: [/[0-9]-beta/, 2], + message: 'Expected to not find matching entry via _excludes', + }}], + }, }; for (const [testName, {actual, expected, diffs}] of Object.entries(testCases)) { diff --git a/cli/test/smokehouse/report-assert.js b/cli/test/smokehouse/report-assert.js index 3856ac141afa..f2e030449840 100644 --- a/cli/test/smokehouse/report-assert.js +++ b/cli/test/smokehouse/report-assert.js @@ -105,6 +105,8 @@ function findDifferences(path, actual, expected) { /** @type {Difference[]} */ const diffs = []; + + /** @type {any[]|undefined} */ let inclExclCopy; // We only care that all expected's own properties are on actual (and not the other way around). @@ -116,15 +118,20 @@ function findDifferences(path, actual, expected) { const expectedValue = expected[key]; if (key === '_includes') { - inclExclCopy = [...actual]; + if (Array.isArray(actual)) { + inclExclCopy = [...actual]; + } else if (typeof actual === 'object') { + inclExclCopy = Object.entries(actual); + } if (!Array.isArray(expectedValue)) throw new Error('Array subset must be array'); - if (!Array.isArray(actual)) { + if (!inclExclCopy) { diffs.push({ path, - actual: 'Actual value is not an array', + actual: 'Actual value is not an array or object', expected, }); + continue; } for (const expectedEntry of expectedValue) { @@ -148,20 +155,33 @@ function findDifferences(path, actual, expected) { if (key === '_excludes') { // Re-use state from `_includes` check, if there was one. - /** @type {any[]} */ - const arrToCheckAgainst = inclExclCopy || actual; + if (!inclExclCopy) { + if (Array.isArray(actual)) { + // We won't be removing items, so we can just copy the reference. + inclExclCopy = actual; + } else if (typeof actual === 'object') { + inclExclCopy = Object.entries(actual); + } + } if (!Array.isArray(expectedValue)) throw new Error('Array subset must be array'); - if (!Array.isArray(actual)) continue; + if (!inclExclCopy) { + diffs.push({ + path, + actual: 'Actual value is not an array or object', + expected, + }); + continue; + } const expectedExclusions = expectedValue; for (const expectedExclusion of expectedExclusions) { - const matchingIndex = arrToCheckAgainst.findIndex(actualEntry => + const matchingIndex = inclExclCopy.findIndex(actualEntry => !findDifferences(keyPath, actualEntry, expectedExclusion)); if (matchingIndex !== -1) { diffs.push({ path, - actual: arrToCheckAgainst[matchingIndex], + actual: inclExclCopy[matchingIndex], expected: { message: 'Expected to not find matching entry via _excludes', expectedExclusion, diff --git a/cli/test/smokehouse/test-definitions/dobetterweb.js b/cli/test/smokehouse/test-definitions/dobetterweb.js index a5da83031a62..0642c4448d22 100644 --- a/cli/test/smokehouse/test-definitions/dobetterweb.js +++ b/cli/test/smokehouse/test-definitions/dobetterweb.js @@ -13,6 +13,24 @@ const config = { ], }; +const imgA = { + top: '650±50', + bottom: '650±50', + left: '10±10', + right: '120±20', + width: '120±20', + height: '20±20', +}; + +const imgB = { + top: '575±50', + bottom: '650±50', + left: '130±10', + right: '250±20', + width: '120±20', + height: '80±20', +}; + /** * @type {Smokehouse.ExpectedRunnerResult} * Expected Lighthouse audit values for Do Better Web tests. @@ -430,39 +448,17 @@ const expectations = { data: /^data:image\/webp;.{500,}/, }, nodes: { - // Test that the numbers for individual elements are in the ballpark. - // Exact ordering and IDs between FR and legacy differ, so fork the expectations. - '4-11-IMG': { - _minChromiumVersion: '104', - _legacyOnly: true, - top: '650±50', - bottom: '650±50', - left: '10±10', - right: '120±20', - width: '120±20', - height: '20±20', - }, - // Legacy runner execution context ID changed after 104.0.5100.0 - '5-11-IMG': { - _maxChromiumVersion: '103', - _legacyOnly: true, - top: '650±50', - bottom: '650±50', - left: '10±10', - right: '120±20', - width: '120±20', - height: '20±20', - }, - '9-1-IMG': { - _fraggleRockOnly: true, - top: '650±50', - bottom: '650±50', - left: '10±10', - right: '120±20', - width: '120±20', - height: '20±20', - }, - // And then many more nodes. + _includes: [ + // Test that the numbers for individual elements are in the ballpark. + [/[0-9]-[0-9]+-IMG/, imgA], + [/[0-9]-[0-9]+-IMG/, imgB], + // And then many more nodes... + ], + _excludes: [ + // Ensure that the nodes we found above are unique. + [/[0-9]-[0-9]+-IMG/, imgA], + [/[0-9]-[0-9]+-IMG/, imgB], + ], }, }, }, diff --git a/cli/test/smokehouse/test-definitions/screenshot.js b/cli/test/smokehouse/test-definitions/screenshot.js index 2abcc313bea0..9f9e1025737b 100644 --- a/cli/test/smokehouse/test-definitions/screenshot.js +++ b/cli/test/smokehouse/test-definitions/screenshot.js @@ -47,25 +47,12 @@ const expectations = { data: /data:image\/webp;base64,.{10000,}$/, }, nodes: { - // Gathered with no execution context isolation, shared between both FR and legacy. - 'page-0-P': {...elements.p}, - - // Legacy execution context IDs. - // Note: The first number (5) in these ids comes from an executionContextId, and has the potential to change. - // The following P is the same element as above but from a different JS context. This element - // starts with height ~18 and grows over time. See screenshot.html. - '5-0-BODY': {_legacyOnly: true, ...elements.body, _maxChromiumVersion: '103'}, - '5-2-P': {_legacyOnly: true, ...elements.p, _maxChromiumVersion: '103'}, - '5-3-HTML': {_legacyOnly: true, _maxChromiumVersion: '103'}, - // Legacy runner execution context ID changed after 104.0.5100.0 - '4-0-BODY': {_legacyOnly: true, ...elements.body, _minChromiumVersion: '104'}, - '4-2-P': {_legacyOnly: true, ...elements.p, _minChromiumVersion: '104'}, - '4-3-HTML': {_legacyOnly: true, _minChromiumVersion: '104'}, - - // Fraggle rock should contain the same elements just with different ids. - '9-0-P': {_fraggleRockOnly: true, ...elements.p}, - '9-2-BODY': {_fraggleRockOnly: true, ...elements.body}, - '9-1-HTML': {_fraggleRockOnly: true}, + _includes: [ + ['page-0-P', elements.p], + [/[0-9]-[0-9]-BODY/, elements.body], + [/[0-9]-[0-9]-P/, elements.p], + [/[0-9]-[0-9]-HTML/, {}], + ], }, }, },