From 1a2002971f68105c121c171dfc1de81c7ad7aa2c Mon Sep 17 00:00:00 2001 From: Kelvin Del Monte Date: Fri, 27 Oct 2023 02:31:25 -0400 Subject: [PATCH 1/3] Add functionality to run same test in multiple browsers --- .github/workflows/main.yml | 2 ++ package.json | 2 +- test/browser.ts | 32 +++++++++++++++----------------- test/helpers/with-page.ts | 31 ++++++++++++++++++++----------- 4 files changed, 38 insertions(+), 29 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index bf55194c..5a6119b9 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -17,6 +17,8 @@ jobs: with: node-version: ${{ matrix.node-version }} - run: npm install + - name: Install Playwright Browsers + run: npx playwright install --with-deps - run: npm test # TODO: Enable again when https://github.com/istanbuljs/nyc/issues/1287 is fixed. # - uses: codecov/codecov-action@v1 diff --git a/package.json b/package.json index a2461598..5feaf2cc 100644 --- a/package.json +++ b/package.json @@ -67,7 +67,7 @@ "expect-type": "^0.16.0", "express": "^4.18.2", "pify": "^6.1.0", - "playwright-chromium": "^1.37.1", + "playwright": "^1.39.0", "raw-body": "^2.5.2", "ts-node": "^10.9.1", "typescript": "^5.2.2", diff --git a/test/browser.ts b/test/browser.ts index 7a5d1d92..7be3faa9 100644 --- a/test/browser.ts +++ b/test/browser.ts @@ -1,11 +1,11 @@ import test, {type ExecutionContext} from 'ava'; import busboy from 'busboy'; import express from 'express'; -import {type Page} from 'playwright-chromium'; +import {chromium, webkit, type Page} from 'playwright'; import type ky from '../source/index.js'; import {createHttpTestServer, type ExtendedHttpTestServer, type HttpServerOptions} from './helpers/create-http-test-server.js'; import {parseRawBody} from './helpers/parse-body.js'; -import {withPage} from './helpers/with-page.js'; +import {browserTest, defaultBrowsersTest} from './helpers/with-page.js'; declare global { // eslint-disable-next-line @typescript-eslint/consistent-type-definitions @@ -46,7 +46,7 @@ test.afterEach(async () => { await server.close(); }); -test.serial('prefixUrl option', withPage, async (t: ExecutionContext, page: Page) => { +defaultBrowsersTest('prefixUrl option', async (t: ExecutionContext, page: Page) => { server.get('/', (_request, response) => { response.end('zebra'); }); @@ -74,7 +74,7 @@ test.serial('prefixUrl option', withPage, async (t: ExecutionContext, page: Page t.deepEqual(results, ['rainbow', 'rainbow', 'rainbow', 'rainbow']); }); -test.serial('aborting a request', withPage, async (t: ExecutionContext, page: Page) => { +defaultBrowsersTest('aborting a request', async (t: ExecutionContext, page: Page) => { server.get('/', (_request, response) => { response.end('meow'); }); @@ -98,7 +98,7 @@ test.serial('aborting a request', withPage, async (t: ExecutionContext, page: Pa t.is(errorName, 'AbortError'); }); -test.serial('should copy origin response info when using `onDownloadProgress`', withPage, async (t: ExecutionContext, page: Page) => { +defaultBrowsersTest('should copy origin response info when using `onDownloadProgress`', async (t: ExecutionContext, page: Page) => { const json = {hello: 'world'}; const status = 202; const statusText = 'Accepted'; @@ -133,7 +133,7 @@ test.serial('should copy origin response info when using `onDownloadProgress`', }); }); -test.serial('should not copy response body with 204 status code when using `onDownloadProgress` ', withPage, async (t: ExecutionContext, page: Page) => { +defaultBrowsersTest('should not copy response body with 204 status code when using `onDownloadProgress` ', async (t: ExecutionContext, page: Page) => { const status = 204; const statusText = 'No content'; server.get('/', (_request, response) => { @@ -182,7 +182,7 @@ test.serial('should not copy response body with 204 status code when using `onDo }]); }); -test.serial('aborting a request with onDonwloadProgress', withPage, async (t: ExecutionContext, page: Page) => { +browserTest('aborting a request with onDownloadProgress', [chromium], async (t: ExecutionContext, page: Page) => { server.get('/', (_request, response) => { response.end('meow'); }); @@ -214,9 +214,8 @@ test.serial('aborting a request with onDonwloadProgress', withPage, async (t: Ex t.is(error, 'TypeError: Failed to fetch'); }); -test.serial( +defaultBrowsersTest( 'throws TimeoutError even though it does not support AbortController', - withPage, async (t: ExecutionContext, page: Page) => { server.get('/', (_request, response) => { response.end(); @@ -249,7 +248,7 @@ test.serial( }, ); -test.serial('onDownloadProgress works', withPage, async (t: ExecutionContext, page: Page) => { +browserTest('onDownloadProgress works', [chromium, webkit], async (t: ExecutionContext, page: Page) => { server.get('/', (_request, response) => { response.writeHead(200, { 'content-length': '4', @@ -289,7 +288,7 @@ test.serial('onDownloadProgress works', withPage, async (t: ExecutionContext, pa t.is(result.text, 'meow'); }); -test.serial('throws if onDownloadProgress is not a function', withPage, async (t: ExecutionContext, page: Page) => { +defaultBrowsersTest('throws if onDownloadProgress is not a function', async (t: ExecutionContext, page: Page) => { server.get('/', (_request, response) => { response.end(); }); @@ -305,7 +304,7 @@ test.serial('throws if onDownloadProgress is not a function', withPage, async (t t.is(error, 'TypeError: The `onDownloadProgress` option must be a function'); }); -test.serial('throws if does not support ReadableStream', withPage, async (t: ExecutionContext, page: Page) => { +defaultBrowsersTest('throws if does not support ReadableStream', async (t: ExecutionContext, page: Page) => { server.get('/', (_request, response) => { response.end(); }); @@ -322,7 +321,7 @@ test.serial('throws if does not support ReadableStream', withPage, async (t: Exe t.is(error, 'Error: Streams are not supported in your environment. `ReadableStream` is missing.'); }); -test.serial('FormData with searchParams', withPage, async (t: ExecutionContext, page: Page) => { +defaultBrowsersTest('FormData with searchParams', async (t: ExecutionContext, page: Page) => { t.plan(3); server.get('/', (_request, response) => { @@ -354,7 +353,7 @@ test.serial('FormData with searchParams', withPage, async (t: ExecutionContext, }, server.url); }); -test.serial('FormData with searchParams ("multipart/form-data" parser)', withPage, async (t: ExecutionContext, page: Page) => { +defaultBrowsersTest('FormData with searchParams ("multipart/form-data" parser)', async (t: ExecutionContext, page: Page) => { t.plan(3); server.get('/', (_request, response) => { @@ -426,9 +425,8 @@ test.serial('FormData with searchParams ("multipart/form-data" parser)', withPag }, server.url); }); -test.serial( +defaultBrowsersTest( 'headers are preserved when input is a Request and there are searchParams in the options', - withPage, async (t: ExecutionContext, page: Page) => { t.plan(2); @@ -459,7 +457,7 @@ test.serial( }, ); -test.serial('retry with body', withPage, async (t: ExecutionContext, page: Page) => { +browserTest('retry with body', [chromium, webkit], async (t: ExecutionContext, page: Page) => { t.plan(4); let requestCount = 0; diff --git a/test/helpers/with-page.ts b/test/helpers/with-page.ts index 230ffc23..7925c84b 100644 --- a/test/helpers/with-page.ts +++ b/test/helpers/with-page.ts @@ -1,19 +1,28 @@ +/* eslint-disable ava/no-ignored-test-files */ import process from 'node:process'; -import type {ExecutionContext, Implementation} from 'ava'; -import {chromium, type Page} from 'playwright-chromium'; +import test from 'ava'; +import {chromium, firefox, webkit, type BrowserType, type Page} from 'playwright'; +import type {ExecutionContext} from 'ava'; type Run = (t: ExecutionContext, page: Page) => Promise; const PWDEBUG = Boolean(process.env['PWDEBUG']); +const DEFAULT_BROWSERS = [chromium, firefox, webkit]; -export const withPage: Implementation = async (t: ExecutionContext, run: Run): Promise => { - const browser = await chromium.launch({ - devtools: PWDEBUG, - }); - const page = await browser.newPage(); - try { - await run(t, page); - } finally { - await browser.close(); +export const browserTest = (title: string, browserTypes: BrowserType[], run: Run) => { + for (const browserType of browserTypes) { + test.serial(`${browserType.name()} - ${title}`, async t => { + const browser = await browserType.launch({devtools: PWDEBUG}); + const page = await browser.newPage(); + try { + await run(t, page); + } finally { + await browser.close(); + } + }); } }; + +export const defaultBrowsersTest = (title: string, run: Run) => { + browserTest(title, DEFAULT_BROWSERS, run); +}; From 9485d30f800ea913b6275b737e4ece39c674b46d Mon Sep 17 00:00:00 2001 From: Kelvin Del Monte Date: Fri, 27 Oct 2023 05:09:25 -0400 Subject: [PATCH 2/3] Fix an issue in firefox where the body option was being detected as an unknown option --- source/core/constants.ts | 17 +++++++++++++++++ source/utils/options.ts | 4 ++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/source/core/constants.ts b/source/core/constants.ts index bbf43106..088ff58d 100644 --- a/source/core/constants.ts +++ b/source/core/constants.ts @@ -58,3 +58,20 @@ export const kyOptionKeys: KyOptionsRegistry = { onDownloadProgress: true, fetch: true, }; + +export const requestOptions = new Set([ + 'options', + 'method', + 'headers', + 'body', + 'mode', + 'credentials', + 'cache', + 'redirect', + 'referrer', + 'referrerPolicy', + 'integrity', + 'keepalive', + 'signal', + 'priority', +]); diff --git a/source/utils/options.ts b/source/utils/options.ts index 22c11d0a..95ae52cc 100644 --- a/source/utils/options.ts +++ b/source/utils/options.ts @@ -1,4 +1,4 @@ -import {kyOptionKeys} from '../core/constants.js'; +import {kyOptionKeys, requestOptions} from '../core/constants.js'; export const findUnknownOptions = ( request: Request, @@ -7,7 +7,7 @@ export const findUnknownOptions = ( const unknownOptions: Record = {}; for (const key in options) { - if (!(key in kyOptionKeys) && !(key in request)) { + if (!requestOptions.has(key) && !(key in kyOptionKeys) && !(key in request)) { unknownOptions[key] = options[key]; } } From 9560921b6bbde0d8cae486ebb9896d5ad4199cd0 Mon Sep 17 00:00:00 2001 From: Kelvin Del Monte Date: Fri, 27 Oct 2023 17:14:11 -0400 Subject: [PATCH 3/3] Replace hardcoded request options list in favor of a registry of options based on Typescript interfaces --- source/core/constants.ts | 34 ++++++++++++++++++---------------- source/types/request.ts | 5 +++++ source/utils/options.ts | 4 ++-- 3 files changed, 25 insertions(+), 18 deletions(-) create mode 100644 source/types/request.ts diff --git a/source/core/constants.ts b/source/core/constants.ts index 088ff58d..0f1fcdbd 100644 --- a/source/core/constants.ts +++ b/source/core/constants.ts @@ -1,5 +1,6 @@ import type {Expect, Equal} from '@type-challenges/utils'; import {type HttpMethod, type KyOptionsRegistry} from '../types/options.js'; +import {type RequestInitRegistry} from '../types/request.js'; export const supportsRequestStreams = (() => { let duplexAccessed = false; @@ -59,19 +60,20 @@ export const kyOptionKeys: KyOptionsRegistry = { fetch: true, }; -export const requestOptions = new Set([ - 'options', - 'method', - 'headers', - 'body', - 'mode', - 'credentials', - 'cache', - 'redirect', - 'referrer', - 'referrerPolicy', - 'integrity', - 'keepalive', - 'signal', - 'priority', -]); +export const requestOptionsRegistry: RequestInitRegistry = { + method: true, + headers: true, + body: true, + mode: true, + credentials: true, + cache: true, + redirect: true, + referrer: true, + referrerPolicy: true, + integrity: true, + keepalive: true, + signal: true, + window: true, + dispatcher: true, + duplex: true, +}; diff --git a/source/types/request.ts b/source/types/request.ts new file mode 100644 index 00000000..15fea38c --- /dev/null +++ b/source/types/request.ts @@ -0,0 +1,5 @@ +import {type RequestInit as UndiciRequestInit} from 'undici-types'; + +type CombinedRequestInit = globalThis.RequestInit & UndiciRequestInit; + +export type RequestInitRegistry = {[K in keyof CombinedRequestInit]-?: true}; diff --git a/source/utils/options.ts b/source/utils/options.ts index 95ae52cc..4968f2bf 100644 --- a/source/utils/options.ts +++ b/source/utils/options.ts @@ -1,4 +1,4 @@ -import {kyOptionKeys, requestOptions} from '../core/constants.js'; +import {kyOptionKeys, requestOptionsRegistry} from '../core/constants.js'; export const findUnknownOptions = ( request: Request, @@ -7,7 +7,7 @@ export const findUnknownOptions = ( const unknownOptions: Record = {}; for (const key in options) { - if (!requestOptions.has(key) && !(key in kyOptionKeys) && !(key in request)) { + if (!(key in requestOptionsRegistry) && !(key in kyOptionKeys) && !(key in request)) { unknownOptions[key] = options[key]; } }