-
Notifications
You must be signed in to change notification settings - Fork 655
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
chore(dep): upgrade to puppeteer 13.1.2 #740
Conversation
Screenshots used during testing can be 1+ MB, which trips up Workaround is to use |
@@ -60,7 +60,7 @@ function cleanStdOutput(output) { | |||
.replace(/(\s+at \S+) .*/g, '$1') | |||
.replace(/\s+at (async|processTicksAndRejections|process._tickCallback)(?=\n|$)/g, '') | |||
.replace( | |||
/appspot.com\/reports\/[0-9-]+.report.html/, | |||
/appspot.com\/reports\/[0-9-]+.report.html/g, |
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.
FAIL packages/cli/test/autorun-github.test.js (51.372s)
● Lighthouse CI autorun CLI with GitHub status check › should submit status check to GitHub
expect(received).toMatchInlineSnapshot(snapshot)
Snapshot name: `Lighthouse CI autorun CLI with GitHub status check should submit status check to GitHub 1`
- Snapshot - 1
+ Received + 1
@@ -16,11 +16,11 @@
Uploading median LHR of http://localhost:XXXX/bad.html...success!
Open the report at storage.googleapis.com/lighthouse-infrastructure.appspot.com/reports/XXXX-XXXX.report.html
Uploading median LHR of http://localhost:XXXX/good.html...success!
- Open the report at storage.googleapis.com/lighthouse-infrastructure.appspot.com/reports/XXXX-XXXX.report.html
+ Open the report at storage.googleapis.com/lighthouse-infrastructure.appspot.com/reports/XXXX-113.report.html
GitHub token found, attempting to set status...
GitHub accepted "failure" status for "lhci/url/bad.html".
GitHub accepted "success" status for "lhci/url/good.html".
↵
70 | );
71 |
> 72 | expect(stdout).toMatchInlineSnapshot(`
| ^
73 | "✅ .lighthouseci/ directory writable
74 | ✅ Configuration file found
75 | ✅ Chrome installation found
at Object.<anonymous> (packages/cli/test/autorun-github.test.js:72:20)
› 1 snapshot failed.
seems storybook ( |
newest chrome resulted in tons of minute text rendering diffs. can't even distinguish them myself but apparently the pixels are different. hence, updating tons of snapshots. |
await page.waitFor(parameters.waitFor); | ||
} | ||
// wait for the webfont request to avoid flakiness with webfont display | ||
await page.waitForNetworkIdle(); |
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.
seems we should just always wait.
RE: #740 (comment) Not sure why, but with the newest storybook this isn't an issue anymore. |
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.
Workaround is to use runInProcess: true
looks you managed to drop this after the storybook upgrades? that's good news.
body { | ||
min-height: 100vh; | ||
} */ | ||
|
||
@import 'https://fonts.googleapis.com/css?family=Roboto:400,500&display=block'; |
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 think these lines can also be removed now (but not 100% sure)
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.
No, this is the only way that storybook can have these fonts. The only other place they are added is to the app entry point, which storybook does not use.
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.
Oh gotcha
No description provided.