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.

Sign up for GitHub

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): check objects against a subset of keys #14270

Merged
merged 5 commits into from
Aug 9, 2022
Merged

tests(smoke): check objects against a subset of keys #14270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eb6bfc1
tests(smoke): check objects against a subset of keys
adamraine Aug 9, 2022
4b307cc
dbw
adamraine Aug 9, 2022
648d26b
perf
adamraine Aug 9, 2022
29f1f51
1 more img
adamraine Aug 9, 2022
8292d11
unique
adamraine Aug 9, 2022
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
8 changes: 6 additions & 2 deletions cli/test/smokehouse/readme.md
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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;
adamraine marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
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
View file
Open in desktop
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],
Copy link
Collaborator

Choose a reason for hiding this comment

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

kinda crazy how well this composes

[/[0-9]-[0-9]-BODY/, elements.body],
[/[0-9]-[0-9]-P/, elements.p],
[/[0-9]-[0-9]-HTML/, {}],
],
},
},
},
Expand Down