From e6e861ce1d1bd12a2ee17eb8770f6e703c2dc1e5 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov <34244704+Lightning00Blade@users.noreply.github.com> Date: Tue, 19 Mar 2024 20:29:42 +0100 Subject: [PATCH] chore: fix interception tests (#12105) --- test/.eslintrc.js | 6 ++ test/src/page.spec.ts | 10 ++- .../requestinterception-experimental.spec.ts | 88 +++++++++++++++---- test/src/requestinterception.spec.ts | 51 ++++++++--- 4 files changed, 118 insertions(+), 37 deletions(-) diff --git a/test/.eslintrc.js b/test/.eslintrc.js index 5f7746a76b9..ea697b0a472 100644 --- a/test/.eslintrc.js +++ b/test/.eslintrc.js @@ -36,6 +36,12 @@ module.exports = { selector: 'CallExpression[callee.object.name="it"] > MemberExpression > Identifier[name="deflake"], CallExpression[callee.object.name="it"] > MemberExpression > Identifier[name="deflakeOnly"]', }, + { + message: + 'No `expect` in EventHandler. They will never throw errors', + selector: + 'CallExpression[callee.property.name="on"] BlockStatement > :not(TryStatement) > ExpressionStatement > CallExpression[callee.object.callee.name="expect"]', + }, ], }, }, diff --git a/test/src/page.spec.ts b/test/src/page.spec.ts index ad88ec6e134..bc07b1d259b 100644 --- a/test/src/page.spec.ts +++ b/test/src/page.spec.ts @@ -1213,13 +1213,15 @@ describe('Page', function () { expect(result).toBe(36); await page.removeExposedFunction('compute'); - let error: Error | null = null; - await page + const error = await page .evaluate(async function () { return (globalThis as any).compute(9, 4); }) - .catch(_error => { - return (error = _error); + .then(() => { + return null; + }) + .catch(error => { + return error; }); expect(error).toBeTruthy(); }); diff --git a/test/src/requestinterception-experimental.spec.ts b/test/src/requestinterception-experimental.spec.ts index 966554fd5df..abb4afe03eb 100644 --- a/test/src/requestinterception-experimental.spec.ts +++ b/test/src/requestinterception-experimental.spec.ts @@ -94,24 +94,36 @@ describe('cooperative request interception', function () { const {page, server} = await getTestState(); await page.setRequestInterception(true); + let requestError; page.on('request', request => { if (isFavicon(request)) { void request.continue({}, 0); return; } - expect(request.url()).toContain('empty.html'); - expect(request.headers()['user-agent']).toBeTruthy(); - expect(request.method()).toBe('GET'); - expect(request.postData()).toBe(undefined); - expect(request.isNavigationRequest()).toBe(true); - expect(request.resourceType()).toBe('document'); - expect(request.frame() === page.mainFrame()).toBe(true); - expect(request.frame()!.url()).toBe('about:blank'); - void request.continue({}, 0); + try { + expect(request).toBeTruthy(); + expect(request.url()).toContain('empty.html'); + expect(request.headers()['user-agent']).toBeTruthy(); + expect(request.method()).toBe('GET'); + expect(request.postData()).toBe(undefined); + expect(request.isNavigationRequest()).toBe(true); + expect(request.resourceType()).toBe('document'); + expect(request.frame()!.url()).toBe('about:blank'); + expect(request.frame() === page.mainFrame()).toBe(true); + } catch (error) { + requestError = error; + } finally { + void request.continue({}, 0); + } }); + const response = (await page.goto(server.EMPTY_PAGE))!; - expect(response!.ok()).toBe(true); - expect(response!.remoteAddress().port).toBe(server.PORT); + if (requestError) { + throw requestError; + } + + expect(response.ok()).toBe(true); + expect(response.remoteAddress().port).toBe(server.PORT); }); // @see https://github.com/puppeteer/puppeteer/pull/3105 it('should work when POST is redirected with 302', async () => { @@ -141,16 +153,24 @@ describe('cooperative request interception', function () { server.setRedirect('/rrredirect', '/empty.html'); await page.setRequestInterception(true); + let requestError; page.on('request', request => { const headers = Object.assign({}, request.headers(), { foo: 'bar', }); void request.continue({headers}, 0); - - expect(request.continueRequestOverrides()).toEqual({headers}); + try { + expect(request.continueRequestOverrides()).toEqual({headers}); + } catch (error) { + requestError = error; + } }); // Make sure that the goto does not time out. await page.goto(server.PREFIX + '/rrredirect'); + + if (requestError) { + throw requestError; + } }); // @see https://github.com/puppeteer/puppeteer/issues/4743 it('should be able to remove headers', async () => { @@ -220,11 +240,20 @@ describe('cooperative request interception', function () { foo: 'bar', }); await page.setRequestInterception(true); + let requestError; page.on('request', request => { - expect(request.headers()['foo']).toBe('bar'); - void request.continue({}, 0); + try { + expect(request.headers()['foo']).toBe('bar'); + } catch (error) { + requestError = error; + } finally { + void request.continue({}, 0); + } }); const response = await page.goto(server.EMPTY_PAGE); + if (requestError) { + throw requestError; + } expect(response!.ok()).toBe(true); }); // @see https://github.com/puppeteer/puppeteer/issues/4337 @@ -250,11 +279,20 @@ describe('cooperative request interception', function () { await page.setExtraHTTPHeaders({referer: server.EMPTY_PAGE}); await page.setRequestInterception(true); + let requestError; page.on('request', request => { - expect(request.headers()['referer']).toBe(server.EMPTY_PAGE); - void request.continue({}, 0); + try { + expect(request.headers()['referer']).toBe(server.EMPTY_PAGE); + } catch (error) { + requestError = error; + } finally { + void request.continue({}, 0); + } }); const response = await page.goto(server.EMPTY_PAGE); + if (requestError) { + throw requestError; + } expect(response!.ok()).toBe(true); }); it('should be abortable', async () => { @@ -947,14 +985,26 @@ describe('cooperative request interception', function () { page.on('request', request => { void request.continue(); }); + let requestError; page.on('request', request => { - expect(request.isInterceptResolutionHandled()).toBeTruthy(); + try { + expect(request.isInterceptResolutionHandled()).toBeTruthy(); + } catch (error) { + requestError = error; + } }); page.on('request', request => { const {action} = request.interceptResolutionState(); - expect(action).toBe(InterceptResolutionAction.AlreadyHandled); + try { + expect(action).toBe(InterceptResolutionAction.AlreadyHandled); + } catch (error) { + requestError = error; + } }); await page.goto(server.EMPTY_PAGE); + if (requestError) { + throw requestError; + } }); }); }); diff --git a/test/src/requestinterception.spec.ts b/test/src/requestinterception.spec.ts index 4b88d30a3b4..277c381d670 100644 --- a/test/src/requestinterception.spec.ts +++ b/test/src/requestinterception.spec.ts @@ -22,23 +22,34 @@ describe('request interception', function () { const {page, server} = await getTestState(); await page.setRequestInterception(true); + let requestError; page.on('request', request => { if (isFavicon(request)) { void request.continue(); return; } - expect(request.url()).toContain('empty.html'); - expect(request.headers()['user-agent']).toBeTruthy(); - expect(request.headers()['accept']).toBeTruthy(); - expect(request.method()).toBe('GET'); - expect(request.postData()).toBe(undefined); - expect(request.isNavigationRequest()).toBe(true); - expect(request.resourceType()).toBe('document'); - expect(request.frame() === page.mainFrame()).toBe(true); - expect(request.frame()!.url()).toBe('about:blank'); - void request.continue(); + try { + expect(request).toBeTruthy(); + expect(request.url()).toContain('empty.html'); + expect(request.headers()['user-agent']).toBeTruthy(); + expect(request.method()).toBe('GET'); + expect(request.postData()).toBe(undefined); + expect(request.isNavigationRequest()).toBe(true); + expect(request.resourceType()).toBe('document'); + expect(request.frame()!.url()).toBe('about:blank'); + expect(request.frame() === page.mainFrame()).toBe(true); + } catch (error) { + requestError = error; + } finally { + void request.continue(); + } }); + const response = (await page.goto(server.EMPTY_PAGE))!; + if (requestError) { + throw requestError; + } + expect(response.ok()).toBe(true); expect(response.remoteAddress().port).toBe(server.PORT); }); @@ -162,11 +173,21 @@ describe('request interception', function () { foo: 'bar', }); await page.setRequestInterception(true); + let requestError; page.on('request', request => { - expect(request.headers()['foo']).toBe('bar'); - void request.continue(); + try { + expect(request.headers()['foo']).toBe('bar'); + } catch (error) { + requestError = error; + } finally { + void request.continue(); + } }); + const response = (await page.goto(server.EMPTY_PAGE))!; + if (requestError) { + throw requestError; + } expect(response.ok()).toBe(true); }); // @see https://github.com/puppeteer/puppeteer/issues/4337 @@ -192,11 +213,13 @@ describe('request interception', function () { await page.setExtraHTTPHeaders({referer: server.EMPTY_PAGE}); await page.setRequestInterception(true); - page.on('request', request => { - expect(request.headers()['referer']).toBe(server.EMPTY_PAGE); + let request!: HTTPRequest; + page.on('request', req => { + request = req; void request.continue(); }); const response = (await page.goto(server.EMPTY_PAGE))!; + expect(request.headers()['referer']).toBe(server.EMPTY_PAGE); expect(response.ok()).toBe(true); }); it('should be abortable', async () => {