chore: refactor graceful close, rename to host/main (#20283)

This commit is contained in:
Pavel Feldman 2023-01-22 15:04:29 -08:00 committed by GitHub
parent 71798d658f
commit 9f31bcfbab
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 71 additions and 41 deletions

View file

@ -19,9 +19,9 @@ import type { SerializedConfig } from './ipc';
import { ProcessHost } from './processHost'; import { ProcessHost } from './processHost';
import { Suite } from './test'; import { Suite } from './test';
export class LoaderHost extends ProcessHost<SerializedConfig> { export class LoaderHost extends ProcessHost {
constructor() { constructor() {
super(require.resolve('./loaderRunner.js'), 'loader'); super(require.resolve('./loaderMain.js'), 'loader');
} }
async start(config: SerializedConfig) { async start(config: SerializedConfig) {

View file

@ -21,7 +21,7 @@ import { ProcessRunner } from './process';
import { loadTestFilesInProcess } from './testLoader'; import { loadTestFilesInProcess } from './testLoader';
import { setFatalErrorSink } from './globals'; import { setFatalErrorSink } from './globals';
export class LoaderRunner extends ProcessRunner { export class LoaderMain extends ProcessRunner {
private _config: SerializedConfig; private _config: SerializedConfig;
private _configLoaderPromise: Promise<ConfigLoader> | undefined; private _configLoaderPromise: Promise<ConfigLoader> | undefined;
@ -45,4 +45,4 @@ export class LoaderRunner extends ProcessRunner {
} }
} }
export const create = (config: SerializedConfig) => new LoaderRunner(config); export const create = (config: SerializedConfig) => new LoaderMain(config);

View file

@ -15,7 +15,7 @@
*/ */
import type { WriteStream } from 'tty'; import type { WriteStream } from 'tty';
import type { ProcessInitParams, TeardownErrorsPayload, TtyParams } from './ipc'; import type { ProcessInitParams, TtyParams } from './ipc';
import { startProfiling, stopProfiling } from './profiler'; import { startProfiling, stopProfiling } from './profiler';
import type { TestInfoError } from './types'; import type { TestInfoError } from './types';
import { serializeError } from './util'; import { serializeError } from './util';
@ -35,7 +35,6 @@ export type ProtocolResponse = {
}; };
export class ProcessRunner { export class ProcessRunner {
appendProcessTeardownDiagnostics(error: TestInfoError) { }
async gracefullyClose(): Promise<void> { } async gracefullyClose(): Promise<void> { }
protected dispatchEvent(method: string, params: any) { protected dispatchEvent(method: string, params: any) {
@ -53,7 +52,7 @@ process.on('SIGINT', () => {});
process.on('SIGTERM', () => {}); process.on('SIGTERM', () => {});
let processRunner: ProcessRunner; let processRunner: ProcessRunner;
let processName: string | undefined; let processName: string;
process.on('message', async message => { process.on('message', async message => {
if (message.method === '__init__') { if (message.method === '__init__') {
const { processParams, runnerParams, runnerScript } = message.params as { processParams: ProcessInitParams, runnerParams: any, runnerScript: string }; const { processParams, runnerParams, runnerScript } = message.params as { processParams: ProcessInitParams, runnerParams: any, runnerScript: string };
@ -62,6 +61,7 @@ process.on('message', async message => {
startProfiling(); startProfiling();
const { create } = require(runnerScript); const { create } = require(runnerScript);
processRunner = create(runnerParams) as ProcessRunner; processRunner = create(runnerParams) as ProcessRunner;
processName = processParams.processName;
return; return;
} }
if (message.method === '__stop__') { if (message.method === '__stop__') {
@ -88,20 +88,8 @@ async function gracefullyCloseAndExit() {
// Force exit after 30 seconds. // Force exit after 30 seconds.
setTimeout(() => process.exit(0), 30000); setTimeout(() => process.exit(0), 30000);
// Meanwhile, try to gracefully shutdown. // Meanwhile, try to gracefully shutdown.
try { await processRunner.gracefullyClose().catch(() => {});
if (processRunner) await stopProfiling(processName).catch(() => {});
await processRunner.gracefullyClose();
if (processName)
await stopProfiling(processName);
} catch (e) {
try {
const error = serializeError(e);
processRunner.appendProcessTeardownDiagnostics(error);
const payload: TeardownErrorsPayload = { fatalErrors: [error] };
sendMessageToParent({ method: 'teardownErrors', params: payload });
} catch {
}
}
process.exit(0); process.exit(0);
} }

View file

@ -26,7 +26,7 @@ export type ProcessExitData = {
signal: NodeJS.Signals | null; signal: NodeJS.Signals | null;
}; };
export class ProcessHost<InitParams> extends EventEmitter { export class ProcessHost extends EventEmitter {
private process!: child_process.ChildProcess; private process!: child_process.ChildProcess;
private _didSendStop = false; private _didSendStop = false;
private _didFail = false; private _didFail = false;
@ -42,7 +42,7 @@ export class ProcessHost<InitParams> extends EventEmitter {
this._processName = processName; this._processName = processName;
} }
protected async startRunner(runnerParams: InitParams, inheritStdio: boolean, env: NodeJS.ProcessEnv) { protected async startRunner(runnerParams: any, inheritStdio: boolean, env: NodeJS.ProcessEnv) {
this.process = child_process.fork(require.resolve('./process'), { this.process = child_process.fork(require.resolve('./process'), {
detached: false, detached: false,
env: { ...process.env, ...env }, env: { ...process.env, ...env },

View file

@ -341,7 +341,7 @@ export class Runner {
} }
private async _loadTests(testFiles: Set<string>): Promise<Suite> { private async _loadTests(testFiles: Set<string>): Promise<Suite> {
if (process.env.PWTEST_OOP_LOADER) { if (process.env.PW_TEST_OOP_LOADER) {
const loaderHost = new LoaderHost(); const loaderHost = new LoaderHost();
await loaderHost.start(this._configLoader.serializedConfig()); await loaderHost.start(this._configLoader.serializedConfig());
try { try {

View file

@ -20,7 +20,7 @@ import { ProcessHost } from './processHost';
let lastWorkerIndex = 0; let lastWorkerIndex = 0;
export class WorkerHost extends ProcessHost<WorkerInitParams> { export class WorkerHost extends ProcessHost {
readonly parallelIndex: number; readonly parallelIndex: number;
readonly workerIndex: number; readonly workerIndex: number;
private _hash: string; private _hash: string;
@ -29,7 +29,7 @@ export class WorkerHost extends ProcessHost<WorkerInitParams> {
constructor(testGroup: TestGroup, parallelIndex: number, config: SerializedConfig) { constructor(testGroup: TestGroup, parallelIndex: number, config: SerializedConfig) {
const workerIndex = lastWorkerIndex++; const workerIndex = lastWorkerIndex++;
super(require.resolve('./workerRunner.js'), `worker-${workerIndex}`); super(require.resolve('./workerMain.js'), `worker-${workerIndex}`);
this.workerIndex = workerIndex; this.workerIndex = workerIndex;
this.parallelIndex = parallelIndex; this.parallelIndex = parallelIndex;
this._hash = testGroup.workerHash; this._hash = testGroup.workerHash;

View file

@ -34,7 +34,7 @@ import { PoolBuilder } from './poolBuilder';
const removeFolderAsync = util.promisify(rimraf); const removeFolderAsync = util.promisify(rimraf);
export class WorkerRunner extends ProcessRunner { export class WorkerMain extends ProcessRunner {
private _params: WorkerInitParams; private _params: WorkerInitParams;
private _configLoader!: ConfigLoader; private _configLoader!: ConfigLoader;
private _testLoader!: TestLoader; private _testLoader!: TestLoader;
@ -109,19 +109,23 @@ export class WorkerRunner extends ProcessRunner {
} }
override async gracefullyClose() { override async gracefullyClose() {
await this._stop(); try {
await this._stop();
// We have to load the project to get the right deadline below.
await this._loadIfNeeded();
await this._teardownScopes();
} catch (e) {
this._fatalErrors.push(serializeError(e));
}
// We have to load the project to get the right deadline below.
await this._loadIfNeeded();
await this._teardownScopes();
if (this._fatalErrors.length) { if (this._fatalErrors.length) {
this.appendProcessTeardownDiagnostics(this._fatalErrors[this._fatalErrors.length - 1]); this._appendProcessTeardownDiagnostics(this._fatalErrors[this._fatalErrors.length - 1]);
const payload: TeardownErrorsPayload = { fatalErrors: this._fatalErrors }; const payload: TeardownErrorsPayload = { fatalErrors: this._fatalErrors };
this.dispatchEvent('teardownErrors', payload); this.dispatchEvent('teardownErrors', payload);
} }
} }
override appendProcessTeardownDiagnostics(error: TestInfoError) { private _appendProcessTeardownDiagnostics(error: TestInfoError) {
if (!this._lastRunningTests.length) if (!this._lastRunningTests.length)
return; return;
const count = this._totalRunningTests === 1 ? '1 test' : `${this._totalRunningTests} tests`; const count = this._totalRunningTests === 1 ? '1 test' : `${this._totalRunningTests} tests`;
@ -643,4 +647,4 @@ function chunkToParams(chunk: Buffer | string): { text?: string, buffer?: strin
return { text: chunk }; return { text: chunk };
} }
export const create = (params: WorkerInitParams) => new WorkerRunner(params); export const create = (params: WorkerInitParams) => new WorkerMain(params);

View file

@ -63,7 +63,7 @@ test('should succeed', async ({ runInlineTest }) => {
test('should report suite errors', async ({ runInlineTest }) => { test('should report suite errors', async ({ runInlineTest }) => {
const { exitCode, failed, output } = await runInlineTest({ const { exitCode, failed, output } = await runInlineTest({
'suite-error.spec.js': ` 'suite-error.spec.js': `
if (new Error().stack.includes('workerRunner')) if (new Error().stack.includes('workerMain'))
throw new Error('Suite error'); throw new Error('Suite error');
const { test } = pwt; const { test } = pwt;

View file

@ -280,7 +280,10 @@ test('should resolve .js import to .tsx file in ESM mode for components', async
test.skip(nodeVersion.major < 16); test.skip(nodeVersion.major < 16);
const result = await runInlineTest({ const result = await runInlineTest({
'package.json': `{ "type": "module" }`, 'package.json': `{ "type": "module" }`,
'playwright.config.ts': `export default { projects: [{name: 'foo'}] };`, 'playwright.config.ts': `
import { defineConfig } from '@playwright/experimental-ct-react';
export default defineConfig({ projects: [{name: 'foo'}] });
`,
'playwright/index.html': `<script type="module" src="./index.ts"></script>`, 'playwright/index.html': `<script type="module" src="./index.ts"></script>`,
'playwright/index.ts': ``, 'playwright/index.ts': ``,

View file

@ -563,6 +563,10 @@ test('should resolve .js import to .tsx file in non-ESM mode', async ({ runInlin
test('should resolve .js import to .tsx file in non-ESM mode for components', async ({ runInlineTest }) => { test('should resolve .js import to .tsx file in non-ESM mode for components', async ({ runInlineTest }) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': `
import { defineConfig } from '@playwright/experimental-ct-react';
export default defineConfig({ projects: [{name: 'foo'}] });
`,
'playwright/index.html': `<script type="module" src="./index.ts"></script>`, 'playwright/index.html': `<script type="module" src="./index.ts"></script>`,
'playwright/index.ts': ``, 'playwright/index.ts': ``,

View file

@ -24,7 +24,6 @@ const outputDir = path.join(__dirname, '..', '..', 'test-results');
const config: Config = { const config: Config = {
timeout: 30000, timeout: 30000,
forbidOnly: !!process.env.CI, forbidOnly: !!process.env.CI,
fullyParallel: !process.env.CI,
workers: process.env.CI ? 2 : undefined, workers: process.env.CI ? 2 : undefined,
preserveOutput: process.env.CI ? 'failures-only' : 'always', preserveOutput: process.env.CI ? 'failures-only' : 'always',
snapshotPathTemplate: '__screenshots__/{testFilePath}/{arg}{ext}', snapshotPathTemplate: '__screenshots__/{testFilePath}/{arg}{ext}',

View file

@ -19,8 +19,14 @@ import fs from 'fs';
test.describe.configure({ mode: 'parallel' }); test.describe.configure({ mode: 'parallel' });
const playwrightConfig = `
import { defineConfig } from '@playwright/experimental-ct-react';
export default defineConfig({ projects: [{name: 'foo'}] });
`;
test('should work with the empty component list', async ({ runInlineTest }, testInfo) => { test('should work with the empty component list', async ({ runInlineTest }, testInfo) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'playwright/index.html': `<script type="module" src="./index.js"></script>`, 'playwright/index.html': `<script type="module" src="./index.js"></script>`,
'playwright/index.js': ``, 'playwright/index.js': ``,
@ -45,6 +51,7 @@ test('should work with the empty component list', async ({ runInlineTest }, test
test('should extract component list', async ({ runInlineTest }, testInfo) => { test('should extract component list', async ({ runInlineTest }, testInfo) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'playwright/index.html': `<script type="module" src="./index.ts"></script>`, 'playwright/index.html': `<script type="module" src="./index.ts"></script>`,
'playwright/index.ts': ``, 'playwright/index.ts': ``,
@ -206,6 +213,7 @@ test('should cache build', async ({ runInlineTest }, testInfo) => {
await test.step('original test', async () => { await test.step('original test', async () => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'playwright/index.html': `<script type="module" src="./index.ts"></script>`, 'playwright/index.html': `<script type="module" src="./index.ts"></script>`,
'playwright/index.ts': ``, 'playwright/index.ts': ``,
@ -232,7 +240,9 @@ test('should cache build', async ({ runInlineTest }, testInfo) => {
}); });
await test.step('re-run same test', async () => { await test.step('re-run same test', async () => {
const result = await runInlineTest({}, { workers: 1 }); const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
}, { workers: 1 });
expect(result.exitCode).toBe(0); expect(result.exitCode).toBe(0);
expect(result.passed).toBe(1); expect(result.passed).toBe(1);
const output = stripAnsi(result.output); const output = stripAnsi(result.output);
@ -241,6 +251,7 @@ test('should cache build', async ({ runInlineTest }, testInfo) => {
await test.step('modify test', async () => { await test.step('modify test', async () => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'src/button.test.tsx': ` 'src/button.test.tsx': `
//@no-header //@no-header
import { test, expect } from '@playwright/experimental-ct-react'; import { test, expect } from '@playwright/experimental-ct-react';
@ -260,6 +271,7 @@ test('should cache build', async ({ runInlineTest }, testInfo) => {
await test.step('modify source', async () => { await test.step('modify source', async () => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'src/button.tsx': ` 'src/button.tsx': `
export const Button = () => <button>Button 2</button>; export const Button = () => <button>Button 2</button>;
`, `,
@ -273,6 +285,7 @@ test('should cache build', async ({ runInlineTest }, testInfo) => {
test('should not use global config for preview', async ({ runInlineTest }) => { test('should not use global config for preview', async ({ runInlineTest }) => {
const result1 = await runInlineTest({ const result1 = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'playwright/index.html': `<script type="module" src="./index.js"></script>`, 'playwright/index.html': `<script type="module" src="./index.js"></script>`,
'playwright/index.js': ``, 'playwright/index.js': ``,
'vite.config.js': ` 'vite.config.js': `
@ -293,7 +306,9 @@ test('should not use global config for preview', async ({ runInlineTest }) => {
expect(result1.exitCode).toBe(0); expect(result1.exitCode).toBe(0);
expect(result1.passed).toBe(1); expect(result1.passed).toBe(1);
const result2 = await runInlineTest({}, { workers: 1 }); const result2 = await runInlineTest({
'playwright.config.ts': playwrightConfig,
}, { workers: 1 });
expect(result2.exitCode).toBe(0); expect(result2.exitCode).toBe(0);
expect(result2.passed).toBe(1); expect(result2.passed).toBe(1);
}); });
@ -304,8 +319,9 @@ test('should work with https enabled', async ({ runInlineTest }) => {
'playwright/index.js': `//@no-header`, 'playwright/index.js': `//@no-header`,
'playwright.config.js': ` 'playwright.config.js': `
//@no-header //@no-header
import { defineConfig } from '@playwright/experimental-ct-react';
import basicSsl from '@vitejs/plugin-basic-ssl'; import basicSsl from '@vitejs/plugin-basic-ssl';
export default { export default defineConfig({
use: { use: {
ignoreHTTPSErrors: true, ignoreHTTPSErrors: true,
ctViteConfig: { ctViteConfig: {
@ -315,7 +331,7 @@ test('should work with https enabled', async ({ runInlineTest }) => {
} }
} }
}, },
}; });
`, `,
'http.test.tsx': ` 'http.test.tsx': `
//@no-header //@no-header

View file

@ -16,8 +16,14 @@
import { test, expect } from './playwright-test-fixtures'; import { test, expect } from './playwright-test-fixtures';
const playwrightConfig = `
import { defineConfig } from '@playwright/experimental-ct-react';
export default defineConfig({ projects: [{name: 'foo'}] });
`;
test('should work with TSX', async ({ runInlineTest }) => { test('should work with TSX', async ({ runInlineTest }) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'playwright/index.html': `<script type="module" src="./index.ts"></script>`, 'playwright/index.html': `<script type="module" src="./index.ts"></script>`,
'playwright/index.ts': ` 'playwright/index.ts': `
//@no-header //@no-header
@ -44,6 +50,7 @@ test('should work with TSX', async ({ runInlineTest }) => {
test('should work with JSX', async ({ runInlineTest }) => { test('should work with JSX', async ({ runInlineTest }) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'playwright/index.html': `<script type="module" src="./index.js"></script>`, 'playwright/index.html': `<script type="module" src="./index.js"></script>`,
'playwright/index.js': ` 'playwright/index.js': `
//@no-header //@no-header
@ -72,6 +79,7 @@ test('should work with JSX', async ({ runInlineTest }) => {
test('should work with JSX in JS', async ({ runInlineTest }) => { test('should work with JSX in JS', async ({ runInlineTest }) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'playwright/index.html': `<script type="module" src="./index.js"></script>`, 'playwright/index.html': `<script type="module" src="./index.js"></script>`,
'playwright/index.js': ` 'playwright/index.js': `
//@no-header //@no-header
@ -100,6 +108,7 @@ test('should work with JSX in JS', async ({ runInlineTest }) => {
test('should work with JSX in JS and in JSX', async ({ runInlineTest }) => { test('should work with JSX in JS and in JSX', async ({ runInlineTest }) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'playwright/index.html': `<script type="module" src="./index.js"></script>`, 'playwright/index.html': `<script type="module" src="./index.js"></script>`,
'playwright/index.js': ` 'playwright/index.js': `
//@no-header //@no-header
@ -140,6 +149,7 @@ test('should work with JSX in JS and in JSX', async ({ runInlineTest }) => {
test('should work with stray TSX import', async ({ runInlineTest }) => { test('should work with stray TSX import', async ({ runInlineTest }) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'playwright/index.html': `<script type="module" src="./index.ts"></script>`, 'playwright/index.html': `<script type="module" src="./index.ts"></script>`,
'playwright/index.ts': ` 'playwright/index.ts': `
//@no-header //@no-header
@ -174,6 +184,7 @@ test('should work with stray TSX import', async ({ runInlineTest }) => {
test('should work with stray JSX import', async ({ runInlineTest }) => { test('should work with stray JSX import', async ({ runInlineTest }) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'playwright/index.html': `<script type="module" src="./index.js"></script>`, 'playwright/index.html': `<script type="module" src="./index.js"></script>`,
'playwright/index.js': ` 'playwright/index.js': `
//@no-header //@no-header
@ -208,6 +219,7 @@ test('should work with stray JSX import', async ({ runInlineTest }) => {
test.fixme('should work with stray JS import', async ({ runInlineTest }) => { test.fixme('should work with stray JS import', async ({ runInlineTest }) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'playwright/index.html': `<script type="module" src="./index.js"></script>`, 'playwright/index.html': `<script type="module" src="./index.js"></script>`,
'playwright/index.js': ` 'playwright/index.js': `
//@no-header //@no-header
@ -242,6 +254,7 @@ test.fixme('should work with stray JS import', async ({ runInlineTest }) => {
test('should work with JSX in variable', async ({ runInlineTest }) => { test('should work with JSX in variable', async ({ runInlineTest }) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'playwright/index.html': `<script type="module" src="./index.js"></script>`, 'playwright/index.html': `<script type="module" src="./index.js"></script>`,
'playwright/index.js': ` 'playwright/index.js': `
//@no-header //@no-header
@ -272,6 +285,7 @@ test('should work with JSX in variable', async ({ runInlineTest }) => {
test('should return root locator for fragments', async ({ runInlineTest }) => { test('should return root locator for fragments', async ({ runInlineTest }) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'playwright/index.html': `<script type="module" src="./index.js"></script>`, 'playwright/index.html': `<script type="module" src="./index.js"></script>`,
'playwright/index.js': `//@no-header`, 'playwright/index.js': `//@no-header`,
@ -299,6 +313,7 @@ test('should return root locator for fragments', async ({ runInlineTest }) => {
test('should respect default property values', async ({ runInlineTest }) => { test('should respect default property values', async ({ runInlineTest }) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'playwright/index.html': `<script type="module" src="./index.ts"></script>`, 'playwright/index.html': `<script type="module" src="./index.ts"></script>`,
'playwright/index.ts': `//@no-header`, 'playwright/index.ts': `//@no-header`,
'src/label.tsx': `//@no-header 'src/label.tsx': `//@no-header
@ -323,6 +338,7 @@ test('should respect default property values', async ({ runInlineTest }) => {
test('should bundle public folder', async ({ runInlineTest }) => { test('should bundle public folder', async ({ runInlineTest }) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': playwrightConfig,
'playwright/index.html': `<script type="module" src="./index.ts"></script>`, 'playwright/index.html': `<script type="module" src="./index.ts"></script>`,
'playwright/index.ts': ` 'playwright/index.ts': `
//@no-header //@no-header