From 91012833c613e7e8e0f75bb418391d572c297603 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 5 Sep 2024 02:22:27 -0700 Subject: [PATCH] chore: move 'dev-server' extensibility point to plugin (#32448) Instead of plumbing it through a custom unspecified config field, make it a part of plugin interface. Additionally, use task runner for starting/stopping dev server. --- packages/playwright-ct-core/index.js | 3 +- .../playwright-ct-core/src/cliOverrides.ts | 6 - packages/playwright-ct-core/src/vitePlugin.ts | 5 + packages/playwright/src/plugins/index.ts | 2 + packages/playwright/src/program.ts | 13 +- packages/playwright/src/runner/reporters.ts | 10 +- packages/playwright/src/runner/runner.ts | 15 +- packages/playwright/src/runner/tasks.ts | 38 +++++ packages/playwright/src/runner/testServer.ts | 55 +++--- .../test-server-connection.spec.ts | 97 ----------- tests/playwright-test/test-server.spec.ts | 158 ++++++++++++++++++ 11 files changed, 253 insertions(+), 149 deletions(-) delete mode 100644 tests/playwright-test/test-server-connection.spec.ts create mode 100644 tests/playwright-test/test-server.spec.ts diff --git a/packages/playwright-ct-core/index.js b/packages/playwright-ct-core/index.js index cda6e7ad557e9..9583273ac2eed 100644 --- a/packages/playwright-ct-core/index.js +++ b/packages/playwright-ct-core/index.js @@ -16,7 +16,7 @@ const { test: baseTest, expect, devices, defineConfig: originalDefineConfig } = require('playwright/test'); const { fixtures } = require('./lib/mount'); -const { clearCacheCommand, runDevServerCommand, findRelatedTestFilesCommand } = require('./lib/cliOverrides'); +const { clearCacheCommand, findRelatedTestFilesCommand } = require('./lib/cliOverrides'); const { createPlugin } = require('./lib/vitePlugin'); const defineConfig = (...configs) => { @@ -31,7 +31,6 @@ const defineConfig = (...configs) => { ], cli: { 'clear-cache': clearCacheCommand, - 'dev-server': runDevServerCommand, 'find-related-test-files': findRelatedTestFilesCommand, }, } diff --git a/packages/playwright-ct-core/src/cliOverrides.ts b/packages/playwright-ct-core/src/cliOverrides.ts index 0e4c328694c43..1b069a7177d62 100644 --- a/packages/playwright-ct-core/src/cliOverrides.ts +++ b/packages/playwright-ct-core/src/cliOverrides.ts @@ -18,10 +18,8 @@ import { affectedTestFiles, cacheDir } from 'playwright/lib/transform/compilationCache'; import { buildBundle } from './vitePlugin'; import { resolveDirs } from './viteUtils'; -import { runDevServer } from './devServer'; import type { FullConfigInternal } from 'playwright/lib/common/config'; import { removeFolderAndLogToConsole } from 'playwright/lib/runner/testServer'; -import type { FullConfig } from 'playwright/types/test'; export async function clearCacheCommand(config: FullConfigInternal) { const dirs = await resolveDirs(config.configDir, config.config); @@ -34,7 +32,3 @@ export async function findRelatedTestFilesCommand(files: string[], config: Full await buildBundle(config.config, config.configDir); return { testFiles: affectedTestFiles(files) }; } - -export async function runDevServerCommand(config: FullConfig) { - return await runDevServer(config); -} diff --git a/packages/playwright-ct-core/src/vitePlugin.ts b/packages/playwright-ct-core/src/vitePlugin.ts index 80f4d21774a63..f954fcd2b2356 100644 --- a/packages/playwright-ct-core/src/vitePlugin.ts +++ b/packages/playwright-ct-core/src/vitePlugin.ts @@ -31,6 +31,7 @@ import type { ImportInfo } from './tsxTransform'; import type { ComponentRegistry } from './viteUtils'; import { createConfig, frameworkConfig, hasJSComponents, populateComponentsFromTests, resolveDirs, resolveEndpoint, transformIndexFile } from './viteUtils'; import { resolveHook } from 'playwright/lib/transform/transform'; +import { runDevServer } from './devServer'; const log = debug('pw:vite'); @@ -73,6 +74,10 @@ export function createPlugin(): TestRunnerPlugin { populateDependencies: async () => { await buildBundle(config, configDir); }, + + startDevServer: async () => { + return await runDevServer(config); + }, }; } diff --git a/packages/playwright/src/plugins/index.ts b/packages/playwright/src/plugins/index.ts index 3502d90966313..c3b6f52bf4e4d 100644 --- a/packages/playwright/src/plugins/index.ts +++ b/packages/playwright/src/plugins/index.ts @@ -21,6 +21,7 @@ export interface TestRunnerPlugin { name: string; setup?(config: FullConfig, configDir: string, reporter: ReporterV2): Promise; populateDependencies?(): Promise; + startDevServer?(): Promise<() => Promise>; begin?(suite: Suite): Promise; end?(): Promise; teardown?(): Promise; @@ -29,6 +30,7 @@ export interface TestRunnerPlugin { export type TestRunnerPluginRegistration = { factory: TestRunnerPlugin | (() => TestRunnerPlugin | Promise); instance?: TestRunnerPlugin; + devServerCleanup?: any; }; export { webServer } from './webServerPlugin'; diff --git a/packages/playwright/src/program.ts b/packages/playwright/src/program.ts index c9b5a885c30e5..5d1f687e1e1fa 100644 --- a/packages/playwright/src/program.ts +++ b/packages/playwright/src/program.ts @@ -98,15 +98,10 @@ function addDevServerCommand(program: Command) { const config = await loadConfigFromFileRestartIfNeeded(options.config); if (!config) return; - const implementation = (config.config as any)['@playwright/test']?.['cli']?.['dev-server']; - if (implementation) { - const runner = new Runner(config); - await runner.loadAllTests(); - await implementation(config.config); - } else { - console.log(`DevServer is not available in the package you are using. Did you mean to use component testing?`); - gracefullyProcessExitDoNotHang(1); - } + const runner = new Runner(config); + const { status } = await runner.runDevServer(); + const exitCode = status === 'interrupted' ? 130 : (status === 'passed' ? 0 : 1); + gracefullyProcessExitDoNotHang(exitCode); }); } diff --git a/packages/playwright/src/runner/reporters.ts b/packages/playwright/src/runner/reporters.ts index 1748292cc0767..5b7d3a83cbbb5 100644 --- a/packages/playwright/src/runner/reporters.ts +++ b/packages/playwright/src/runner/reporters.ts @@ -16,7 +16,7 @@ import path from 'path'; import type { FullConfig, TestError } from '../../types/testReporter'; -import { formatError } from '../reporters/base'; +import { colors, formatError } from '../reporters/base'; import DotReporter from '../reporters/dot'; import EmptyReporter from '../reporters/empty'; import GitHubReporter from '../reporters/github'; @@ -86,6 +86,14 @@ export async function createReporterForTestServer(file: string, messageSink: (me })); } +export function createConsoleReporter() { + return wrapReporterAsV2({ + onError(error: TestError) { + process.stdout.write(formatError(error, colors.enabled).message + '\n'); + } + }); +} + function reporterOptions(config: FullConfigInternal, mode: 'list' | 'test' | 'merge', isTestServer: boolean) { return { configDir: config.configDir, diff --git a/packages/playwright/src/runner/runner.ts b/packages/playwright/src/runner/runner.ts index c37964ce2d54c..2b762f9350619 100644 --- a/packages/playwright/src/runner/runner.ts +++ b/packages/playwright/src/runner/runner.ts @@ -21,8 +21,8 @@ import { monotonicTime } from 'playwright-core/lib/utils'; import type { FullResult, TestError } from '../../types/testReporter'; import { webServerPluginsForConfig } from '../plugins/webServerPlugin'; import { collectFilesForProject, filterProjects } from './projectUtils'; -import { createReporters } from './reporters'; -import { TestRun, createTaskRunner, createTaskRunnerForList } from './tasks'; +import { createConsoleReporter, createReporters } from './reporters'; +import { TestRun, createTaskRunner, createTaskRunnerForDevServer, createTaskRunnerForList } from './tasks'; import type { FullConfigInternal } from '../common/config'; import type { Suite } from '../common/test'; import { wrapReporterAsV2 } from '../reporters/reporterV2'; @@ -143,6 +143,17 @@ export class Runner { return await override(resolvedFiles, this._config); return { testFiles: affectedTestFiles(resolvedFiles) }; } + + async runDevServer() { + const reporter = new InternalReporter([createConsoleReporter()]); + const taskRunner = createTaskRunnerForDevServer(this._config, reporter, 'in-process', true); + const testRun = new TestRun(this._config); + reporter.onConfigure(this._config.config); + const status = await taskRunner.run(testRun, 0); + await reporter.onEnd({ status }); + await reporter.onExit(); + return { status }; + } } export type LastRunInfo = { diff --git a/packages/playwright/src/runner/tasks.ts b/packages/playwright/src/runner/tasks.ts index 8a74c0337ab62..b90b02904f99a 100644 --- a/packages/playwright/src/runner/tasks.ts +++ b/packages/playwright/src/runner/tasks.ts @@ -113,6 +113,22 @@ export function createTaskRunnerForListFiles(config: FullConfigInternal, reporte return taskRunner; } +export function createTaskRunnerForDevServer(config: FullConfigInternal, reporter: InternalReporter, mode: 'in-process' | 'out-of-process', setupAndWait: boolean): TaskRunner { + const taskRunner = TaskRunner.create(reporter, config.config.globalTimeout); + if (setupAndWait) { + for (const plugin of config.plugins) + taskRunner.addTask('plugin setup', createPluginSetupTask(plugin)); + } + taskRunner.addTask('load tests', createLoadTask(mode, { failOnLoadErrors: true, filterOnly: false })); + taskRunner.addTask('start dev server', createStartDevServerTask()); + if (setupAndWait) { + taskRunner.addTask('wait until interrupted', { + setup: async () => new Promise(() => {}), + }); + } + return taskRunner; +} + function createReportBeginTask(): Task { return { setup: async (reporter, { rootSuite }) => { @@ -349,3 +365,25 @@ function createRunTestsTask(): Task { }, }; } + +function createStartDevServerTask(): Task { + return { + setup: async (reporter, testRun, errors, softErrors) => { + if (testRun.config.plugins.some(plugin => !!plugin.devServerCleanup)) { + errors.push({ message: `DevServer is already running` }); + return; + } + for (const plugin of testRun.config.plugins) + plugin.devServerCleanup = await plugin.instance?.startDevServer?.(); + if (!testRun.config.plugins.some(plugin => !!plugin.devServerCleanup)) + errors.push({ message: `DevServer is not available in the package you are using. Did you mean to use component testing?` }); + }, + + teardown: async (reporter, testRun) => { + for (const plugin of testRun.config.plugins) { + await plugin.devServerCleanup?.(); + plugin.devServerCleanup = undefined; + } + }, + }; +} diff --git a/packages/playwright/src/runner/testServer.ts b/packages/playwright/src/runner/testServer.ts index d0370fcfc370b..f0444cbc4305b 100644 --- a/packages/playwright/src/runner/testServer.ts +++ b/packages/playwright/src/runner/testServer.ts @@ -23,7 +23,7 @@ import type * as reporterTypes from '../../types/testReporter'; import { collectAffectedTestFiles, dependenciesForTestFile } from '../transform/compilationCache'; import type { ConfigLocation, FullConfigInternal } from '../common/config'; import { createReporterForTestServer, createReporters } from './reporters'; -import { TestRun, createTaskRunnerForList, createTaskRunnerForTestServer, createTaskRunnerForWatchSetup, createTaskRunnerForListFiles } from './tasks'; +import { TestRun, createTaskRunnerForList, createTaskRunnerForTestServer, createTaskRunnerForWatchSetup, createTaskRunnerForListFiles, createTaskRunnerForDevServer } from './tasks'; import { open } from 'playwright-core/lib/utilsBundle'; import ListReporter from '../reporters/list'; import { SigIntWatcher } from './sigIntWatcher'; @@ -75,12 +75,12 @@ export class TestServerDispatcher implements TestServerInterface { readonly transport: Transport; private _queue = Promise.resolve(); private _globalSetup: { cleanup: () => Promise, report: ReportEntry[] } | undefined; + private _devServer: { cleanup: () => Promise, report: ReportEntry[] } | undefined; readonly _dispatchEvent: TestServerInterfaceEventEmitters['dispatchEvent']; private _plugins: TestRunnerPluginRegistration[] | undefined; private _serializer = require.resolve('./uiModeReporter'); private _watchTestDirs = false; private _closeOnDisconnect = false; - private _devServerHandle: (() => Promise) | undefined; constructor(configLocation: ConfigLocation) { this._configLocation = configLocation; @@ -174,41 +174,32 @@ export class TestServerDispatcher implements TestServerInterface { } async startDevServer(params: Parameters[0]): ReturnType { - if (this._devServerHandle) - return { status: 'failed', report: [] }; - const { config, report, reporter, status } = await this._innerListTests({}); + await this.stopDevServer({}); + + const { reporter, report } = await this._collectingInternalReporter(); + const config = await this._loadConfigOrReportError(reporter); if (!config) - return { status, report }; - const devServerCommand = (config.config as any)['@playwright/test']?.['cli']?.['dev-server']; - if (!devServerCommand) { - reporter.onError({ message: 'No dev-server command found in the configuration' }); - return { status: 'failed', report }; - } - try { - this._devServerHandle = await devServerCommand(config.config); - return { status: 'passed', report }; - } catch (e) { - reporter.onError(serializeError(e)); - return { status: 'failed', report }; + return { report, status: 'failed' }; + + const taskRunner = createTaskRunnerForDevServer(config, reporter, 'out-of-process', false); + const testRun = new TestRun(config); + reporter.onConfigure(config.config); + const { status, cleanup } = await taskRunner.runDeferCleanup(testRun, 0); + await reporter.onEnd({ status }); + await reporter.onExit(); + if (status !== 'passed') { + await cleanup(); + return { report, status }; } + this._devServer = { cleanup, report }; + return { report, status }; } async stopDevServer(params: Parameters[0]): ReturnType { - if (!this._devServerHandle) - return { status: 'failed', report: [] }; - try { - await this._devServerHandle(); - this._devServerHandle = undefined; - return { status: 'passed', report: [] }; - } catch (e) { - const { reporter, report } = await this._collectingInternalReporter(); - // Produce dummy config when it has an error. - reporter.onConfigure(baseFullConfig); - reporter.onError(serializeError(e)); - await reporter.onEnd({ status: 'failed' }); - await reporter.onExit(); - return { status: 'failed', report }; - } + const devServer = this._devServer; + const status = await devServer?.cleanup(); + this._devServer = undefined; + return { status, report: devServer?.report || [] }; } async clearCache(params: Parameters[0]): ReturnType { diff --git a/tests/playwright-test/test-server-connection.spec.ts b/tests/playwright-test/test-server-connection.spec.ts deleted file mode 100644 index aef2b63483331..0000000000000 --- a/tests/playwright-test/test-server-connection.spec.ts +++ /dev/null @@ -1,97 +0,0 @@ -/** - * Copyright (c) Microsoft Corporation. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import { test as baseTest, expect } from './ui-mode-fixtures'; - -import { TestServerConnection, WebSocketTestServerTransport } from '../../packages/playwright/lib/isomorphic/testServerConnection'; - -class TestServerConnectionUnderTest extends TestServerConnection { - events: [string, any][] = []; - - constructor(wsUrl: string) { - super(new WebSocketTestServerTransport(wsUrl)); - this.onTestFilesChanged(params => this.events.push(['testFilesChanged', params])); - this.onStdio(params => this.events.push(['stdio', params])); - this.onLoadTraceRequested(params => this.events.push(['loadTraceRequested', params])); - } -} - -const test = baseTest.extend<{ testServerConnection: TestServerConnectionUnderTest }>({ - testServerConnection: async ({ startCLICommand }, use, testInfo) => { - testInfo.skip(!globalThis.WebSocket, 'WebSocket not available prior to Node 22.4.0'); - - const testServerProcess = await startCLICommand({}, 'test-server'); - - await testServerProcess.waitForOutput('Listening on'); - const line = testServerProcess.output.split('\n').find(l => l.includes('Listening on')); - const wsEndpoint = line!.split(' ')[2]; - - await use(new TestServerConnectionUnderTest(wsEndpoint)); - - await testServerProcess.kill(); - } -}); - -test('file watching', async ({ testServerConnection, writeFiles }, testInfo) => { - await writeFiles({ - 'utils.ts': ` - export const expected = 42; - `, - 'a.test.ts': ` - import { test } from '@playwright/test'; - import { expected } from "./utils"; - test('foo', () => { - expect(123).toBe(expected); - }); - `, - }); - - const tests = await testServerConnection.listTests({}); - expect(tests.report.map(e => e.method)).toEqual(['onConfigure', 'onProject', 'onBegin', 'onEnd']); - - await testServerConnection.watch({ fileNames: [testInfo.outputPath('a.test.ts')] }); - - await writeFiles({ - 'utils.ts': ` - export const expected = 123; - `, - }); - - await expect.poll(() => testServerConnection.events).toHaveLength(1); - expect(testServerConnection.events).toEqual([ - ['testFilesChanged', { testFiles: [testInfo.outputPath('a.test.ts')] }] - ]); -}); - -test('stdio interception', async ({ testServerConnection, writeFiles }) => { - await testServerConnection.initialize({ interceptStdio: true }); - await writeFiles({ - 'a.test.ts': ` - import { test, expect } from '@playwright/test'; - test('foo', () => { - console.log("this goes to stdout"); - console.error("this goes to stderr"); - expect(true).toBe(true); - }); - `, - }); - - const tests = await testServerConnection.runTests({ trace: 'on' }); - expect(tests).toEqual({ status: 'passed' }); - await expect.poll(() => testServerConnection.events).toEqual(expect.arrayContaining([ - ['stdio', { type: 'stderr', text: 'this goes to stderr\n' }], - ['stdio', { type: 'stdout', text: 'this goes to stdout\n' }] - ])); -}); \ No newline at end of file diff --git a/tests/playwright-test/test-server.spec.ts b/tests/playwright-test/test-server.spec.ts new file mode 100644 index 0000000000000..def1ed4ed1d98 --- /dev/null +++ b/tests/playwright-test/test-server.spec.ts @@ -0,0 +1,158 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { test as baseTest, expect } from './ui-mode-fixtures'; +import { TestServerConnection } from '../../packages/playwright/lib/isomorphic/testServerConnection'; +import { playwrightCtConfigText } from './playwright-test-fixtures'; +import ws from 'ws'; +import type { TestChildProcess } from 'tests/config/commonFixtures'; + +class WSTransport { + private _ws: ws.WebSocket; + constructor(url: string) { + this._ws = new ws.WebSocket(url); + } + onmessage(listener: (message: string) => void) { + this._ws.addEventListener('message', event => listener(event.data.toString())); + } + onopen(listener: () => void) { + this._ws.addEventListener('open', listener); + } + onerror(listener: () => void) { + this._ws.addEventListener('error', listener); + } + onclose(listener: () => void) { + this._ws.addEventListener('close', listener); + } + send(data: string) { + this._ws.send(data); + } + close() { + this._ws.close(); + } +} + +class TestServerConnectionUnderTest extends TestServerConnection { + events: [string, any][] = []; + + constructor(wsUrl: string) { + super(new WSTransport(wsUrl)); + this.onTestFilesChanged(params => this.events.push(['testFilesChanged', params])); + this.onStdio(params => this.events.push(['stdio', params])); + this.onLoadTraceRequested(params => this.events.push(['loadTraceRequested', params])); + } +} + +const test = baseTest.extend<{ startTestServer: () => Promise }>({ + startTestServer: async ({ startCLICommand }, use, testInfo) => { + let testServerProcess: TestChildProcess | undefined; + await use(async () => { + testServerProcess = await startCLICommand({}, 'test-server'); + await testServerProcess.waitForOutput('Listening on'); + const line = testServerProcess.output.split('\n').find(l => l.includes('Listening on')); + const wsEndpoint = line!.split(' ')[2]; + return new TestServerConnectionUnderTest(wsEndpoint); + }); + await testServerProcess?.kill(); + } +}); + +test('file watching', async ({ startTestServer, writeFiles }, testInfo) => { + await writeFiles({ + 'utils.ts': ` + export const expected = 42; + `, + 'a.test.ts': ` + import { test } from '@playwright/test'; + import { expected } from "./utils"; + test('foo', () => { + expect(123).toBe(expected); + }); + `, + }); + + const testServerConnection = await startTestServer(); + const tests = await testServerConnection.listTests({}); + expect(tests.report.map(e => e.method)).toEqual(['onConfigure', 'onProject', 'onBegin', 'onEnd']); + + await testServerConnection.watch({ fileNames: [testInfo.outputPath('a.test.ts')] }); + + await writeFiles({ + 'utils.ts': ` + export const expected = 123; + `, + }); + + await expect.poll(() => testServerConnection.events).toHaveLength(1); + expect(testServerConnection.events).toEqual([ + ['testFilesChanged', { testFiles: [testInfo.outputPath('a.test.ts')] }] + ]); +}); + +test('stdio interception', async ({ startTestServer, writeFiles }) => { + const testServerConnection = await startTestServer(); + await testServerConnection.initialize({ interceptStdio: true }); + await writeFiles({ + 'a.test.ts': ` + import { test, expect } from '@playwright/test'; + test('foo', () => { + console.log("this goes to stdout"); + console.error("this goes to stderr"); + expect(true).toBe(true); + }); + `, + }); + + const tests = await testServerConnection.runTests({ trace: 'on' }); + expect(tests).toEqual({ status: 'passed' }); + await expect.poll(() => testServerConnection.events).toEqual(expect.arrayContaining([ + ['stdio', { type: 'stderr', text: 'this goes to stderr\n' }], + ['stdio', { type: 'stdout', text: 'this goes to stdout\n' }] + ])); +}); + +test('start dev server', async ({ startTestServer, writeFiles, runInlineTest }) => { + await writeFiles({ + 'playwright.config.ts': playwrightCtConfigText, + 'playwright/index.html': ``, + 'playwright/index.ts': ``, + 'src/button.tsx': ` + export const Button = () => ; + `, + 'src/button.test.tsx': ` + import { test, expect } from '@playwright/experimental-ct-react'; + import { Button } from './button'; + + test('pass', async ({ mount }) => { + const component = await mount(); + await expect(component).toHaveText('Button', { timeout: 1 }); + }); + `, + }); + + const testServerConnection = await startTestServer(); + await testServerConnection.initialize({ interceptStdio: true }); + expect((await testServerConnection.runGlobalSetup({})).status).toBe('passed'); + expect((await testServerConnection.startDevServer({})).status).toBe('passed'); + + const result = await runInlineTest({}, { workers: 1 }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(1); + expect(result.output).toContain('Dev Server is already running at'); + + expect((await testServerConnection.stopDevServer({})).status).toBe('passed'); + expect((await testServerConnection.runGlobalTeardown({})).status).toBe('passed'); +});