diff --git a/packages/playwright-ct-core/src/vitePlugin.ts b/packages/playwright-ct-core/src/vitePlugin.ts index 13b2150264..64331bafa2 100644 --- a/packages/playwright-ct-core/src/vitePlugin.ts +++ b/packages/playwright-ct-core/src/vitePlugin.ts @@ -173,7 +173,8 @@ export function createPlugin( }, end: async () => { - await new Promise(f => stoppableServer.stop(f)); + if (stoppableServer) + await new Promise(f => stoppableServer.stop(f)); }, }; } diff --git a/packages/playwright-test/src/runner/taskRunner.ts b/packages/playwright-test/src/runner/taskRunner.ts index 98c24f7bf2..7e990391d6 100644 --- a/packages/playwright-test/src/runner/taskRunner.ts +++ b/packages/playwright-test/src/runner/taskRunner.ts @@ -21,8 +21,8 @@ import { SigIntWatcher } from './sigIntWatcher'; import { serializeError } from '../util'; import type { ReporterV2 } from '../reporters/reporterV2'; -type TaskTeardown = () => Promise | undefined; -export type Task = (context: Context, errors: TestError[], softErrors: TestError[]) => Promise | undefined; +type TaskPhase = (context: Context, errors: TestError[], softErrors: TestError[]) => Promise | void; +export type Task = { setup?: TaskPhase, teardown?: TaskPhase }; export class TaskRunner { private _tasks: { name: string, task: Task }[] = []; @@ -50,7 +50,7 @@ export class TaskRunner { async runDeferCleanup(context: Context, deadline: number, cancelPromise = new ManualPromise()): Promise<{ status: FullResult['status'], cleanup: () => Promise }> { const sigintWatcher = new SigIntWatcher(); const timeoutWatcher = new TimeoutWatcher(deadline); - const teardownRunner = new TaskRunner(this._reporter, this._globalTimeoutForError); + const teardownRunner = new TaskRunner(this._reporter, this._globalTimeoutForError); teardownRunner._isTearDown = true; let currentTaskName: string | undefined; @@ -64,9 +64,8 @@ export class TaskRunner { const errors: TestError[] = []; const softErrors: TestError[] = []; try { - const teardown = await task(context, errors, softErrors); - if (teardown) - teardownRunner._tasks.unshift({ name: `teardown for ${name}`, task: teardown }); + teardownRunner._tasks.unshift({ name: `teardown for ${name}`, task: { setup: task.teardown } }); + await task.setup?.(context, errors, softErrors); } catch (e) { debug('pw:test:task')(`error in "${name}": `, e); errors.push(serializeError(e)); @@ -106,12 +105,14 @@ export class TaskRunner { status = 'failed'; } cancelPromise?.resolve(); + // Note that upon hitting deadline, we "run cleanup", but it exits immediately + // because of the same deadline. Essentially, we're not perfomring any cleanup. const cleanup = () => teardownRunner.runDeferCleanup(context, deadline).then(r => r.status); return { status, cleanup }; } } -export class TimeoutWatcher { +class TimeoutWatcher { private _timedOut = false; readonly promise = new ManualPromise(); private _timer: NodeJS.Timeout | undefined; diff --git a/packages/playwright-test/src/runner/tasks.ts b/packages/playwright-test/src/runner/tasks.ts index 26a8b2adb6..8a5399680e 100644 --- a/packages/playwright-test/src/runner/tasks.ts +++ b/packages/playwright-test/src/runner/tasks.ts @@ -93,7 +93,6 @@ function addRunTasks(taskRunner: TaskRunner, config: FullConfigInternal taskRunner.addTask('report begin', createReportBeginTask()); for (const plugin of config.plugins) taskRunner.addTask('plugin begin', createPluginBeginTask(plugin)); - taskRunner.addTask('start workers', createWorkersTask()); taskRunner.addTask('test suite', createRunTestsTask()); return taskRunner; } @@ -106,189 +105,209 @@ export function createTaskRunnerForList(config: FullConfigInternal, reporter: Re } function createReportBeginTask(): Task { - return async ({ config, reporter, rootSuite }) => { - const montonicStartTime = monotonicTime(); - reporter.onBegin(rootSuite!); - return async () => { + let montonicStartTime = 0; + return { + setup: async ({ config, reporter, rootSuite }) => { + montonicStartTime = monotonicTime(); + reporter.onBegin(rootSuite!); + }, + teardown: async ({ config }) => { config.config.metadata.totalTime = monotonicTime() - montonicStartTime; - }; + }, }; } function createPluginSetupTask(plugin: TestRunnerPluginRegistration): Task { - return async ({ config, reporter }) => { - if (typeof plugin.factory === 'function') - plugin.instance = await plugin.factory(); - else - plugin.instance = plugin.factory; - await plugin.instance?.setup?.(config.config, config.configDir, reporter); - return () => plugin.instance?.teardown?.(); + return { + setup: async ({ config, reporter }) => { + if (typeof plugin.factory === 'function') + plugin.instance = await plugin.factory(); + else + plugin.instance = plugin.factory; + await plugin.instance?.setup?.(config.config, config.configDir, reporter); + }, + teardown: async () => { + await plugin.instance?.teardown?.(); + }, }; } function createPluginBeginTask(plugin: TestRunnerPluginRegistration): Task { - return async ({ rootSuite }) => { - await plugin.instance?.begin?.(rootSuite!); - return () => plugin.instance?.end?.(); + return { + setup: async ({ rootSuite }) => { + await plugin.instance?.begin?.(rootSuite!); + }, + teardown: async () => { + await plugin.instance?.end?.(); + }, }; } function createGlobalSetupTask(): Task { - return async ({ config }) => { - const setupHook = config.config.globalSetup ? await loadGlobalHook(config, config.config.globalSetup) : undefined; - const teardownHook = config.config.globalTeardown ? await loadGlobalHook(config, config.config.globalTeardown) : undefined; - const globalSetupResult = setupHook ? await setupHook(config.config) : undefined; - return async () => { + let globalSetupResult: any; + let globalSetupFinished = false; + let teardownHook: any; + return { + setup: async ({ config }) => { + const setupHook = config.config.globalSetup ? await loadGlobalHook(config, config.config.globalSetup) : undefined; + teardownHook = config.config.globalTeardown ? await loadGlobalHook(config, config.config.globalTeardown) : undefined; + globalSetupResult = setupHook ? await setupHook(config.config) : undefined; + globalSetupFinished = true; + }, + teardown: async ({ config }) => { if (typeof globalSetupResult === 'function') await globalSetupResult(); - await teardownHook?.(config.config); - }; + if (globalSetupFinished) + await teardownHook?.(config.config); + }, }; } function createRemoveOutputDirsTask(): Task { - return async ({ config }) => { - if (process.env.PW_TEST_NO_REMOVE_OUTPUT_DIRS) - return; - const outputDirs = new Set(); - for (const p of config.projects) { - if (!config.cliProjectFilter || config.cliProjectFilter.includes(p.project.name)) - outputDirs.add(p.project.outputDir); - } - - await Promise.all(Array.from(outputDirs).map(outputDir => rimraf(outputDir).catch(async (error: any) => { - if ((error as any).code === 'EBUSY') { - // We failed to remove folder, might be due to the whole folder being mounted inside a container: - // https://github.com/microsoft/playwright/issues/12106 - // Do a best-effort to remove all files inside of it instead. - const entries = await readDirAsync(outputDir).catch(e => []); - await Promise.all(entries.map(entry => rimraf(path.join(outputDir, entry)))); - } else { - throw error; + return { + setup: async ({ config }) => { + if (process.env.PW_TEST_NO_REMOVE_OUTPUT_DIRS) + return; + const outputDirs = new Set(); + for (const p of config.projects) { + if (!config.cliProjectFilter || config.cliProjectFilter.includes(p.project.name)) + outputDirs.add(p.project.outputDir); } - }))); + + await Promise.all(Array.from(outputDirs).map(outputDir => rimraf(outputDir).catch(async (error: any) => { + if ((error as any).code === 'EBUSY') { + // We failed to remove folder, might be due to the whole folder being mounted inside a container: + // https://github.com/microsoft/playwright/issues/12106 + // Do a best-effort to remove all files inside of it instead. + const entries = await readDirAsync(outputDir).catch(e => []); + await Promise.all(entries.map(entry => rimraf(path.join(outputDir, entry)))); + } else { + throw error; + } + }))); + }, }; } function createLoadTask(mode: 'out-of-process' | 'in-process', options: { filterOnly: boolean, failOnLoadErrors: boolean, doNotRunTestsOutsideProjectFilter?: boolean, additionalFileMatcher?: Matcher }): Task { - return async (testRun, errors, softErrors) => { - await collectProjectsAndTestFiles(testRun, !!options.doNotRunTestsOutsideProjectFilter, options.additionalFileMatcher); - await loadFileSuites(testRun, mode, options.failOnLoadErrors ? errors : softErrors); - testRun.rootSuite = await createRootSuite(testRun, options.failOnLoadErrors ? errors : softErrors, !!options.filterOnly); - // Fail when no tests. - if (options.failOnLoadErrors && !testRun.rootSuite.allTests().length && !testRun.config.cliPassWithNoTests && !testRun.config.config.shard) - throw new Error(`No tests found`); + return { + setup: async (testRun, errors, softErrors) => { + await collectProjectsAndTestFiles(testRun, !!options.doNotRunTestsOutsideProjectFilter, options.additionalFileMatcher); + await loadFileSuites(testRun, mode, options.failOnLoadErrors ? errors : softErrors); + testRun.rootSuite = await createRootSuite(testRun, options.failOnLoadErrors ? errors : softErrors, !!options.filterOnly); + // Fail when no tests. + if (options.failOnLoadErrors && !testRun.rootSuite.allTests().length && !testRun.config.cliPassWithNoTests && !testRun.config.config.shard) + throw new Error(`No tests found`); + }, }; } function createPhasesTask(): Task { - return async testRun => { - let maxConcurrentTestGroups = 0; + return { + setup: async testRun => { + let maxConcurrentTestGroups = 0; - const processed = new Set(); - const projectToSuite = new Map(testRun.rootSuite!.suites.map(suite => [suite._fullProject!, suite])); - const allProjects = [...projectToSuite.keys()]; - const teardownToSetups = buildTeardownToSetupsMap(allProjects); - const teardownToSetupsDependents = new Map(); - for (const [teardown, setups] of teardownToSetups) { - const closure = buildDependentProjects(setups, allProjects); - closure.delete(teardown); - teardownToSetupsDependents.set(teardown, [...closure]); - } - - for (let i = 0; i < projectToSuite.size; i++) { - // Find all projects that have all their dependencies processed by previous phases. - const phaseProjects: FullProjectInternal[] = []; - for (const project of projectToSuite.keys()) { - if (processed.has(project)) - continue; - const projectsThatShouldFinishFirst = [...project.deps, ...(teardownToSetupsDependents.get(project) || [])]; - if (projectsThatShouldFinishFirst.find(p => !processed.has(p))) - continue; - phaseProjects.push(project); + const processed = new Set(); + const projectToSuite = new Map(testRun.rootSuite!.suites.map(suite => [suite._fullProject!, suite])); + const allProjects = [...projectToSuite.keys()]; + const teardownToSetups = buildTeardownToSetupsMap(allProjects); + const teardownToSetupsDependents = new Map(); + for (const [teardown, setups] of teardownToSetups) { + const closure = buildDependentProjects(setups, allProjects); + closure.delete(teardown); + teardownToSetupsDependents.set(teardown, [...closure]); } - // Create a new phase. - for (const project of phaseProjects) - processed.add(project); - if (phaseProjects.length) { - let testGroupsInPhase = 0; - const phase: Phase = { dispatcher: new Dispatcher(testRun.config, testRun.reporter), projects: [] }; - testRun.phases.push(phase); - for (const project of phaseProjects) { - const projectSuite = projectToSuite.get(project)!; - const testGroups = createTestGroups(projectSuite, testRun.config.config.workers); - phase.projects.push({ project, projectSuite, testGroups }); - testGroupsInPhase += testGroups.length; + for (let i = 0; i < projectToSuite.size; i++) { + // Find all projects that have all their dependencies processed by previous phases. + const phaseProjects: FullProjectInternal[] = []; + for (const project of projectToSuite.keys()) { + if (processed.has(project)) + continue; + const projectsThatShouldFinishFirst = [...project.deps, ...(teardownToSetupsDependents.get(project) || [])]; + if (projectsThatShouldFinishFirst.find(p => !processed.has(p))) + continue; + phaseProjects.push(project); + } + + // Create a new phase. + for (const project of phaseProjects) + processed.add(project); + if (phaseProjects.length) { + let testGroupsInPhase = 0; + const phase: Phase = { dispatcher: new Dispatcher(testRun.config, testRun.reporter), projects: [] }; + testRun.phases.push(phase); + for (const project of phaseProjects) { + const projectSuite = projectToSuite.get(project)!; + const testGroups = createTestGroups(projectSuite, testRun.config.config.workers); + phase.projects.push({ project, projectSuite, testGroups }); + testGroupsInPhase += testGroups.length; + } + debug('pw:test:task')(`created phase #${testRun.phases.length} with ${phase.projects.map(p => p.project.project.name).sort()} projects, ${testGroupsInPhase} testGroups`); + maxConcurrentTestGroups = Math.max(maxConcurrentTestGroups, testGroupsInPhase); } - debug('pw:test:task')(`created phase #${testRun.phases.length} with ${phase.projects.map(p => p.project.project.name).sort()} projects, ${testGroupsInPhase} testGroups`); - maxConcurrentTestGroups = Math.max(maxConcurrentTestGroups, testGroupsInPhase); } - } - testRun.config.config.metadata.actualWorkers = Math.min(testRun.config.config.workers, maxConcurrentTestGroups); - }; -} - -function createWorkersTask(): Task { - return async ({ phases }) => { - return async () => { - for (const { dispatcher } of phases.reverse()) - await dispatcher.stop(); - }; + testRun.config.config.metadata.actualWorkers = Math.min(testRun.config.config.workers, maxConcurrentTestGroups); + }, }; } function createRunTestsTask(): Task { - return async testRun => { - const { phases } = testRun; - const successfulProjects = new Set(); - const extraEnvByProjectId: EnvByProjectId = new Map(); - const teardownToSetups = buildTeardownToSetupsMap(phases.map(phase => phase.projects.map(p => p.project)).flat()); + return { + setup: async ({ phases }) => { + const successfulProjects = new Set(); + const extraEnvByProjectId: EnvByProjectId = new Map(); + const teardownToSetups = buildTeardownToSetupsMap(phases.map(phase => phase.projects.map(p => p.project)).flat()); - for (const { dispatcher, projects } of phases) { - // Each phase contains dispatcher and a set of test groups. - // We don't want to run the test groups beloning to the projects - // that depend on the projects that failed previously. - const phaseTestGroups: TestGroup[] = []; - for (const { project, testGroups } of projects) { - // Inherit extra enviroment variables from dependencies. - let extraEnv: Record = {}; - for (const dep of project.deps) - extraEnv = { ...extraEnv, ...extraEnvByProjectId.get(dep.id) }; - for (const setup of teardownToSetups.get(project) || []) - extraEnv = { ...extraEnv, ...extraEnvByProjectId.get(setup.id) }; - extraEnvByProjectId.set(project.id, extraEnv); + for (const { dispatcher, projects } of phases) { + // Each phase contains dispatcher and a set of test groups. + // We don't want to run the test groups beloning to the projects + // that depend on the projects that failed previously. + const phaseTestGroups: TestGroup[] = []; + for (const { project, testGroups } of projects) { + // Inherit extra enviroment variables from dependencies. + let extraEnv: Record = {}; + for (const dep of project.deps) + extraEnv = { ...extraEnv, ...extraEnvByProjectId.get(dep.id) }; + for (const setup of teardownToSetups.get(project) || []) + extraEnv = { ...extraEnv, ...extraEnvByProjectId.get(setup.id) }; + extraEnvByProjectId.set(project.id, extraEnv); - const hasFailedDeps = project.deps.some(p => !successfulProjects.has(p)); - if (!hasFailedDeps) { - phaseTestGroups.push(...testGroups); - } else { - for (const testGroup of testGroups) { - for (const test of testGroup.tests) - test._appendTestResult().status = 'skipped'; + const hasFailedDeps = project.deps.some(p => !successfulProjects.has(p)); + if (!hasFailedDeps) { + phaseTestGroups.push(...testGroups); + } else { + for (const testGroup of testGroups) { + for (const test of testGroup.tests) + test._appendTestResult().status = 'skipped'; + } + } + } + + if (phaseTestGroups.length) { + await dispatcher!.run(phaseTestGroups, extraEnvByProjectId); + await dispatcher.stop(); + for (const [projectId, envProduced] of dispatcher.producedEnvByProjectId()) { + const extraEnv = extraEnvByProjectId.get(projectId) || {}; + extraEnvByProjectId.set(projectId, { ...extraEnv, ...envProduced }); + } + } + + // If the worker broke, fail everything, we have no way of knowing which + // projects failed. + if (!dispatcher.hasWorkerErrors()) { + for (const { project, projectSuite } of projects) { + const hasFailedDeps = project.deps.some(p => !successfulProjects.has(p)); + if (!hasFailedDeps && !projectSuite.allTests().some(test => !test.ok())) + successfulProjects.add(project); } } } - - if (phaseTestGroups.length) { - await dispatcher!.run(phaseTestGroups, extraEnvByProjectId); + }, + teardown: async ({ phases }) => { + for (const { dispatcher } of phases.reverse()) await dispatcher.stop(); - for (const [projectId, envProduced] of dispatcher.producedEnvByProjectId()) { - const extraEnv = extraEnvByProjectId.get(projectId) || {}; - extraEnvByProjectId.set(projectId, { ...extraEnv, ...envProduced }); - } - } - - // If the worker broke, fail everything, we have no way of knowing which - // projects failed. - if (!dispatcher.hasWorkerErrors()) { - for (const { project, projectSuite } of projects) { - const hasFailedDeps = project.deps.some(p => !successfulProjects.has(p)); - if (!hasFailedDeps && !projectSuite.allTests().some(test => !test.ok())) - successfulProjects.add(project); - } - } - } + }, }; } diff --git a/tests/playwright-test/runner.spec.ts b/tests/playwright-test/runner.spec.ts index 3ee7cbab40..db472523f6 100644 --- a/tests/playwright-test/runner.spec.ts +++ b/tests/playwright-test/runner.spec.ts @@ -399,16 +399,19 @@ test('sigint should stop plugins', async ({ runInlineTest }) => { `, 'a.spec.js': ` import { test, expect } from '@playwright/test'; - test('test', async () => { }); + test('test', async () => { + console.log('testing!'); + }); `, }, { 'workers': 1 }, {}, { sendSIGINTAfter: 1 }); expect(result.exitCode).toBe(130); expect(result.passed).toBe(0); const output = result.output; expect(output).toContain('Plugin1 setup'); - expect(output).not.toContain('Plugin1 teardown'); + expect(output).toContain('Plugin1 teardown'); expect(output).not.toContain('Plugin2 setup'); expect(output).not.toContain('Plugin2 teardown'); + expect(output).not.toContain('testing!'); }); test('sigint should stop plugins 2', async ({ runInlineTest }) => { @@ -440,7 +443,9 @@ test('sigint should stop plugins 2', async ({ runInlineTest }) => { `, 'a.spec.js': ` import { test, expect } from '@playwright/test'; - test('test', async () => { }); + test('test', async () => { + console.log('testing!'); + }); `, }, { 'workers': 1 }, {}, { sendSIGINTAfter: 1 }); expect(result.exitCode).toBe(130); @@ -449,7 +454,8 @@ test('sigint should stop plugins 2', async ({ runInlineTest }) => { expect(output).toContain('Plugin1 setup'); expect(output).toContain('Plugin2 setup'); expect(output).toContain('Plugin1 teardown'); - expect(output).not.toContain('Plugin2 teardown'); + expect(output).toContain('Plugin2 teardown'); + expect(output).not.toContain('testing!'); }); test('should not crash with duplicate titles and .only', async ({ runInlineTest }) => {