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

Merged
merged 5 commits into from
Aug 9, 2022

Conversation

adamraine
Copy link
Member

Chrome execution context ids just changed under us again, so instead of adding another chrome version case we can change our smoke assertions to be more flexible.

This allows us to check the Object.entries of an object using _includes and _excludes.

@adamraine adamraine requested a review from a team as a code owner August 9, 2022 20:34
@adamraine adamraine requested review from connorjclark and removed request for a team August 9, 2022 20:34
'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

Copy link
Collaborator

@connorjclark connorjclark left a comment

Choose a reason for hiding this comment

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

Much better, nice!

@brendankenny
Copy link
Member

FWIW this doesn't really exist for jest's expect, probably because at this point you'd just ask, well why did you use object keys for these? :)

The ship has sailed on that decision if we want non-annoying backwards compatibility, but instead can we just pick non-flakey IDs instead of context IDs? We literally control these nodes' universe, there's no reason for us to have to use these Chrome-assigned numbers for their names.

@adamraine adamraine merged commit 65d1ec0 into master Aug 9, 2022
@adamraine adamraine deleted the smoke-object-subset branch August 9, 2022 21:47
alexnj pushed a commit to alexnj/lighthouse that referenced this pull request Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants