deps: upgrade puppeteer to v21.3.6 #15490
Conversation
adamraine
requested review from
brendankenny
and removed request for
a team
adamraine
changed the title
| @@ -190,7 +190,6 @@ async function buildBundle(entryPath, distPath, opts = {minify: true}) { | |||
| // resolved eventually. | |||
| plugins.partialLoaders.inlineFs({ | |||
| verbose: Boolean(process.env.DEBUG), | |||
| ignorePaths: [require.resolve('puppeteer-core/lib/esm/puppeteer/common/Page.js')], | |||
There was a problem hiding this comment.
Page.js has moved but it looks like the comment that contained fs.readFileSync is not longer there so we should be good to remove.
| frameUrl: 'http://localhost:10200/dobetterweb/dbw_tester.html', | ||
| }], | ||
| }, | ||
| }, |
There was a problem hiding this comment.
drive-by outdated bfcache failure
|
|
||
| const browser = await puppeteer.launch({ |
There was a problem hiding this comment.
Puppeteer will now disable the bfcache if we aren't using tab target which is currently only available through an environment variable PUPPETEER_INTERNAL_TAB_TARGET. The env variable needs to be set before puppeteer is imported so we would need to do some annoying dynamic imports.
This is an alternative approach which just uses chrome-launcher with our desired flags. This should be better for our smoke tests in general because it's closer to how Lighthouse launches Chrome.
|
related unit failure?
|
Yup, I'll investigate. |
| @@ -164,7 +164,7 @@ describe('Individual modes API', function() { | |||
| const run = await api.startTimespan(state.page); | |||
| for (const iframe of page.frames()) { | |||
| if (iframe.url().includes('/oopif-simple-page.html')) { | |||
| iframe.click('button'); | |||
| await iframe.click('button'); | |||
There was a problem hiding this comment.
Without this await the button click could happen after we end the timespan
This is a fix for the issue mentioned in #15490 (comment)
| expect(networkRequests).toHaveLength(10); | ||
| expect(networkRequests.filter(r => r.sessionTargetType === 'page')).toHaveLength(2); | ||
| expect(networkRequests.filter(r => r.sessionTargetType === 'iframe')).toHaveLength(2); | ||
| expect(networkRequests.filter(r => r.sessionTargetType === 'worker')).toHaveLength(6); |
There was a problem hiding this comment.
All of these checks are redundant with the snapshot check just below. The snapshot check provides better details on failure so we should just fall to that if a request is missing.
adamraine
changed the title
adamraine
changed the title
Issues that forced us to pin the version (#15458) should be fixed now.