-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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): make specific assertions about deprecations #13191
Conversation
items: [ | ||
{ | ||
value: /Application Cache API manifest/, | ||
_maxChromiumMilestone: 94, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, is it 96?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like 94 is correct: https://www.chromestatus.com/feature/6192449487634432
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't think we want to assert order here or we end up with more flakiness, no?
value: /'window.webkitStorageInfo' is deprecated/, | ||
}, | ||
{ | ||
value: /Synchronous XMLHttpRequest on the main thread is deprecated/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how is this passing? won't this fail in later milestones because it's at index 1
not 2
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well currently canary crashes due to unrelated reasons, so I couldn't test it locally.. but you are correct. Added a quick fix to the report asserting that should make this better
@@ -200,26 +200,42 @@ function pruneExpectations(localConsole, lhr, expected, reportOptions) { | |||
JSON.stringify(value, null, 2), | |||
`Actual Chromium version: ${getChromeVersion()}`, | |||
].join(' ')); | |||
delete obj[key]; | |||
if (Array.isArray(obj)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removeKey
helper?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not feeling DRY is necessary here
items: [ | ||
{ | ||
value: /Application Cache API manifest/, | ||
_maxChromiumMilestone: 94, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like 94 is correct: https://www.chromestatus.com/feature/6192449487634432
fixed this before seeing #13179... my fix was independent. oops :)
going through with it still because being specific here in the assertions is nice