From 60a415b1a858e228ca9b33dcc9b5abf64af76e4a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 25 Oct 2021 09:56:39 +0000 Subject: [PATCH 1/6] feat: handle unhandled promise rejections in tests --- package.json | 2 +- src/common/Browser.ts | 4 ++++ test/frame.spec.ts | 2 +- test/launcher.spec.ts | 3 ++- test/mocha-utils.ts | 4 ++++ test/network.spec.ts | 20 +++++++++----------- 6 files changed, 21 insertions(+), 14 deletions(-) diff --git a/package.json b/package.json index 6ae2619391a36..6fb63eb90b208 100644 --- a/package.json +++ b/package.json @@ -104,7 +104,7 @@ "minimist": "1.2.0", "mocha": "9.1.3", "ncp": "2.0.0", - "pixelmatch": "4.0.2", + "pixelmatch": "5.2.1", "pngjs": "6.0.0", "prettier": "2.3.0", "sinon": "9.2.4", diff --git a/src/common/Browser.ts b/src/common/Browser.ts index e8ff50bb601ef..16bbee1a5e607 100644 --- a/src/common/Browser.ts +++ b/src/common/Browser.ts @@ -240,6 +240,7 @@ export class Browser extends EventEmitter { private _defaultContext: BrowserContext; private _contexts: Map; private _screenshotTaskQueue: TaskQueue; + private _ignoredTargets = new Set(); /** * @internal * Used in Target.ts directly so cannot be marked private. @@ -374,6 +375,7 @@ export class Browser extends EventEmitter { const shouldAttachToTarget = this._targetFilterCallback(targetInfo); if (!shouldAttachToTarget) { + this._ignoredTargets.add(targetInfo.targetId); return; } @@ -398,6 +400,7 @@ export class Browser extends EventEmitter { } private async _targetDestroyed(event: { targetId: string }): Promise { + if (this._ignoredTargets.has(event.targetId)) return; const target = this._targets.get(event.targetId); target._initializedCallback(false); this._targets.delete(event.targetId); @@ -413,6 +416,7 @@ export class Browser extends EventEmitter { private _targetInfoChanged( event: Protocol.Target.TargetInfoChangedEvent ): void { + if (this._ignoredTargets.has(event.targetInfo.targetId)) return; const target = this._targets.get(event.targetInfo.targetId); assert(target, 'target should exist before targetInfoChanged'); const previousURL = target.url(); diff --git a/test/frame.spec.ts b/test/frame.spec.ts index f1310f6905dfd..29ae553a32392 100644 --- a/test/frame.spec.ts +++ b/test/frame.spec.ts @@ -88,7 +88,7 @@ describe('Frame specs', function () { // This test checks if Frame.evaluate allows a readonly array to be an argument. // See https://github.com/puppeteer/puppeteer/issues/6953. const readonlyArray: readonly string[] = ['a', 'b', 'c']; - mainFrame.evaluate((arr) => arr, readonlyArray); + await mainFrame.evaluate((arr) => arr, readonlyArray); }); }); diff --git a/test/launcher.spec.ts b/test/launcher.spec.ts index 9ec84e936684e..dabb19c772494 100644 --- a/test/launcher.spec.ts +++ b/test/launcher.spec.ts @@ -611,7 +611,8 @@ describe('Launcher specs', function () { await page2.close(); await page1.close(); - await browser.close(); + await browser.disconnect(); + await originalBrowser.close(); expect(pages.map((p: Page) => p.url()).sort()).toEqual([ 'about:blank', diff --git a/test/mocha-utils.ts b/test/mocha-utils.ts index 238addfef23ac..6f701f470c707 100644 --- a/test/mocha-utils.ts +++ b/test/mocha-utils.ts @@ -221,6 +221,10 @@ console.log( }` ); +process.on('unhandledRejection', (reason) => { + throw reason; +}); + export const setupTestBrowserHooks = (): void => { before(async () => { const browser = await puppeteer.launch(defaultBrowserOptions); diff --git a/test/network.spec.ts b/test/network.spec.ts index 9450a76be2878..390ec8e6176b8 100644 --- a/test/network.spec.ts +++ b/test/network.spec.ts @@ -69,27 +69,25 @@ describe('network', function () { }); describeFailsFirefox('Request.continue', () => { - it('should split a request header at new line characters and add the header multiple times instead', async () => { + it('should fail if the header value in invalid', async () => { const { page, server } = getTestState(); - let resolve; - const errorPromise = new Promise((r) => { - resolve = r; - }); - + let error; await page.setRequestInterception(true); page.on('request', async (request) => { await request .continue({ headers: { - 'X-Test-Header': 'a\nb', + 'X-Invalid-Header': 'a\nb', }, }) - .catch(resolve); + .catch((error_) => { + error = error_; + }); + await request.continue(); }); - page.goto(server.PREFIX + '/empty.html'); - const error = await errorPromise; - expect(error).toBeTruthy(); + await page.goto(server.PREFIX + '/empty.html'); + expect(error.message).toMatch(/Invalid header/); }); }); describe('Request.frame', function () { From c7f8bccba36689185d1a9ed46482ecc7cd981c2d Mon Sep 17 00:00:00 2001 From: Jan Scheffler Date: Tue, 26 Oct 2021 15:28:02 +0200 Subject: [PATCH 2/6] chore: update tests --- src/common/HTTPRequest.ts | 12 ++++++--- test/network.spec.ts | 23 ----------------- test/requestinterception.spec.ts | 43 ++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 26 deletions(-) diff --git a/src/common/HTTPRequest.ts b/src/common/HTTPRequest.ts index d4ba66333f7ee..fb8546d44cbf4 100644 --- a/src/common/HTTPRequest.ts +++ b/src/common/HTTPRequest.ts @@ -230,7 +230,7 @@ export class HTTPRequest { async finalizeInterceptions(): Promise { await this._interceptActions.reduce( (promiseChain, interceptAction) => - promiseChain.then(interceptAction).catch(handleError), + promiseChain.then(interceptAction), Promise.resolve() ); const [resolution] = this.interceptResolution(); @@ -440,7 +440,10 @@ export class HTTPRequest { postData: postDataBinaryBase64, headers: headers ? headersArray(headers) : undefined, }) - .catch(handleError); + .catch((error) => { + this._interceptionHandled = false; + return handleError(error); + }); } /** @@ -532,7 +535,10 @@ export class HTTPRequest { responseHeaders: headersArray(responseHeaders), body: responseBody ? responseBody.toString('base64') : undefined, }) - .catch(handleError); + .catch((error) => { + this._interceptionHandled = false; + return handleError(error); + }); } /** diff --git a/test/network.spec.ts b/test/network.spec.ts index 390ec8e6176b8..de6f1402ba9a0 100644 --- a/test/network.spec.ts +++ b/test/network.spec.ts @@ -67,29 +67,6 @@ describe('network', function () { expect(requests.length).toBe(2); }); }); - - describeFailsFirefox('Request.continue', () => { - it('should fail if the header value in invalid', async () => { - const { page, server } = getTestState(); - - let error; - await page.setRequestInterception(true); - page.on('request', async (request) => { - await request - .continue({ - headers: { - 'X-Invalid-Header': 'a\nb', - }, - }) - .catch((error_) => { - error = error_; - }); - await request.continue(); - }); - await page.goto(server.PREFIX + '/empty.html'); - expect(error.message).toMatch(/Invalid header/); - }); - }); describe('Request.frame', function () { it('should work for main frame navigation request', async () => { const { page, server } = getTestState(); diff --git a/test/requestinterception.spec.ts b/test/requestinterception.spec.ts index 600e882961e07..0bde69e013c87 100644 --- a/test/requestinterception.spec.ts +++ b/test/requestinterception.spec.ts @@ -626,6 +626,26 @@ describe('request interception', function () { expect(serverRequest.method).toBe('POST'); expect(await serverRequest.postBody).toBe('doggo'); }); + it('should fail if the header value is invalid', async () => { + const { page, server } = getTestState(); + + let error; + await page.setRequestInterception(true); + page.on('request', async (request) => { + await request + .continue({ + headers: { + 'X-Invalid-Header': 'a\nb', + }, + }) + .catch((error_) => { + error = error_; + }); + await request.continue(); + }); + await page.goto(server.PREFIX + '/empty.html'); + expect(error.message).toMatch(/Invalid header/); + }); }); describeFailsFirefox('Request.respond', function () { @@ -732,6 +752,29 @@ describe('request interception', function () { 'Yo, page!' ); }); + it('should fail if the header value is invalid', async () => { + const { page, server } = getTestState(); + + let error; + await page.setRequestInterception(true); + page.on('request', async (request) => { + await request + .respond({ + headers: { + 'X-Invalid-Header': 'a\nb', + }, + }) + .catch((error_) => { + error = error_; + }); + await request.respond({ + status: 200, + body: 'Hello World', + }); + }); + await page.goto(server.PREFIX + '/empty.html'); + expect(error.message).toMatch(/Invalid header/); + }); }); }); From 241b9d8cd648f5fab140c593307b0b0fbb7f4540 Mon Sep 17 00:00:00 2001 From: Jan Scheffler Date: Tue, 26 Oct 2021 15:32:14 +0200 Subject: [PATCH 3/6] chore: eslint --- src/common/HTTPRequest.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/common/HTTPRequest.ts b/src/common/HTTPRequest.ts index fb8546d44cbf4..6f1dd4b543f62 100644 --- a/src/common/HTTPRequest.ts +++ b/src/common/HTTPRequest.ts @@ -229,8 +229,7 @@ export class HTTPRequest { */ async finalizeInterceptions(): Promise { await this._interceptActions.reduce( - (promiseChain, interceptAction) => - promiseChain.then(interceptAction), + (promiseChain, interceptAction) => promiseChain.then(interceptAction), Promise.resolve() ); const [resolution] = this.interceptResolution(); From 3817cd62f89a114f54be457f929f53f72e301933 Mon Sep 17 00:00:00 2001 From: Jan Scheffler Date: Tue, 26 Oct 2021 16:03:31 +0200 Subject: [PATCH 4/6] chore: test why firefox fails --- test/launcher.spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/launcher.spec.ts b/test/launcher.spec.ts index dabb19c772494..3b213bf13b93a 100644 --- a/test/launcher.spec.ts +++ b/test/launcher.spec.ts @@ -603,8 +603,10 @@ describe('Launcher specs', function () { const browser = await puppeteer.connect({ browserWSEndpoint, - targetFilter: (targetInfo: Protocol.Target.TargetInfo) => - !targetInfo.url.includes('should-be-ignored'), + targetFilter: (targetInfo: Protocol.Target.TargetInfo) => { + console.log(targetInfo); + return !targetInfo.url.includes('should-be-ignored'); + }, }); const pages = await browser.pages(); From 6ad30f4074452d09ddfafcf8d8f20722e5a2693c Mon Sep 17 00:00:00 2001 From: Jan Scheffler Date: Wed, 27 Oct 2021 11:54:02 +0200 Subject: [PATCH 5/6] chore: fix error on FF --- src/common/DOMWorld.ts | 4 ++++ test/launcher.spec.ts | 29 ++++++++++++++--------------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/common/DOMWorld.ts b/src/common/DOMWorld.ts index 57bdd2efdda73..14d5b1037eb9d 100644 --- a/src/common/DOMWorld.ts +++ b/src/common/DOMWorld.ts @@ -115,6 +115,10 @@ export class DOMWorld { async _setContext(context?: ExecutionContext): Promise { if (context) { + assert( + this._contextResolveCallback, + 'Execution Context has already been set.' + ); this._ctxBindings.clear(); this._contextResolveCallback.call(null, context); this._contextResolveCallback = null; diff --git a/test/launcher.spec.ts b/test/launcher.spec.ts index 3b213bf13b93a..fbf4fae0f71d6 100644 --- a/test/launcher.spec.ts +++ b/test/launcher.spec.ts @@ -133,7 +133,7 @@ describe('Launcher specs', function () { describe('Browser.disconnect', function () { it('should reject navigation when browser closes', async () => { const { server, puppeteer, defaultBrowserOptions } = getTestState(); - server.setRoute('/one-style.css', () => {}); + server.setRoute('/one-style.css', () => { }); const browser = await puppeteer.launch(defaultBrowserOptions); const remote = await puppeteer.connect({ browserWSEndpoint: browser.wsEndpoint(), @@ -153,7 +153,7 @@ describe('Launcher specs', function () { it('should reject waitForSelector when browser closes', async () => { const { server, puppeteer, defaultBrowserOptions } = getTestState(); - server.setRoute('/empty.html', () => {}); + server.setRoute('/empty.html', () => { }); const browser = await puppeteer.launch(defaultBrowserOptions); const remote = await puppeteer.connect({ browserWSEndpoint: browser.wsEndpoint(), @@ -197,7 +197,7 @@ describe('Launcher specs', function () { const page = await browser.newPage(); let error = null; const neverResolves = page - .evaluate(() => new Promise(() => {})) + .evaluate(() => new Promise(() => { })) .catch((error_) => (error = error_)); await browser.close(); await neverResolves; @@ -225,7 +225,7 @@ describe('Launcher specs', function () { await browser.close(); expect(fs.readdirSync(userDataDir).length).toBeGreaterThan(0); // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 - await rmAsync(userDataDir).catch(() => {}); + await rmAsync(userDataDir).catch(() => { }); }); it('userDataDir argument', async () => { const { isChrome, puppeteer, defaultBrowserOptions } = getTestState(); @@ -249,7 +249,7 @@ describe('Launcher specs', function () { await browser.close(); expect(fs.readdirSync(userDataDir).length).toBeGreaterThan(0); // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 - await rmAsync(userDataDir).catch(() => {}); + await rmAsync(userDataDir).catch(() => { }); }); it('userDataDir option should restore state', async () => { const { server, puppeteer, defaultBrowserOptions } = getTestState(); @@ -268,7 +268,7 @@ describe('Launcher specs', function () { expect(await page2.evaluate(() => localStorage.hey)).toBe('hello'); await browser2.close(); // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 - await rmAsync(userDataDir).catch(() => {}); + await rmAsync(userDataDir).catch(() => { }); }); // This mysteriously fails on Windows on AppVeyor. See // https://github.com/puppeteer/puppeteer/issues/4111 @@ -282,8 +282,8 @@ describe('Launcher specs', function () { await page.goto(server.EMPTY_PAGE); await page.evaluate( () => - (document.cookie = - 'doSomethingOnlyOnce=true; expires=Fri, 31 Dec 9999 23:59:59 GMT') + (document.cookie = + 'doSomethingOnlyOnce=true; expires=Fri, 31 Dec 9999 23:59:59 GMT') ); await browser.close(); @@ -295,7 +295,7 @@ describe('Launcher specs', function () { ); await browser2.close(); // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 - await rmAsync(userDataDir).catch(() => {}); + await rmAsync(userDataDir).catch(() => { }); }); it('should return the default arguments', async () => { const { isChrome, isFirefox, puppeteer } = getTestState(); @@ -589,7 +589,8 @@ describe('Launcher specs', function () { await page.close(); await browser.close(); }); - it('should support targetFilter option', async () => { + // @see https://github.com/puppeteer/puppeteer/issues/4197 + itFailsFirefox('should support targetFilter option', async () => { const { server, puppeteer, defaultBrowserOptions } = getTestState(); const originalBrowser = await puppeteer.launch(defaultBrowserOptions); @@ -603,10 +604,8 @@ describe('Launcher specs', function () { const browser = await puppeteer.connect({ browserWSEndpoint, - targetFilter: (targetInfo: Protocol.Target.TargetInfo) => { - console.log(targetInfo); - return !targetInfo.url.includes('should-be-ignored'); - }, + targetFilter: (targetInfo: Protocol.Target.TargetInfo) => + !targetInfo.url?.includes('should-be-ignored'), }); const pages = await browser.pages(); @@ -615,7 +614,7 @@ describe('Launcher specs', function () { await page1.close(); await browser.disconnect(); await originalBrowser.close(); - + expect(pages.map((p: Page) => p.url()).sort()).toEqual([ 'about:blank', server.EMPTY_PAGE, From 22344a957a4ba405fa417c67f6fcfc9be5ec557d Mon Sep 17 00:00:00 2001 From: Jan Scheffler Date: Wed, 27 Oct 2021 12:04:08 +0200 Subject: [PATCH 6/6] chore: eslint-fix --- test/launcher.spec.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/launcher.spec.ts b/test/launcher.spec.ts index fbf4fae0f71d6..614161c99dc77 100644 --- a/test/launcher.spec.ts +++ b/test/launcher.spec.ts @@ -133,7 +133,7 @@ describe('Launcher specs', function () { describe('Browser.disconnect', function () { it('should reject navigation when browser closes', async () => { const { server, puppeteer, defaultBrowserOptions } = getTestState(); - server.setRoute('/one-style.css', () => { }); + server.setRoute('/one-style.css', () => {}); const browser = await puppeteer.launch(defaultBrowserOptions); const remote = await puppeteer.connect({ browserWSEndpoint: browser.wsEndpoint(), @@ -153,7 +153,7 @@ describe('Launcher specs', function () { it('should reject waitForSelector when browser closes', async () => { const { server, puppeteer, defaultBrowserOptions } = getTestState(); - server.setRoute('/empty.html', () => { }); + server.setRoute('/empty.html', () => {}); const browser = await puppeteer.launch(defaultBrowserOptions); const remote = await puppeteer.connect({ browserWSEndpoint: browser.wsEndpoint(), @@ -197,7 +197,7 @@ describe('Launcher specs', function () { const page = await browser.newPage(); let error = null; const neverResolves = page - .evaluate(() => new Promise(() => { })) + .evaluate(() => new Promise(() => {})) .catch((error_) => (error = error_)); await browser.close(); await neverResolves; @@ -225,7 +225,7 @@ describe('Launcher specs', function () { await browser.close(); expect(fs.readdirSync(userDataDir).length).toBeGreaterThan(0); // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 - await rmAsync(userDataDir).catch(() => { }); + await rmAsync(userDataDir).catch(() => {}); }); it('userDataDir argument', async () => { const { isChrome, puppeteer, defaultBrowserOptions } = getTestState(); @@ -249,7 +249,7 @@ describe('Launcher specs', function () { await browser.close(); expect(fs.readdirSync(userDataDir).length).toBeGreaterThan(0); // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 - await rmAsync(userDataDir).catch(() => { }); + await rmAsync(userDataDir).catch(() => {}); }); it('userDataDir option should restore state', async () => { const { server, puppeteer, defaultBrowserOptions } = getTestState(); @@ -268,7 +268,7 @@ describe('Launcher specs', function () { expect(await page2.evaluate(() => localStorage.hey)).toBe('hello'); await browser2.close(); // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 - await rmAsync(userDataDir).catch(() => { }); + await rmAsync(userDataDir).catch(() => {}); }); // This mysteriously fails on Windows on AppVeyor. See // https://github.com/puppeteer/puppeteer/issues/4111 @@ -282,8 +282,8 @@ describe('Launcher specs', function () { await page.goto(server.EMPTY_PAGE); await page.evaluate( () => - (document.cookie = - 'doSomethingOnlyOnce=true; expires=Fri, 31 Dec 9999 23:59:59 GMT') + (document.cookie = + 'doSomethingOnlyOnce=true; expires=Fri, 31 Dec 9999 23:59:59 GMT') ); await browser.close(); @@ -295,7 +295,7 @@ describe('Launcher specs', function () { ); await browser2.close(); // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 - await rmAsync(userDataDir).catch(() => { }); + await rmAsync(userDataDir).catch(() => {}); }); it('should return the default arguments', async () => { const { isChrome, isFirefox, puppeteer } = getTestState(); @@ -614,7 +614,7 @@ describe('Launcher specs', function () { await page1.close(); await browser.disconnect(); await originalBrowser.close(); - + expect(pages.map((p: Page) => p.url()).sort()).toEqual([ 'about:blank', server.EMPTY_PAGE,