-
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(viewer): upgrade pptr to handle new CSSOM use in the report #5191
Conversation
|
||
// Rethrow to make sure it's auditable as an error. | ||
setTimeout(_ => { | ||
throw new Error(msg); |
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.
Why the settimeout?
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.
Why the settimeout?
it's a hack that lets logger catch it so the report still acts nicely (with a toast) even if the report is invalid or whatever, but now it also async throws the error so that the problem can be picked up in the console and (more importantly) in the puppeteer pageerror
listener we add in the test :)
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.
added to the comment to clarify
@@ -97,7 +97,7 @@ | |||
"npm-run-posix-or-windows": "^2.0.2", | |||
"nyc": "^11.6.0", | |||
"postinstall-prepare": "^1.0.1", | |||
"puppeteer": "^1.1.1", | |||
"puppeteer": "1.4.0", |
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.
Should the version be more flexible?
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.
Should the version be more flexible?
I have no strong feelings, but with the yarn.lock file we pretty much have to manually rev anyways, so not sure if it matters
@@ -66,6 +66,10 @@ describe('Lighthouse Viewer', function() { | |||
}); | |||
|
|||
after(async function() { | |||
// Log any page load errors encountered in case before() failed. | |||
// eslint-disable-next-line no-console | |||
console.log(pageErrors); |
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.
console.error
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.
console.error
done
computedStyleMap
in the report fails in the Chromium version that our current version of puppeteer ships with. This revs the version so that we get a more recent Chromium.Drive by change to viewer and the viewer pptr test so that when there's an in-page error like this, the test can pick it up and print it to console instead of just timing out in
before()
waiting for the report to load.