Skip to content

Commit

Permalink
tests(smoke): check objects against a subset of keys (#14270)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine committed Aug 9, 2022
1 parent d14c7e6 commit 65d1ec0
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 62 deletions.
8 changes: 6 additions & 2 deletions cli/test/smokehouse/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand All @@ -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

Expand Down
34 changes: 34 additions & 0 deletions cli/test/smokehouse/report-assert-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]},
Expand All @@ -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]},
Expand All @@ -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)) {
Expand Down
36 changes: 28 additions & 8 deletions cli/test/smokehouse/report-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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) {
Expand All @@ -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,
Expand Down
62 changes: 29 additions & 33 deletions cli/test/smokehouse/test-definitions/dobetterweb.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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],
],
},
},
},
Expand Down
25 changes: 6 additions & 19 deletions cli/test/smokehouse/test-definitions/screenshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/, {}],
],
},
},
},
Expand Down

0 comments on commit 65d1ec0

Please sign in to comment.