From ca1c50fa867ffde558ccdbf1006e328f57189345 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 17 Jan 2023 12:50:08 +0100 Subject: [PATCH 1/3] feat(scripts-pupeteer): provide launch function as single source of truth for creating new browser instance --- apps/ssr-tests-v9/package.json | 3 +- apps/ssr-tests-v9/src/test.ts | 12 +- apps/ssr-tests-v9/src/utils/launchBrowser.ts | 24 +--- apps/ssr-tests-v9/src/utils/visitPage.test.ts | 34 ----- apps/ssr-tests-v9/src/utils/visitPage.ts | 24 +--- scripts/gulp/src/tasks/browserAdapters.ts | 33 +---- .../projects-test/src/performBrowserTest.ts | 89 +++++++------ scripts/puppeteer/src/index.ts | 2 +- scripts/puppeteer/src/puppeteer.config.ts | 16 --- scripts/puppeteer/src/types.ts | 2 + scripts/puppeteer/src/utils.spec.ts | 122 ++++++++++++++++++ scripts/puppeteer/src/utils.ts | 57 ++++++++ 12 files changed, 253 insertions(+), 165 deletions(-) delete mode 100644 apps/ssr-tests-v9/src/utils/visitPage.test.ts delete mode 100644 scripts/puppeteer/src/puppeteer.config.ts create mode 100644 scripts/puppeteer/src/types.ts create mode 100644 scripts/puppeteer/src/utils.spec.ts create mode 100644 scripts/puppeteer/src/utils.ts diff --git a/apps/ssr-tests-v9/package.json b/apps/ssr-tests-v9/package.json index 2226cf43850a5a..e874ca85b6d38f 100644 --- a/apps/ssr-tests-v9/package.json +++ b/apps/ssr-tests-v9/package.json @@ -25,6 +25,7 @@ "devDependencies": { "@fluentui/eslint-plugin": "*", "@fluentui/scripts-tasks": "*", - "@fluentui/scripts-storybook": "*" + "@fluentui/scripts-storybook": "*", + "@fluentui/scripts-puppeteer": "*" } } diff --git a/apps/ssr-tests-v9/src/test.ts b/apps/ssr-tests-v9/src/test.ts index 7630b40f4f8064..0578e608174da6 100644 --- a/apps/ssr-tests-v9/src/test.ts +++ b/apps/ssr-tests-v9/src/test.ts @@ -15,18 +15,18 @@ async function test(): Promise { const startTime = process.hrtime(); console.log('Starting a browser...'); + const htmlPath = path.resolve(__dirname, '..', 'dist', 'index.html'); + + if (!fs.existsSync(htmlPath)) { + throw new Error('"dist/index.html" does not exist, please run "yarn build" first'); + } + let browser: Browser | undefined; try { browser = await launchBrowser(); console.log('Using', await browser.version()); - const htmlPath = path.resolve(__dirname, '..', 'dist', 'index.html'); - - if (!fs.existsSync(htmlPath)) { - throw new Error('"dist/index.html" does not exist, please run "yarn build" first'); - } - const url = `file://${htmlPath}`; console.log(`Using "${url}"`); diff --git a/apps/ssr-tests-v9/src/utils/launchBrowser.ts b/apps/ssr-tests-v9/src/utils/launchBrowser.ts index a3811e705b3a9d..ef79bf98e682b0 100644 --- a/apps/ssr-tests-v9/src/utils/launchBrowser.ts +++ b/apps/ssr-tests-v9/src/utils/launchBrowser.ts @@ -1,23 +1 @@ -import { Browser, launch } from 'puppeteer'; - -export async function launchBrowser(): Promise { - let browser; - let attempt = 1; - - while (!browser) { - try { - browser = await launch(); - } catch (err) { - if (attempt === 5) { - console.error(`Failed to launch a browser after 5 attempts...`); - throw err; - } - - console.warn('A browser failed to start, retrying...'); - console.warn(err); - attempt++; - } - } - - return browser; -} +export { launch as launchBrowser } from '@fluentui/scripts-puppeteer'; diff --git a/apps/ssr-tests-v9/src/utils/visitPage.test.ts b/apps/ssr-tests-v9/src/utils/visitPage.test.ts deleted file mode 100644 index f598ae9d734670..00000000000000 --- a/apps/ssr-tests-v9/src/utils/visitPage.test.ts +++ /dev/null @@ -1,34 +0,0 @@ -import type { Page } from 'puppeteer'; -import { visitUrl } from './visitPage'; - -// eslint-disable-next-line @typescript-eslint/no-empty-function -const noop = () => {}; - -describe('visitUrl', () => { - it('calls .goto() 5 times before a failure', async () => { - expect.assertions(2); - - jest.spyOn(console, 'warn').mockImplementation(noop); - jest.spyOn(console, 'error').mockImplementation(noop); - - const pageMock: Partial = { - goto: jest.fn().mockImplementation(() => Promise.reject(new Error('page wont open - mock'))), - }; - - await expect(visitUrl(pageMock as Page, 'https://localhost:8080')).rejects.toMatchInlineSnapshot( - `[Error: page wont open - mock]`, - ); - expect(pageMock.goto).toHaveBeenCalledTimes(5); - }); - - it('calls .goto once if successful', async () => { - expect.assertions(2); - - const pageMock: Partial = { - goto: jest.fn(), - }; - - await expect(visitUrl(pageMock as Page, 'https://localhost:8080')).resolves.toBeUndefined(); - expect(pageMock.goto).toHaveBeenCalledTimes(1); - }); -}); diff --git a/apps/ssr-tests-v9/src/utils/visitPage.ts b/apps/ssr-tests-v9/src/utils/visitPage.ts index 0c25a2c24e6c5c..bc69845ac2810b 100644 --- a/apps/ssr-tests-v9/src/utils/visitPage.ts +++ b/apps/ssr-tests-v9/src/utils/visitPage.ts @@ -1,31 +1,11 @@ -import type { Browser, Page } from 'puppeteer'; +import type { Browser } from 'puppeteer'; +import { visitUrl } from '@fluentui/scripts-puppeteer'; import { PROVIDER_ID } from './constants'; class RenderError extends Error { public name = 'RangeError'; } -export async function visitUrl(page: Page, url: string) { - let attempt = 1; - - while (attempt <= 5) { - try { - await page.goto(url, { timeout: 10 * 1000 /* 10 seconds */ }); - break; - } catch (err) { - if (attempt === 5) { - console.error(`Failed to navigate to a page after 5 attempts...`); - throw err; - } - - console.warn('A browser failed to navigate to a page, retrying...'); - console.warn(err); - - attempt++; - } - } -} - export async function visitPage(browser: Browser, url: string) { const page = await browser.newPage(); await page.setRequestInterception(true); diff --git a/scripts/gulp/src/tasks/browserAdapters.ts b/scripts/gulp/src/tasks/browserAdapters.ts index 244209cb82e4c2..58efd6f4f029bc 100644 --- a/scripts/gulp/src/tasks/browserAdapters.ts +++ b/scripts/gulp/src/tasks/browserAdapters.ts @@ -1,9 +1,8 @@ import { spawn, spawnSync } from 'child_process'; -import CDP from 'chrome-remote-interface'; -import puppeteer from 'puppeteer'; import * as net from 'net'; -import { safeLaunchOptions } from '@fluentui/scripts-puppeteer'; +import { launch } from '@fluentui/scripts-puppeteer'; +import CDP from 'chrome-remote-interface'; export type Page = { executeJavaScript: (code: string) => Promise; @@ -17,28 +16,13 @@ export type Browser = { }; export async function createChrome(): Promise { - const options = safeLaunchOptions(); - let browser: puppeteer.Browser | undefined; - let attempt = 1; - while (!browser) { - try { - browser = await puppeteer.launch(options); - } catch (err) { - if (attempt === 5) { - console.error(`Puppeteer failed to launch after 5 attempts`); - throw err; - } - console.warn('Puppeteer failed to launch (will retry):'); - console.warn(err); - attempt++; - } - } + const browser = await launch(); console.log(`Chromium version: ${await browser.version()}`); return { openPage: async url => { - const page = await (browser as puppeteer.Browser).newPage(); + const page = await browser.newPage(); await page.goto(url); @@ -46,10 +30,10 @@ export async function createChrome(): Promise { executeJavaScript: async code => { return page.evaluate(code); }, - close: async () => page.close(), + close: () => page.close(), }; }, - close: async () => (browser as puppeteer.Browser).close(), + close: () => browser.close(), }; } @@ -137,9 +121,6 @@ export async function createElectron(electronPath: string): Promise { }, }; }, - - // FIXME: this async is not necessary - // eslint-disable-next-line @typescript-eslint/no-empty-function - close: async () => {}, + close: () => Promise.resolve(), }; } diff --git a/scripts/projects-test/src/performBrowserTest.ts b/scripts/projects-test/src/performBrowserTest.ts index 568958e2f7cc7d..4c37e4c45e41f8 100644 --- a/scripts/projects-test/src/performBrowserTest.ts +++ b/scripts/projects-test/src/performBrowserTest.ts @@ -1,21 +1,35 @@ import http from 'http'; import { AddressInfo } from 'net'; -import { safeLaunchOptions } from '@fluentui/scripts-puppeteer'; +import { launch, visitUrl } from '@fluentui/scripts-puppeteer'; import express from 'express'; -import puppeteer from 'puppeteer'; +function startServer(rootDirectory: string, listenPort: number, host = 'localhost') { + return new Promise<{ server: http.Server; port: number; url: string }>((resolve, reject) => { + let port: number; + try { + console.log('express: starting server'); -const SERVER_HOST = 'localhost'; + const server = express() + .use(express.static(rootDirectory)) + .listen(listenPort, host, () => { + console.log(`express: server running at http://${host}:${port} from directory "${rootDirectory}"`); + port = listenPort === 0 ? (server.address() as AddressInfo).port : listenPort; + const url = `http://${host}:${port}`; + resolve({ server, port, url }); + }); -function startServer(publicDirectory: string, listenPort: number) { - return new Promise((resolve, reject) => { - try { - const app = express(); - app.use(express.static(publicDirectory)); + server.on('error', err => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore - improper Error type in typings -> https://nodejs.org/api/net.html#serverlisten + if (err.code === 'EADDRINUSE') { + console.error('express: Address in use ...', { port, host }); + } - const server = app.listen(listenPort, SERVER_HOST, () => { - resolve(server); + throw err; + }); + server.on('close', () => { + console.error('express: Terminating server ...', { port, host }); }); } catch (err) { reject(err); @@ -23,32 +37,38 @@ function startServer(publicDirectory: string, listenPort: number) { }); } -export async function performBrowserTest(publicDirectory: string) { - const server = await startServer(publicDirectory, 0); - const { port } = server.address() as AddressInfo; +async function launchServer(root: string) { + /** + * If port is omitted or is 0, the operating system will assign an arbitrary unused port, + * which can be retrieved by using server.address().port after the 'listening' event has been emitted. + * + * @see https://nodejs.org/api/net.html#serverlisten + */ + const PORT = 0; - console.log(`Starting server on port ${port} from directory ${publicDirectory}`); - console.log('Started server. Launching Puppeteer...'); + try { + const { server, ...rest } = await startServer(root, PORT); + /** + * + * .close is asynchronous (not promisified thus we are wrapping it) + * @see https://nodejs.org/api/net.html#serverclosecallback + */ + const closeServer = () => + new Promise((resolve, reject) => server.close(err => (err ? reject(err) : resolve()))); - const options = safeLaunchOptions(); - let browser: puppeteer.Browser | undefined; - let attempt = 1; - while (!browser) { - try { - browser = await puppeteer.launch(options); - console.log('Launched Puppeteer'); - } catch (err) { - if (attempt === 5) { - console.error(`Puppeteer failed to launch after 5 attempts`); - throw err; - } - console.warn('Puppeteer failed to launch (will retry):'); - console.warn(err); - attempt++; - } + return { server, closeServer, ...rest }; + } catch (err) { + console.error('express: start failed!'); + console.error(err); + throw err; } +} +export async function performBrowserTest(publicDirectory: string) { + const { closeServer, url } = await launchServer(publicDirectory); + const browser = await launch(); const page = await browser.newPage(); + let error: Error | undefined; page.on('console', message => { @@ -60,14 +80,11 @@ export async function performBrowserTest(publicDirectory: string) { error = pageError; }); - const url = `http://${SERVER_HOST}:${port}`; - console.log(`Loading ${url} in puppeteer...`); - await page.goto(url); - console.log('Page loaded'); + await visitUrl(page, url); await page.close(); await browser.close(); - await new Promise(resolve => server.close(resolve)); + await closeServer(); if (error) { throw error; diff --git a/scripts/puppeteer/src/index.ts b/scripts/puppeteer/src/index.ts index 994c4f49550d51..ba2e80d12bef27 100644 --- a/scripts/puppeteer/src/index.ts +++ b/scripts/puppeteer/src/index.ts @@ -1 +1 @@ -export { safeLaunchOptions } from './puppeteer.config'; +export { launch, visitUrl } from './utils'; diff --git a/scripts/puppeteer/src/puppeteer.config.ts b/scripts/puppeteer/src/puppeteer.config.ts deleted file mode 100644 index 13a97549eb4df8..00000000000000 --- a/scripts/puppeteer/src/puppeteer.config.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { launch } from 'puppeteer'; - -type LaunchOptions = NonNullable[0]>; - -/** Common set of args to be passed to Chromium. */ -const chromiumArgs = [ - // Workaround for newPage hang in CircleCI: https://github.com/GoogleChrome/puppeteer/issues/1409#issuecomment-453845568 - process.env.TF_BUILD && '--single-process', -].filter(Boolean) as NonNullable; - -export const safeLaunchOptions = (launchOptions?: LaunchOptions): LaunchOptions => { - const mergedChromiumArgs = [...((launchOptions && launchOptions.args) || []), ...chromiumArgs]; - - // eslint-disable-next-line prefer-object-spread - return Object.assign({}, launchOptions, { args: mergedChromiumArgs }); -}; diff --git a/scripts/puppeteer/src/types.ts b/scripts/puppeteer/src/types.ts new file mode 100644 index 00000000000000..0e98f46146b3d0 --- /dev/null +++ b/scripts/puppeteer/src/types.ts @@ -0,0 +1,2 @@ +import * as puppeteer from 'puppeteer'; +export type LaunchOptions = NonNullable[0]>; diff --git a/scripts/puppeteer/src/utils.spec.ts b/scripts/puppeteer/src/utils.spec.ts new file mode 100644 index 00000000000000..b333c18d50b85d --- /dev/null +++ b/scripts/puppeteer/src/utils.spec.ts @@ -0,0 +1,122 @@ +import * as puppeteer from 'puppeteer'; + +import { launch, visitUrl } from './utils'; + +// eslint-disable-next-line @typescript-eslint/no-empty-function +const noop = () => {}; + +describe(`utils`, () => { + let consoleWarnSpy: jest.SpyInstance; + let consoleErrorSpy: jest.SpyInstance; + let consoleLogSpy: jest.SpyInstance; + + beforeEach(() => { + consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(noop); + consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(noop); + consoleLogSpy = jest.spyOn(console, 'log').mockImplementation(noop); + }); + + describe(`#launch`, () => { + function setup() { + const puppeteerLaunchSpy = jest.spyOn(puppeteer, 'launch').mockImplementation(_options => { + return Promise.resolve({} as puppeteer.Browser); + }); + + return { puppeteerLaunchSpy }; + } + + it('should call .launch() 5 times before a failure', async () => { + const { puppeteerLaunchSpy } = setup(); + puppeteerLaunchSpy.mockImplementation(options => { + return Promise.reject(new Error('browser wont launch - mock')); + }); + + await expect(launch()).rejects.toMatchInlineSnapshot(`[Error: browser wont launch - mock]`); + expect(puppeteerLaunchSpy).toHaveBeenCalledTimes(5); + + expect(consoleWarnSpy.mock.calls.flat()).toMatchInlineSnapshot(` + Array [ + "puppeteer: launch failed (will retry 4 more times)", + [Error: browser wont launch - mock], + "puppeteer: launch failed (will retry 3 more times)", + [Error: browser wont launch - mock], + "puppeteer: launch failed (will retry 2 more times)", + [Error: browser wont launch - mock], + "puppeteer: launch failed (will retry 1 more times)", + [Error: browser wont launch - mock], + ] + `); + }); + + it('should call .launch once if successful', async () => { + const { puppeteerLaunchSpy } = setup(); + + await expect(launch()).resolves.not.toBeUndefined(); + expect(puppeteerLaunchSpy).toHaveBeenCalledTimes(1); + + expect(consoleLogSpy.mock.calls.flat()[0]).toEqual( + expect.stringContaining('puppeteer: launching with settings: {'), + ); + expect(consoleLogSpy.mock.calls.flat()[1]).toMatchInlineSnapshot(`"puppeteer: launched..."`); + }); + }); + + describe('#visitUrl', () => { + function setup() { + const pageMock = { + goto: jest.fn(), + }; + + return { pageMock }; + } + + it('should call .goto() 5 times before a failure', async () => { + expect.assertions(4); + + const { pageMock } = setup(); + pageMock.goto.mockImplementation(() => Promise.reject(new Error('page wont open - mock'))); + + await expect( + visitUrl((pageMock as unknown) as puppeteer.Page, 'https://localhost:8080'), + ).rejects.toMatchInlineSnapshot(`[Error: page wont open - mock]`); + + expect(consoleWarnSpy.mock.calls.flat()).toMatchInlineSnapshot(` + Array [ + "puppeteer: failed to navigate to a page (will retry 4 more times)...", + [Error: page wont open - mock], + "puppeteer: failed to navigate to a page (will retry 3 more times)...", + [Error: page wont open - mock], + "puppeteer: failed to navigate to a page (will retry 2 more times)...", + [Error: page wont open - mock], + "puppeteer: failed to navigate to a page (will retry 1 more times)...", + [Error: page wont open - mock], + ] + `); + + expect(consoleErrorSpy.mock.calls.flat()).toMatchInlineSnapshot(` + Array [ + "puppeteer: failed to navigate to a page after 5 attempts...", + ] + `); + expect(pageMock.goto).toHaveBeenCalledTimes(5); + }); + + it('should call .goto once if successful', async () => { + expect.assertions(3); + + const { pageMock } = setup(); + + await expect( + visitUrl((pageMock as unknown) as puppeteer.Page, 'https://localhost:8080'), + ).resolves.toBeUndefined(); + expect(pageMock.goto).toHaveBeenCalledTimes(1); + + expect(consoleLogSpy.mock.calls.flat()).toMatchInlineSnapshot(` + Array [ + "puppeteer: loading url \\"https://localhost:8080\\"...", + "puppeteer: url loaded...", + ] + `); + }); + }); +}); diff --git a/scripts/puppeteer/src/utils.ts b/scripts/puppeteer/src/utils.ts new file mode 100644 index 00000000000000..27d6c40b3b58d4 --- /dev/null +++ b/scripts/puppeteer/src/utils.ts @@ -0,0 +1,57 @@ +import * as puppeteer from 'puppeteer'; + +import type { LaunchOptions } from './types'; + +export async function launch(options: LaunchOptions = {}) { + const maxAttempts = 5; + + let attempt = 1; + + let browser: puppeteer.Browser | undefined; + + console.log(`puppeteer: launching with settings: ${JSON.stringify(options)}`); + + while (!browser) { + try { + browser = await puppeteer.launch(options); + console.log('puppeteer: launched...'); + } catch (err) { + if (attempt === maxAttempts) { + console.error(`puppeteer: launch failed 5 attempts`); + throw err; + } + console.warn(`puppeteer: launch failed (will retry ${maxAttempts - attempt} more times)`); + console.warn(err); + + attempt++; + } + } + + return browser; +} + +export async function visitUrl(page: puppeteer.Page, url: string) { + const TEN_SECONDS = 10 * 1000; + const maxAttempts = 5; + let attempt = 1; + + console.log(`puppeteer: loading url "${url}"...`); + + while (attempt <= 5) { + try { + await page.goto(url, { timeout: TEN_SECONDS }); + console.log(`puppeteer: url loaded...`); + break; + } catch (err) { + if (attempt === maxAttempts) { + console.error(`puppeteer: failed to navigate to a page after 5 attempts...`); + throw err; + } + + console.warn(`puppeteer: failed to navigate to a page (will retry ${maxAttempts - attempt} more times)...`); + console.warn(err); + + attempt++; + } + } +} From 6de22841cf34649332ee871c435e6a4ce9eefc30 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 17 Jan 2023 19:14:18 +0100 Subject: [PATCH 2/3] chore: bump puppeteer to v17 --- package.json | 2 +- yarn.lock | 43 +++++++++++++++++++++---------------------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/package.json b/package.json index a4e104cba77afc..dbb38900c44dcf 100644 --- a/package.json +++ b/package.json @@ -291,7 +291,7 @@ "pretty-bytes": "5.6.0", "progress": "2.0.3", "prompts": "2.4.2", - "puppeteer": "14.4.0", + "puppeteer": "17.1.3", "raw-loader": "4.0.2", "react": "17.0.2", "react-app-polyfill": "2.0.0", diff --git a/yarn.lock b/yarn.lock index 9840936b180bfb..e3b0ac05e263ad 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10877,10 +10877,10 @@ detect-port@^1.3.0: address "^1.0.1" debug "^2.6.0" -devtools-protocol@0.0.1001819: - version "0.0.1001819" - resolved "https://registry.yarnpkg.com/devtools-protocol/-/devtools-protocol-0.0.1001819.tgz#0a98f44cefdb02cc684f3d5e6bd898a1690231d9" - integrity sha512-G6OsIFnv/rDyxSqBa2lDLR6thp9oJioLsb2Gl+LbQlyoA9/OBAkrTU9jiCcQ8Pnh7z4d6slDiLaogR5hzgJLmQ== +devtools-protocol@0.0.1036444: + version "0.0.1036444" + resolved "https://registry.yarnpkg.com/devtools-protocol/-/devtools-protocol-0.0.1036444.tgz#a570d3cdde61527c82f9b03919847b8ac7b1c2b9" + integrity sha512-0y4f/T8H9lsESV9kKP1HDUXgHxCdniFeJh6Erq+FbdOEvp/Ydp9t8kcAAM5gOd17pMrTDlFWntoHtzzeTUWKNw== devtools-protocol@0.0.894172: version "0.0.894172" @@ -20586,13 +20586,6 @@ pixel-buffer-diff@1.3.3: resolved "https://registry.yarnpkg.com/pixel-buffer-diff/-/pixel-buffer-diff-1.3.3.tgz#081965c3711663db4c749281c79d0caf461b4cbf" integrity sha512-Idq8Wps2P5iKgvP3DEb0aZuKqqDuqsiqY0jJmqHMYfCzq4xuNPygFg1zdZSM5k5m14u8Ww72mTY39Bu2XtT2+A== -pkg-dir@4.2.0, pkg-dir@^4.1.0, pkg-dir@^4.2.0: - version "4.2.0" - resolved "https://registry.yarnpkg.com/pkg-dir/-/pkg-dir-4.2.0.tgz#f099133df7ede422e81d1d8448270eeb3e4261f3" - integrity sha512-HRDzbaKjC+AOWVXxAU/x54COGeIv9eb+6CkDSQoNTt4XyWoIJvuPsXizxu/Fr23EiekbtZwmh1IcIG/l/a10GQ== - dependencies: - find-up "^4.0.0" - pkg-dir@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/pkg-dir/-/pkg-dir-3.0.0.tgz#2749020f239ed990881b1f71210d51eb6523bea3" @@ -20600,6 +20593,13 @@ pkg-dir@^3.0.0: dependencies: find-up "^3.0.0" +pkg-dir@^4.1.0, pkg-dir@^4.2.0: + version "4.2.0" + resolved "https://registry.yarnpkg.com/pkg-dir/-/pkg-dir-4.2.0.tgz#f099133df7ede422e81d1d8448270eeb3e4261f3" + integrity sha512-HRDzbaKjC+AOWVXxAU/x54COGeIv9eb+6CkDSQoNTt4XyWoIJvuPsXizxu/Fr23EiekbtZwmh1IcIG/l/a10GQ== + dependencies: + find-up "^4.0.0" + pkg-dir@^5.0.0: version "5.0.0" resolved "https://registry.yarnpkg.com/pkg-dir/-/pkg-dir-5.0.0.tgz#a02d6aebe6ba133a928f74aec20bafdfe6b8e760" @@ -21137,23 +21137,22 @@ punycode@^2.1.0, punycode@^2.1.1: resolved "https://registry.yarnpkg.com/punycode/-/punycode-2.1.1.tgz#b58b010ac40c22c5657616c8d2c2c02c7bf479ec" integrity sha512-XRsRjdf+j5ml+y/6GKHPZbrF/8p2Yga0JPtdqTIY2Xe5ohJPD9saDJJLPvp9+NSBprVvevdXZybnj2cv8OEd0A== -puppeteer@14.4.0: - version "14.4.0" - resolved "https://registry.yarnpkg.com/puppeteer/-/puppeteer-14.4.0.tgz#e711789814f785484ff5906d464a32a8146a1304" - integrity sha512-hAXoJX7IAmnRBwf4VrowoRdrS8hqWZsGuQ1Dg5R0AwDK5juaxnNO/obySo9+ytyF7pp9/VsmIA9yFE1GLSouCQ== +puppeteer@17.1.3: + version "17.1.3" + resolved "https://registry.yarnpkg.com/puppeteer/-/puppeteer-17.1.3.tgz#2814cf221925e19c681c69aa97401a68b30240c9" + integrity sha512-tVtvNSOOqlq75rUgwLeDAEQoLIiBqmRg0/zedpI6fuqIocIkuxG23A7FIl1oVSkuSMMLgcOP5kVhNETmsmjvPw== dependencies: cross-fetch "3.1.5" debug "4.3.4" - devtools-protocol "0.0.1001819" + devtools-protocol "0.0.1036444" extract-zip "2.0.1" https-proxy-agent "5.0.1" - pkg-dir "4.2.0" progress "2.0.3" proxy-from-env "1.1.0" rimraf "3.0.2" tar-fs "2.1.1" unbzip2-stream "1.4.3" - ws "8.7.0" + ws "8.8.1" puppeteer@^1.13.0: version "1.17.0" @@ -26356,10 +26355,10 @@ write-pkg@^4.0.0: type-fest "^0.4.1" write-json-file "^3.2.0" -ws@8.7.0, ws@>=8.7.0, ws@^8.2.3, ws@^8.4.2: - version "8.7.0" - resolved "https://registry.yarnpkg.com/ws/-/ws-8.7.0.tgz#eaf9d874b433aa00c0e0d8752532444875db3957" - integrity sha512-c2gsP0PRwcLFzUiA8Mkr37/MI7ilIlHQxaEAtd0uNMbVMoy8puJyafRlm0bV9MbGSabUPeLrRRaqIBcFcA2Pqg== +ws@8.8.1, ws@>=8.7.0, ws@^8.2.3, ws@^8.4.2: + version "8.8.1" + resolved "https://registry.yarnpkg.com/ws/-/ws-8.8.1.tgz#5dbad0feb7ade8ecc99b830c1d77c913d4955ff0" + integrity sha512-bGy2JzvzkPowEJV++hF07hAD6niYSr0JzBNo/J29WsB57A2r7Wlc1UFcTR9IzrPvuNVO4B8LGqF8qcpsVOhJCA== ws@^6.1.0: version "6.2.1" From 31bbf9cd21e62939449b22c6a3bd713af622ceea Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Wed, 18 Jan 2023 12:26:59 +0100 Subject: [PATCH 3/3] refactor(scripts): make startServer generic and used everywhere --- scripts/gulp/package.json | 3 +- scripts/gulp/src/tasks/serve.ts | 53 +++++--------- scripts/projects-test/src/index.ts | 9 +-- .../projects-test/src/performBrowserTest.ts | 72 ++++++++++++------- 4 files changed, 70 insertions(+), 67 deletions(-) diff --git a/scripts/gulp/package.json b/scripts/gulp/package.json index 91fa3c70f31940..036d7e9e1587d7 100644 --- a/scripts/gulp/package.json +++ b/scripts/gulp/package.json @@ -15,6 +15,7 @@ "@fluentui/scripts-utils": "*", "@fluentui/scripts-prettier": "*", "@fluentui/scripts-puppeteer": "*", - "@fluentui/scripts-babel": "*" + "@fluentui/scripts-babel": "*", + "@fluentui/scripts-projects-test": "*" } } diff --git a/scripts/gulp/src/tasks/serve.ts b/scripts/gulp/src/tasks/serve.ts index df5aab4e21f914..cb2cc03fa28c43 100644 --- a/scripts/gulp/src/tasks/serve.ts +++ b/scripts/gulp/src/tasks/serve.ts @@ -1,44 +1,27 @@ -import express from 'express'; +import type { Server } from 'http'; +import { closeServer, startServer } from '@fluentui/scripts-projects-test'; import historyApiFallback from 'connect-history-api-fallback'; -import { Server } from 'http'; -import { colors, log } from 'gulp-util'; +import type { Express } from 'express'; -type Express = ReturnType; - -export const serve = ( +export const serve = async ( directoryPath: string, host: string, port: number, - configureMiddleware: (express: Express) => Express = app => app, + configureMiddleware = (app: Express) => app, ): Promise => { - return new Promise((resolve, reject) => { - try { - const server = configureMiddleware( - express().use( - historyApiFallback({ - verbose: false, - }), - ), - ) - .use(express.static(directoryPath)) - .listen(port, host, () => { - log(colors.yellow(`Server running at http://${host}:${port}`)); - resolve(server); - }); - } catch (err) { - reject(err); - } - }); -}; + const middleware = (app: Express) => { + return configureMiddleware( + app.use( + historyApiFallback({ + verbose: false, + }), + ), + ); + }; -export const forceClose = (server: Server): Promise => { - if (!server) { - return Promise.resolve(); - } - - return new Promise((resolve, reject) => { - server.keepAliveTimeout = 1000; - server.close(err => (err ? reject(err) : resolve())); - }); + const { server } = await startServer({ root: directoryPath, host, port }, middleware); + return server; }; + +export const forceClose = closeServer; diff --git a/scripts/projects-test/src/index.ts b/scripts/projects-test/src/index.ts index da237d82dfbe42..d843e48affed8e 100644 --- a/scripts/projects-test/src/index.ts +++ b/scripts/projects-test/src/index.ts @@ -1,4 +1,5 @@ -export * from './createReactApp'; -export * from './packPackages'; -export * from './performBrowserTest'; -export * from './utils'; +export { prepareCreateReactApp } from './createReactApp'; +export { addResolutionPathsForProjectPackages, packProjectPackages } from './packPackages'; +export { performBrowserTest, startServer, closeServer } from './performBrowserTest'; +export { createTempDir, generateFiles, log, prepareTempDirs, shEcho, workspaceRoot } from './utils'; +export type { TempPaths } from './utils'; diff --git a/scripts/projects-test/src/performBrowserTest.ts b/scripts/projects-test/src/performBrowserTest.ts index 4c37e4c45e41f8..7b87b29f900e01 100644 --- a/scripts/projects-test/src/performBrowserTest.ts +++ b/scripts/projects-test/src/performBrowserTest.ts @@ -1,35 +1,60 @@ -import http from 'http'; +import { Server } from 'http'; import { AddressInfo } from 'net'; import { launch, visitUrl } from '@fluentui/scripts-puppeteer'; -import express from 'express'; +import express, { Express } from 'express'; -function startServer(rootDirectory: string, listenPort: number, host = 'localhost') { - return new Promise<{ server: http.Server; port: number; url: string }>((resolve, reject) => { - let port: number; +/** + * + * .close is asynchronous (not promisified thus we are wrapping it) + * @see https://nodejs.org/api/net.html#serverclosecallback + */ +export function closeServer(server: Server): Promise { + if (!server) { + return Promise.resolve(); + } + + return new Promise((resolve, reject) => { + // TODO: Default is 5 seconds. why is this set to 1 ? + // https://nodejs.org/api/http.html#serverkeepalivetimeout + server.keepAliveTimeout = 1000; + server.close(err => (err ? reject(err) : resolve())); + }); +} + +export function startServer( + options: { root: string; port: number; host?: string }, + configureMiddleware = (app: Express) => app, +) { + const { root, port, host = 'localhost' } = options; + return new Promise<{ server: Server; port: number; url: string }>((resolve, reject) => { + let usedPort: number; try { console.log('express: starting server'); - const server = express() - .use(express.static(rootDirectory)) - .listen(listenPort, host, () => { - console.log(`express: server running at http://${host}:${port} from directory "${rootDirectory}"`); - port = listenPort === 0 ? (server.address() as AddressInfo).port : listenPort; - const url = `http://${host}:${port}`; - resolve({ server, port, url }); - }); + const middleware = (app: Express) => app.use(express.static(root)); + const app = middleware(configureMiddleware(express())); + const server = app.listen(port, host, () => { + const shouldAssignArbitraryUnusedPort = port === 0; + usedPort = shouldAssignArbitraryUnusedPort ? (server.address() as AddressInfo).port : port; + const url = `http://${host}:${usedPort}`; + + console.log(`express: server running at http://${host}:${usedPort} from directory "${root}"`); + + resolve({ server, port: usedPort, url }); + }); server.on('error', err => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore - improper Error type in typings -> https://nodejs.org/api/net.html#serverlisten if (err.code === 'EADDRINUSE') { - console.error('express: Address in use ...', { port, host }); + console.error('express: Address in use ...', { port: usedPort, host }); } throw err; }); server.on('close', () => { - console.error('express: Terminating server ...', { port, host }); + console.error('express: Terminating server ...', { port: usedPort, host }); }); } catch (err) { reject(err); @@ -47,16 +72,9 @@ async function launchServer(root: string) { const PORT = 0; try { - const { server, ...rest } = await startServer(root, PORT); - /** - * - * .close is asynchronous (not promisified thus we are wrapping it) - * @see https://nodejs.org/api/net.html#serverclosecallback - */ - const closeServer = () => - new Promise((resolve, reject) => server.close(err => (err ? reject(err) : resolve()))); - - return { server, closeServer, ...rest }; + const api = await startServer({ root, port: PORT }); + + return api; } catch (err) { console.error('express: start failed!'); console.error(err); @@ -65,7 +83,7 @@ async function launchServer(root: string) { } export async function performBrowserTest(publicDirectory: string) { - const { closeServer, url } = await launchServer(publicDirectory); + const { server, url } = await launchServer(publicDirectory); const browser = await launch(); const page = await browser.newPage(); @@ -84,7 +102,7 @@ export async function performBrowserTest(publicDirectory: string) { await page.close(); await browser.close(); - await closeServer(); + await closeServer(server); if (error) { throw error;