diff --git a/packages/playwright/src/common/testType.ts b/packages/playwright/src/common/testType.ts index 1411b249b4..35e2a4668c 100644 --- a/packages/playwright/src/common/testType.ts +++ b/packages/playwright/src/common/testType.ts @@ -21,7 +21,7 @@ import { wrapFunctionWithLocation } from '../transform/transform'; import type { FixturesWithLocation } from './config'; import type { Fixtures, TestType, TestDetails } from '../../types/test'; import type { Location } from '../../types/testReporter'; -import { getPackageManagerExecCommand } from 'playwright-core/lib/utils'; +import { getPackageManagerExecCommand, zones } from 'playwright-core/lib/utils'; const testTypeSymbol = Symbol('testType'); @@ -263,9 +263,16 @@ export class TestTypeImpl { const testInfo = currentTestInfo(); if (!testInfo) throw new Error(`test.step() can only be called from a test`); - return testInfo._runAsStep({ category: 'test.step', title, box: options.box }, async () => { - // Make sure that internal "step" is not leaked to the user callback. - return await body(); + const step = testInfo._addStep({ wallTime: Date.now(), category: 'test.step', title, box: options.box }); + return await zones.run('stepZone', step, async () => { + try { + const result = await body(); + step.complete({}); + return result; + } catch (error) { + step.complete({ error }); + throw error; + } }); } diff --git a/packages/playwright/src/worker/fixtureRunner.ts b/packages/playwright/src/worker/fixtureRunner.ts index 85c2dddab9..caea7fde4b 100644 --- a/packages/playwright/src/worker/fixtureRunner.ts +++ b/packages/playwright/src/worker/fixtureRunner.ts @@ -65,22 +65,14 @@ class Fixture { return; } - testInfo._timeoutManager.setCurrentFixture(this._setupDescription); - const beforeStep = this._shouldGenerateStep ? testInfo._addStep({ - title: `fixture: ${this.registration.name}`, - category: 'fixture', + await testInfo._runAsStage({ location: this._isInternalFixture ? this.registration.location : undefined, - wallTime: Date.now(), - }) : undefined; - try { + stepInfo: this._shouldGenerateStep ? { title: `fixture: ${this.registration.name}`, category: 'fixture' } : undefined, + }, async () => { + testInfo._timeoutManager.setCurrentFixture(this._setupDescription); await this._setupInternal(testInfo); - beforeStep?.complete({}); - } catch (error) { - beforeStep?.complete({ error }); - throw error; - } finally { testInfo._timeoutManager.setCurrentFixture(undefined); - } + }); } private async _setupInternal(testInfo: TestInfoImpl) { @@ -135,24 +127,16 @@ class Fixture { } async teardown(testInfo: TestInfoImpl) { - const afterStep = this._shouldGenerateStep ? testInfo?._addStep({ - wallTime: Date.now(), - title: `fixture: ${this.registration.name}`, - category: 'fixture', + await testInfo._runAsStage({ location: this._isInternalFixture ? this.registration.location : undefined, - }) : undefined; - testInfo._timeoutManager.setCurrentFixture(this._teardownDescription); - try { + stepInfo: this._shouldGenerateStep ? { title: `fixture: ${this.registration.name}`, category: 'fixture' } : undefined, + }, async () => { + testInfo._timeoutManager.setCurrentFixture(this._teardownDescription); if (!this._teardownWithDepsComplete) this._teardownWithDepsComplete = this._teardownInternal(); await this._teardownWithDepsComplete; - afterStep?.complete({}); - } catch (error) { - afterStep?.complete({ error }); - throw error; - } finally { testInfo._timeoutManager.setCurrentFixture(undefined); - } + }); } private async _teardownInternal() { @@ -214,16 +198,18 @@ export class FixtureRunner { collector.add(registration); } - async teardownScope(scope: FixtureScope, testInfo: TestInfoImpl, onFixtureError: (error: Error) => void) { + async teardownScope(scope: FixtureScope, testInfo: TestInfoImpl) { // Teardown fixtures in the reverse order. const fixtures = Array.from(this.instanceForId.values()).reverse(); const collector = new Set(); for (const fixture of fixtures) fixture._collectFixturesInTeardownOrder(scope, collector); - for (const fixture of collector) - await fixture.teardown(testInfo).catch(onFixtureError); - if (scope === 'test') - this.testScopeClean = true; + await testInfo._runAsStage({}, async () => { + for (const fixture of collector) + await fixture.teardown(testInfo); + if (scope === 'test') + this.testScopeClean = true; + }); } async resolveParametersForFunction(fn: Function, testInfo: TestInfoImpl, autoFixtures: 'worker' | 'test' | 'all-hooks-only'): Promise { @@ -250,17 +236,16 @@ export class FixtureRunner { this._collectFixturesInSetupOrder(this.pool!.resolve(name)!, collector); // Setup fixtures. - for (const registration of collector) { - const fixture = await this._setupFixtureForRegistration(registration, testInfo); - if (fixture.failed) - return null; - } + await testInfo._runAsStage({ stopOnChildError: true }, async () => { + for (const registration of collector) + await this._setupFixtureForRegistration(registration, testInfo); + }); // Create params object. const params: { [key: string]: any } = {}; for (const name of names) { const registration = this.pool!.resolve(name)!; - const fixture = this.instanceForId.get(registration.id)!; + const fixture = this.instanceForId.get(registration.id); if (!fixture || fixture.failed) return null; params[name] = fixture.value; @@ -274,7 +259,9 @@ export class FixtureRunner { // Do not run the function when fixture setup has already failed. return null; } - return fn(params, testInfo); + await testInfo._runAsStage({}, async () => { + await fn(params, testInfo); + }); } private async _setupFixtureForRegistration(registration: FixtureRegistration, testInfo: TestInfoImpl): Promise { diff --git a/packages/playwright/src/worker/testInfo.ts b/packages/playwright/src/worker/testInfo.ts index e79bd61529..1124377697 100644 --- a/packages/playwright/src/worker/testInfo.ts +++ b/packages/playwright/src/worker/testInfo.ts @@ -45,9 +45,25 @@ export interface TestStepInternal { infectParentStepsWithError?: boolean; box?: boolean; isSoft?: boolean; - forceNoParent?: boolean; + isStage?: boolean; } +export type TestStage = { + runnableType?: RunnableType; + runnableSlot?: TimeSlot; + stepInfo?: { + title: string; + category: 'hook' | 'fixture'; + }; + location?: Location; + allowAndStopOnSkips?: boolean; + stopOnChildError?: boolean; + + step?: TestStepInternal; + error?: Error; + triggeredSkip?: boolean; +}; + export class TestInfoImpl implements TestInfo { private _onStepBegin: (payload: StepBeginPayload) => void; private _onStepEnd: (payload: StepEndPayload) => void; @@ -64,9 +80,9 @@ export class TestInfoImpl implements TestInfo { private readonly _requireFile: string; readonly _projectInternal: FullProjectInternal; readonly _configInternal: FullConfigInternal; - readonly _steps: TestStepInternal[] = []; + private readonly _steps: TestStepInternal[] = []; _onDidFinishTestFunction: (() => Promise) | undefined; - + private readonly _stages: TestStage[] = []; _hasNonRetriableError = false; // ------------ TestInfo fields ------------ @@ -234,20 +250,6 @@ export class TestInfoImpl implements TestInfo { this.duration = this._timeoutManager.defaultSlotTimings().elapsed | 0; } - async _runAndFailOnError(fn: () => Promise, skips?: 'allowSkips'): Promise { - try { - await fn(); - } catch (error) { - if (skips === 'allowSkips' && error instanceof SkipError) { - if (this.status === 'passed') - this.status = 'skipped'; - } else { - this._failWithError(error, true /* isHardError */, true /* retriable */); - return error; - } - } - } - private _findLastNonFinishedStep(filter: (step: TestStepInternal) => boolean) { let result: TestStepInternal | undefined; const visit = (step: TestStepInternal) => { @@ -259,33 +261,32 @@ export class TestInfoImpl implements TestInfo { return result; } + private _findLastStageStep() { + for (let i = this._stages.length - 1; i >= 0; i--) { + if (this._stages[i].step) + return this._stages[i].step; + } + } + _addStep(data: Omit): TestStepInternal { const stepId = `${data.category}@${++this._lastStepId}`; const rawStack = captureRawStack(); let parentStep: TestStepInternal | undefined; - if (data.category === 'hook' || data.category === 'fixture') { - // Predefined steps form a fixed hierarchy - find the last non-finished one. - parentStep = this._findLastNonFinishedStep(step => step.category === 'fixture' || step.category === 'hook'); + if (data.isStage) { + // Predefined stages form a fixed hierarchy - use the current one as parent. + parentStep = this._findLastStageStep(); } else { parentStep = zones.zoneData('stepZone', rawStack!) || undefined; - if (parentStep?.category === 'hook' || parentStep?.category === 'fixture') { - // Prefer last non-finished predefined step over the on-stack one, because - // some predefined steps may be missing on the stack. - parentStep = this._findLastNonFinishedStep(step => step.category === 'fixture' || step.category === 'hook'); - } else if (!parentStep) { - if (data.category === 'test.step') { - // Nest test.step without a good stack in the last non-finished predefined step like a hook. - parentStep = this._findLastNonFinishedStep(step => step.category === 'fixture' || step.category === 'hook'); - } else { - // Do not nest chains of route.continue. - parentStep = this._findLastNonFinishedStep(step => step.title !== data.title); - } + if (!parentStep && data.category !== 'test.step') { + // API steps (but not test.step calls) can be nested by time, instead of by stack. + // However, do not nest chains of route.continue by checking the title. + parentStep = this._findLastNonFinishedStep(step => step.title !== data.title); + } + if (!parentStep) { + // If no parent step on stack, assume the current stage as parent. + parentStep = this._findLastStageStep(); } - } - if (data.forceNoParent) { - // This is used to reset step hierarchy after test timeout. - parentStep = undefined; } const filteredStack = filteredStackTrace(rawStack); @@ -377,6 +378,12 @@ export class TestInfoImpl implements TestInfo { return; if (isHardError) this._hasHardError = true; + const stage = this._stages[this._stages.length - 1]; + if (stage) { + // Save the error to the stage here, so that unhandled rejections + // are attributed to the current stage. Among many errors, prefer the first one. + stage.error = stage.error ?? error; + } if (this.status === 'passed' || this.status === 'skipped') this.status = 'failed'; const serialized = serializeError(error); @@ -387,33 +394,57 @@ export class TestInfoImpl implements TestInfo { this._tracing.appendForError(serialized); } - async _runAsStepWithRunnable( - stepInfo: Omit & { - wallTime?: number, - runnableType: RunnableType; - runnableSlot?: TimeSlot; - }, cb: (step: TestStepInternal) => Promise): Promise { - return await this._timeoutManager.withRunnable({ - type: stepInfo.runnableType, - slot: stepInfo.runnableSlot, - location: stepInfo.location, - }, async () => { - return await this._runAsStep(stepInfo, cb); + async _runAsStage(stage: TestStage, cb: () => Promise) { + // Inherit some properties from parent. + const parent = this._stages[this._stages.length - 1]; + stage.allowAndStopOnSkips = stage.allowAndStopOnSkips ?? parent?.allowAndStopOnSkips ?? false; + + if (parent?.allowAndStopOnSkips && parent?.triggeredSkip) { + // Do not run more child steps after "skip" has been triggered. + return; + } + if (parent?.stopOnChildError && parent?.error) { + // Do not run more child steps after a previous one failed. + return; + } + + const runnable = stage.runnableType ? { + type: stage.runnableType, + slot: stage.runnableSlot, + location: stage.location, + } : undefined; + + return await this._timeoutManager.withRunnable(runnable, async () => { + stage.step = stage.stepInfo ? this._addStep({ ...stage.stepInfo, location: stage.location, wallTime: Date.now(), isStage: true }) : undefined; + this._stages.push(stage); + + try { + await cb(); + } catch (e) { + if (stage.allowAndStopOnSkips && (e instanceof SkipError)) { + stage.triggeredSkip = true; + if (this.status === 'passed') + this.status = 'skipped'; + } else { + this._failWithError(e, true /* isHardError */, true /* retriable */); + } + } + + if (parent) { + // Notify parent about child error and skip. + parent.error = parent.error ?? stage.error; + parent.triggeredSkip = parent.triggeredSkip || stage.triggeredSkip; + } + + const index = this._stages.indexOf(stage); + if (index !== -1) + this._stages.splice(index, 1); + stage.step?.complete({ error: stage.error }); }); } - async _runAsStep(stepInfo: Omit & { wallTime?: number }, cb: (step: TestStepInternal) => Promise): Promise { - const step = this._addStep({ wallTime: Date.now(), ...stepInfo }); - return await zones.run('stepZone', step, async () => { - try { - const result = await cb(step); - step.complete({}); - return result; - } catch (e) { - step.complete({ error: e instanceof SkipError ? undefined : e }); - throw e; - } - }); + _resetStages() { + this._stages.splice(0, this._stages.length); } _isFailure() { diff --git a/packages/playwright/src/worker/timeoutManager.ts b/packages/playwright/src/worker/timeoutManager.ts index 94e182f682..8cded8ea46 100644 --- a/packages/playwright/src/worker/timeoutManager.ts +++ b/packages/playwright/src/worker/timeoutManager.ts @@ -56,7 +56,9 @@ export class TimeoutManager { this._timeoutRunner.interrupt(); } - async withRunnable(runnable: RunnableDescription, cb: () => Promise): Promise { + async withRunnable(runnable: RunnableDescription | undefined, cb: () => Promise): Promise { + if (!runnable) + return await cb(); const existingRunnable = this._runnable; const effectiveRunnable = { ...runnable }; if (!effectiveRunnable.slot) diff --git a/packages/playwright/src/worker/workerMain.ts b/packages/playwright/src/worker/workerMain.ts index d678e485aa..18afaba238 100644 --- a/packages/playwright/src/worker/workerMain.ts +++ b/packages/playwright/src/worker/workerMain.ts @@ -23,14 +23,13 @@ import type { Suite, TestCase } from '../common/test'; import type { Annotation, FullConfigInternal, FullProjectInternal } from '../common/config'; import { FixtureRunner } from './fixtureRunner'; import { ManualPromise, gracefullyCloseAll, removeFolders } from 'playwright-core/lib/utils'; -import { TestInfoImpl } from './testInfo'; +import { TestInfoImpl, TestStage } from './testInfo'; import { ProcessRunner } from '../common/process'; import { loadTestFile } from '../common/testLoader'; import { applyRepeatEachIndex, bindFileSuiteToProject, filterTestsRemoveEmptySuites } from '../common/suiteUtils'; import { PoolBuilder } from '../common/poolBuilder'; import type { TestInfoError } from '../../types/test'; import type { Location } from '../../types/testReporter'; -import type { FixtureScope } from '../common/fixtures'; import { inheritFixutreNames } from '../common/fixtures'; export class WorkerMain extends ProcessRunner { @@ -144,29 +143,13 @@ export class WorkerMain extends ProcessRunner { } } - private async _teardownScope(scope: FixtureScope, testInfo: TestInfoImpl) { - const error = await this._teardownScopeAndReturnFirstError(scope, testInfo); - if (error) - throw error; - } - - private async _teardownScopeAndReturnFirstError(scope: FixtureScope, testInfo: TestInfoImpl): Promise { - let error: Error | undefined; - await this._fixtureRunner.teardownScope(scope, testInfo, e => { - testInfo._failWithError(e, true, false); - if (error === undefined) - error = e; - }); - return error; - } - private async _teardownScopes() { // TODO: separate timeout for teardown? const fakeTestInfo = new TestInfoImpl(this._config, this._project, this._params, undefined, 0, () => {}, () => {}, () => {}); - await fakeTestInfo._timeoutManager.withRunnable({ type: 'teardown' }, async () => { + await fakeTestInfo._runAsStage({ runnableType: 'teardown' }, async () => { await fakeTestInfo._runWithTimeout(async () => { - await this._teardownScopeAndReturnFirstError('test', fakeTestInfo); - await this._teardownScopeAndReturnFirstError('worker', fakeTestInfo); + await this._fixtureRunner.teardownScope('test', fakeTestInfo); + await this._fixtureRunner.teardownScope('worker', fakeTestInfo); }); }); this._fatalErrors.push(...fakeTestInfo.errors); @@ -323,128 +306,95 @@ export class WorkerMain extends ProcessRunner { this._lastRunningTests.push(test); if (this._lastRunningTests.length > 10) this._lastRunningTests.shift(); - let didFailBeforeAllForSuite: Suite | undefined; let shouldRunAfterEachHooks = false; await testInfo._runWithTimeout(async () => { - const traceError = await testInfo._runAndFailOnError(async () => { - // Ideally, "trace" would be an config-level option belonging to the - // test runner instead of a fixture belonging to Playwright. - // However, for backwards compatibility, we have to read it from a fixture today. - // We decided to not introduce the config-level option just yet. - const traceFixtureRegistration = test._pool!.resolve('trace'); - if (!traceFixtureRegistration) + await testInfo._runAsStage({ allowAndStopOnSkips: true, stopOnChildError: true }, async () => { + await testInfo._runAsStage({}, async () => { + // Ideally, "trace" would be an config-level option belonging to the + // test runner instead of a fixture belonging to Playwright. + // However, for backwards compatibility, we have to read it from a fixture today. + // We decided to not introduce the config-level option just yet. + const traceFixtureRegistration = test._pool!.resolve('trace'); + if (!traceFixtureRegistration) + return; + if (typeof traceFixtureRegistration.fn === 'function') + throw new Error(`"trace" option cannot be a function`); + await testInfo._tracing.startIfNeeded(traceFixtureRegistration.fn); + }); + + if (this._isStopped || isSkipped) { + // Two reasons to get here: + // - Last test is skipped, so we should not run the test, but run the cleanup. + // - Worker is requested to stop, but was not able to run full cleanup yet. + // We should skip the test, but run the cleanup. + testInfo.status = 'skipped'; return; - if (typeof traceFixtureRegistration.fn === 'function') - throw new Error(`"trace" option cannot be a function`); - await testInfo._tracing.startIfNeeded(traceFixtureRegistration.fn); - }); - if (traceError) - return; - - if (this._isStopped || isSkipped) { - // Two reasons to get here: - // - Last test is skipped, so we should not run the test, but run the cleanup. - // - Worker is requested to stop, but was not able to run full cleanup yet. - // We should skip the test, but run the cleanup. - testInfo.status = 'skipped'; - didFailBeforeAllForSuite = undefined; - return; - } - - await removeFolders([testInfo.outputDir]); - - let testFunctionParams: object | null = null; - await testInfo._runAsStep({ category: 'hook', title: 'Before Hooks' }, async step => { - // Run "beforeAll" hooks, unless already run during previous tests. - for (const suite of suites) { - didFailBeforeAllForSuite = suite; // Assume failure, unless reset below. - const beforeAllError = await this._runBeforeAllHooksForSuite(suite, testInfo); - if (beforeAllError) { - step.complete({ error: beforeAllError }); - return; - } - didFailBeforeAllForSuite = undefined; - if (testInfo.expectedStatus === 'skipped') - return; } - const beforeEachError = await testInfo._runAndFailOnError(async () => { + await removeFolders([testInfo.outputDir]); + + let testFunctionParams: object | null = null; + const beforeHooksStage: TestStage = { stepInfo: { category: 'hook', title: 'Before Hooks' }, stopOnChildError: true }; + await testInfo._runAsStage(beforeHooksStage, async () => { + // Run "beforeAll" hooks, unless already run during previous tests. + for (const suite of suites) + await this._runBeforeAllHooksForSuite(suite, testInfo); + // Run "beforeEach" hooks. Once started with "beforeEach", we must run all "afterEach" hooks as well. - shouldRunAfterEachHooks = true; + shouldRunAfterEachHooks = !beforeHooksStage.error && !beforeHooksStage.triggeredSkip; await this._runEachHooksForSuites(suites, 'beforeEach', testInfo); - }, 'allowSkips'); - if (beforeEachError) { - step.complete({ error: beforeEachError }); - return; - } - if (testInfo.expectedStatus === 'skipped') - return; - const fixturesError = await testInfo._runAndFailOnError(async () => { // Setup fixtures required by the test. testFunctionParams = await this._fixtureRunner.resolveParametersForFunction(test.fn, testInfo, 'test'); - }, 'allowSkips'); - if (fixturesError) - step.complete({ error: fixturesError }); + }); + + if (testFunctionParams === null) { + // Fixture setup failed or was skipped, we should not run the test now. + return; + } + + await testInfo._runAsStage({}, async () => { + // Now run the test itself. + debugTest(`test function started`); + const fn = test.fn; // Extract a variable to get a better stack trace ("myTest" vs "TestCase.myTest [as fn]"). + await fn(testFunctionParams, testInfo); + debugTest(`test function finished`); + }); }); - - if (testFunctionParams === null) { - // Fixture setup failed or was skipped, we should not run the test now. - return; - } - - await testInfo._runAndFailOnError(async () => { - // Now run the test itself. - debugTest(`test function started`); - const fn = test.fn; // Extract a variable to get a better stack trace ("myTest" vs "TestCase.myTest [as fn]"). - await fn(testFunctionParams, testInfo); - debugTest(`test function finished`); - }, 'allowSkips'); }); - if (didFailBeforeAllForSuite) { - // This will inform dispatcher that we should not run more tests from this group - // because we had a beforeAll error. - // This behavior avoids getting the same common error for each test. - this._skipRemainingTestsInSuite = didFailBeforeAllForSuite; - } + // Reset stages after a possible timeout. Note: we can remove this if we race each stage against the timeout. + testInfo._resetStages(); // A timed-out test gets a full additional timeout to run after hooks. const afterHooksSlot = testInfo._didTimeout ? { timeout: this._project.project.timeout, elapsed: 0 } : undefined; - await testInfo._runAsStepWithRunnable({ category: 'hook', title: 'After Hooks', runnableType: 'afterHooks', runnableSlot: afterHooksSlot, forceNoParent: true }, async step => { - let firstAfterHooksError: Error | undefined; + await testInfo._runAsStage({ stepInfo: { category: 'hook', title: 'After Hooks' }, runnableType: 'afterHooks', runnableSlot: afterHooksSlot }, async () => { await testInfo._runWithTimeout(async () => { // Note: do not wrap all teardown steps together, because failure in any of them // does not prevent further teardown steps from running. // Run "immediately upon test function finish" callback. debugTest(`on-test-function-finish callback started`); - const didFinishTestFunctionError = await testInfo._runAndFailOnError(async () => testInfo._onDidFinishTestFunction?.()); - firstAfterHooksError = firstAfterHooksError || didFinishTestFunctionError; + await testInfo._runAsStage({}, async () => testInfo._onDidFinishTestFunction?.()); debugTest(`on-test-function-finish callback finished`); // Run "afterEach" hooks, unless we failed at beforeAll stage. - if (shouldRunAfterEachHooks) { - const afterEachError = await testInfo._runAndFailOnError(() => this._runEachHooksForSuites(reversedSuites, 'afterEach', testInfo)); - firstAfterHooksError = firstAfterHooksError || afterEachError; - } + if (shouldRunAfterEachHooks) + await this._runEachHooksForSuites(reversedSuites, 'afterEach', testInfo); // Teardown test-scoped fixtures. Attribute to 'test' so that users understand // they should probably increase the test timeout to fix this issue. debugTest(`tearing down test scope started`); - const testScopeError = await this._teardownScopeAndReturnFirstError('test', testInfo); + await this._fixtureRunner.teardownScope('test', testInfo); debugTest(`tearing down test scope finished`); - firstAfterHooksError = firstAfterHooksError || testScopeError; // Run "afterAll" hooks for suites that are not shared with the next test. // In case of failure the worker will be stopped and we have to make sure that afterAll // hooks run before worker fixtures teardown. for (const suite of reversedSuites) { - if (!nextSuites.has(suite) || testInfo._isFailure()) { - const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo); - firstAfterHooksError = firstAfterHooksError || afterAllError; - } + if (!nextSuites.has(suite) || testInfo._isFailure()) + await this._runAfterAllHooksForSuite(suite, testInfo); } }); @@ -461,32 +411,25 @@ export class WorkerMain extends ProcessRunner { debugTest(`running full cleanup after the failure`); const teardownSlot = { timeout: this._project.project.timeout, elapsed: 0 }; - await testInfo._timeoutManager.withRunnable({ type: 'teardown', slot: teardownSlot }, async () => { + await testInfo._runAsStage({ runnableType: 'teardown', runnableSlot: teardownSlot }, async () => { // Attribute to 'test' so that users understand they should probably increate the test timeout to fix this issue. debugTest(`tearing down test scope started`); - const testScopeError = await this._teardownScopeAndReturnFirstError('test', testInfo); + await this._fixtureRunner.teardownScope('test', testInfo); debugTest(`tearing down test scope finished`); - firstAfterHooksError = firstAfterHooksError || testScopeError; - for (const suite of reversedSuites) { - const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo); - firstAfterHooksError = firstAfterHooksError || afterAllError; - } + for (const suite of reversedSuites) + await this._runAfterAllHooksForSuite(suite, testInfo); // Attribute to 'teardown' because worker fixtures are not perceived as a part of a test. debugTest(`tearing down worker scope started`); - const workerScopeError = await this._teardownScopeAndReturnFirstError('worker', testInfo); + await this._fixtureRunner.teardownScope('worker', testInfo); debugTest(`tearing down worker scope finished`); - firstAfterHooksError = firstAfterHooksError || workerScopeError; }); }); } - - if (firstAfterHooksError) - step.complete({ error: firstAfterHooksError }); }); - await testInfo._runAndFailOnError(async () => { + await testInfo._runAsStage({}, async () => { await testInfo._tracing.stopIfNeeded(); }); @@ -526,76 +469,69 @@ export class WorkerMain extends ProcessRunner { private async _runBeforeAllHooksForSuite(suite: Suite, testInfo: TestInfoImpl) { if (this._activeSuites.has(suite)) - return; + return 'success'; const extraAnnotations: Annotation[] = []; this._activeSuites.set(suite, extraAnnotations); return await this._runAllHooksForSuite(suite, testInfo, 'beforeAll', extraAnnotations); } private async _runAllHooksForSuite(suite: Suite, testInfo: TestInfoImpl, type: 'beforeAll' | 'afterAll', extraAnnotations?: Annotation[]) { - const allowSkips = type === 'beforeAll'; - let firstError: Error | undefined; - for (const hook of this._collectHooksAndModifiers(suite, type, testInfo)) { - debugTest(`${hook.type} hook at "${formatLocation(hook.location)}" started`); - const error = await testInfo._runAndFailOnError(async () => { + // Always run all the hooks, and capture the first error. + await testInfo._runAsStage({}, async () => { + for (const hook of this._collectHooksAndModifiers(suite, type, testInfo)) { + debugTest(`${hook.type} hook at "${formatLocation(hook.location)}" started`); // Separate time slot for each beforeAll/afterAll hook. const timeSlot = { timeout: this._project.project.timeout, elapsed: 0 }; - await testInfo._runAsStepWithRunnable({ - category: 'hook', - title: hook.title, - location: hook.location, + const stage: TestStage = { runnableType: hook.type, runnableSlot: timeSlot, - }, async () => { + stepInfo: { category: 'hook', title: hook.title }, + location: hook.location, + }; + await testInfo._runAsStage(stage, async () => { const existingAnnotations = new Set(testInfo.annotations); - try { - await this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'all-hooks-only'); - } finally { - if (extraAnnotations) { - // Inherit all annotations defined in the beforeAll/modifer to all tests in the suite. - const newAnnotations = testInfo.annotations.filter(a => !existingAnnotations.has(a)); - extraAnnotations.push(...newAnnotations); - } - // Each beforeAll/afterAll hook has its own scope for test fixtures. Attribute to the same runnable and timeSlot. - // Note: we must teardown even after hook fails, because we'll run more hooks. - await this._teardownScope('test', testInfo); + await this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'all-hooks-only'); + if (extraAnnotations) { + // Inherit all annotations defined in the beforeAll/modifer to all tests in the suite. + const newAnnotations = testInfo.annotations.filter(a => !existingAnnotations.has(a)); + extraAnnotations.push(...newAnnotations); } + // Each beforeAll/afterAll hook has its own scope for test fixtures. Attribute to the same runnable and timeSlot. + // Note: we must teardown even after hook fails, because we'll run more hooks. + await this._fixtureRunner.teardownScope('test', testInfo); }); - }, allowSkips ? 'allowSkips' : undefined); - firstError = firstError || error; - debugTest(`${hook.type} hook at "${formatLocation(hook.location)}" finished`); - // Skip inside a beforeAll hook/modifier prevents others from running. - if (allowSkips && testInfo.expectedStatus === 'skipped') - break; - } - return firstError; + if (stage.error && !this._skipRemainingTestsInSuite) { + // This will inform dispatcher that we should not run more tests from this group + // because we had a beforeAll error. + // This behavior avoids getting the same common error for each test. + this._skipRemainingTestsInSuite = suite; + } + debugTest(`${hook.type} hook at "${formatLocation(hook.location)}" finished`); + } + }); } - private async _runAfterAllHooksForSuite(suite: Suite, testInfo: TestInfoImpl): Promise { + private async _runAfterAllHooksForSuite(suite: Suite, testInfo: TestInfoImpl) { if (!this._activeSuites.has(suite)) - return; + return 'success'; this._activeSuites.delete(suite); return await this._runAllHooksForSuite(suite, testInfo, 'afterAll'); } private async _runEachHooksForSuites(suites: Suite[], type: 'beforeEach' | 'afterEach', testInfo: TestInfoImpl) { - const hooks = suites.map(suite => this._collectHooksAndModifiers(suite, type, testInfo)).flat(); - let error: Error | undefined; - for (const hook of hooks) { - try { - await testInfo._runAsStepWithRunnable({ - category: 'hook', - title: hook.title, - location: hook.location, + // Always run all the hooks, and capture the first error. + await testInfo._runAsStage({}, async () => { + const hooks = suites.map(suite => this._collectHooksAndModifiers(suite, type, testInfo)).flat(); + for (const hook of hooks) { + await testInfo._runAsStage({ runnableType: hook.type, - }, () => this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'test')); - } catch (e) { - // Always run all the hooks, and capture the first error. - error = error || e; + location: hook.location, + stepInfo: { category: 'hook', title: hook.title }, + }, async () => { + await this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'test'); + }); } - } - if (error) - throw error; + }); } } diff --git a/tests/playwright-test/playwright.trace.spec.ts b/tests/playwright-test/playwright.trace.spec.ts index 41ce802500..95450bf483 100644 --- a/tests/playwright-test/playwright.trace.spec.ts +++ b/tests/playwright-test/playwright.trace.spec.ts @@ -752,7 +752,7 @@ test('should not throw when screenshot on failure fails', async ({ runInlineTest expect(result.exitCode).toBe(0); expect(result.passed).toBe(1); const trace = await parseTrace(testInfo.outputPath('test-results', 'a-has-pdf-page', 'trace.zip')); - const attachedScreenshots = trace.actionTree.filter(s => s === ` attach "screenshot"`); + const attachedScreenshots = trace.actionTree.filter(s => s.trim() === `attach "screenshot"`); // One screenshot for the page, no screenshot for pdf page since it should have failed. expect(attachedScreenshots.length).toBe(1); });