diff --git a/packages/playwright-test/src/dispatcher.ts b/packages/playwright-test/src/dispatcher.ts index 7002130a63..a7fa3433c7 100644 --- a/packages/playwright-test/src/dispatcher.ts +++ b/packages/playwright-test/src/dispatcher.ts @@ -59,15 +59,8 @@ export class Dispatcher { this._queue = testGroups; for (const group of testGroups) { this._queueHashCount.set(group.workerHash, 1 + (this._queueHashCount.get(group.workerHash) || 0)); - for (const test of group.tests) { + for (const test of group.tests) this._testById.set(test._id, { test, resultByWorkerIndex: new Map() }); - for (let suite: Suite | undefined = test.parent; suite; suite = suite.parent) { - for (const hook of suite.hooks) { - if (!this._testById.has(hook._id)) - this._testById.set(hook._id, { test: hook, resultByWorkerIndex: new Map() }); - } - } - } } } @@ -184,25 +177,20 @@ export class Dispatcher { const remainingByTestId = new Map(testGroup.tests.map(e => [ e._id, e ])); const failedTestIds = new Set(); - let runningHookId: string | undefined; const onTestBegin = (params: TestBeginPayload) => { const data = this._testById.get(params.testId)!; - if (data.test._type !== 'test') - runningHookId = params.testId; if (this._hasReachedMaxFailures()) return; const result = data.test._appendTestResult(); data.resultByWorkerIndex.set(worker.workerIndex, { result, stepStack: new Set(), steps: new Map() }); result.workerIndex = worker.workerIndex; result.startTime = new Date(params.startWallTime); - if (data.test._type === 'test') - this._reporter.onTestBegin?.(data.test, result); + this._reporter.onTestBegin?.(data.test, result); }; worker.addListener('testBegin', onTestBegin); const onTestEnd = (params: TestEndPayload) => { - runningHookId = undefined; remainingByTestId.delete(params.testId); if (this._hasReachedMaxFailures()) return; @@ -224,7 +212,7 @@ export class Dispatcher { test.annotations = params.annotations; test.timeout = params.timeout; const isFailure = result.status !== 'skipped' && result.status !== test.expectedStatus; - if (isFailure && test._type === 'test') + if (isFailure) failedTestIds.add(params.testId); this._reportTestEnd(test, result); }; @@ -290,7 +278,7 @@ export class Dispatcher { // - there are no remaining // - we are here not because something failed // - no unrecoverable worker error - if (!remaining.length && !failedTestIds.size && !params.fatalErrors.length) { + if (!remaining.length && !failedTestIds.size && !params.fatalErrors.length && !params.skipRemaining) { if (this._isWorkerRedundant(worker)) worker.stop(); doneWithJob(); @@ -302,18 +290,8 @@ export class Dispatcher { // In case of fatal error, report first remaining test as failing with this error, // and all others as skipped. - if (params.fatalErrors.length) { - // Perhaps we were running a hook - report it as failed. - if (runningHookId) { - const data = this._testById.get(runningHookId)!; - const { result } = data.resultByWorkerIndex.get(worker.workerIndex)!; - result.errors = [...params.fatalErrors]; - result.error = result.errors[0]; - result.status = 'failed'; - this._reporter.onTestEnd?.(data.test, result); - } - - let first = true; + if (params.fatalErrors.length || params.skipRemaining) { + let shouldAddFatalErrorsToNextTest = params.fatalErrors.length > 0; for (const test of remaining) { if (this._hasReachedMaxFailures()) break; @@ -325,24 +303,23 @@ export class Dispatcher { result = runData.result; } else { result = data.test._appendTestResult(); - if (test._type === 'test') - this._reporter.onTestBegin?.(test, result); + this._reporter.onTestBegin?.(test, result); } - result.errors = [...params.fatalErrors]; + result.errors = shouldAddFatalErrorsToNextTest ? [...params.fatalErrors] : []; result.error = result.errors[0]; - result.status = first ? 'failed' : 'skipped'; + result.status = shouldAddFatalErrorsToNextTest ? 'failed' : 'skipped'; this._reportTestEnd(test, result); failedTestIds.add(test._id); - first = false; + shouldAddFatalErrorsToNextTest = false; } - if (first) { + if (shouldAddFatalErrorsToNextTest) { // We had a fatal error after all tests have passed - most likely in the afterAll hook. // Let's just fail the test run. this._hasWorkerErrors = true; for (const error of params.fatalErrors) this._reporter.onError?.(error); } - // Since we pretend that all remaining tests failed, there is nothing else to run, + // Since we pretend that all remaining tests failed/skipped, there is nothing else to run, // except for possible retries. remaining = []; } @@ -375,8 +352,7 @@ export class Dispatcher { // Emulate a "skipped" run, and drop this test from remaining. const result = test._appendTestResult(); - if (test._type === 'test') - this._reporter.onTestBegin?.(test, result); + this._reporter.onTestBegin?.(test, result); result.status = 'skipped'; this._reportTestEnd(test, result); return false; @@ -408,7 +384,7 @@ export class Dispatcher { worker.on('done', onDone); const onExit = (expectedly: boolean) => { - onDone({ fatalErrors: expectedly ? [] : [{ value: 'Worker process exited unexpectedly' }] }); + onDone({ skipRemaining: false, fatalErrors: expectedly ? [] : [{ value: 'Worker process exited unexpectedly' }] }); }; worker.on('exit', onExit); @@ -460,10 +436,9 @@ export class Dispatcher { } private _reportTestEnd(test: TestCase, result: TestResult) { - if (test._type === 'test' && result.status !== 'skipped' && result.status !== test.expectedStatus) + if (result.status !== 'skipped' && result.status !== test.expectedStatus) ++this._failureCount; - if (test._type === 'test') - this._reporter.onTestEnd?.(test, result); + this._reporter.onTestEnd?.(test, result); const maxFailures = this._loader.fullConfig().maxFailures; if (maxFailures && this._failureCount === maxFailures) this.stop().catch(e => {}); diff --git a/packages/playwright-test/src/fixtures.ts b/packages/playwright-test/src/fixtures.ts index f0886bbd8d..2bb6462711 100644 --- a/packages/playwright-test/src/fixtures.ts +++ b/packages/playwright-test/src/fixtures.ts @@ -51,7 +51,7 @@ class Fixture { this.value = null; } - async setup(workerInfo: WorkerInfo, testInfo: TestInfo | undefined) { + async setup(testInfo: TestInfo) { if (typeof this.registration.fn !== 'function') { this.value = this.registration.fn; return; @@ -60,7 +60,7 @@ class Fixture { const params: { [key: string]: any } = {}; for (const name of this.registration.deps) { const registration = this.runner.pool!.resolveDependency(this.registration, name)!; - const dep = await this.runner.setupFixtureForRegistration(registration, workerInfo, testInfo); + const dep = await this.runner.setupFixtureForRegistration(registration, testInfo); dep.usages.add(this); params[name] = dep.value; } @@ -77,6 +77,7 @@ class Fixture { useFuncStarted.resolve(); await this._useFuncFinished; }; + const workerInfo: WorkerInfo = { config: testInfo.config, parallelIndex: testInfo.parallelIndex, workerIndex: testInfo.workerIndex, project: testInfo.project }; const info = this.registration.scope === 'worker' ? workerInfo : testInfo; this._selfTeardownComplete = Promise.resolve().then(() => this.registration.fn(params, useFunc, info)).catch((e: any) => { if (!useFuncStarted.isDone()) @@ -261,12 +262,12 @@ export class FixtureRunner { throw error; } - async resolveParametersForFunction(fn: Function, workerInfo: WorkerInfo, testInfo: TestInfo | undefined): Promise { + async resolveParametersForFunction(fn: Function, testInfo: TestInfo): Promise { // Install all automatic fixtures. for (const registration of this.pool!.registrations.values()) { const shouldSkip = !testInfo && registration.scope === 'test'; if (registration.auto && !shouldSkip) - await this.setupFixtureForRegistration(registration, workerInfo, testInfo); + await this.setupFixtureForRegistration(registration, testInfo); } // Install used fixtures. @@ -274,18 +275,18 @@ export class FixtureRunner { const params: { [key: string]: any } = {}; for (const name of names) { const registration = this.pool!.registrations.get(name)!; - const fixture = await this.setupFixtureForRegistration(registration, workerInfo, testInfo); + const fixture = await this.setupFixtureForRegistration(registration, testInfo); params[name] = fixture.value; } return params; } - async resolveParametersAndRunFunction(fn: Function, workerInfo: WorkerInfo, testInfo: TestInfo | undefined) { - const params = await this.resolveParametersForFunction(fn, workerInfo, testInfo); - return fn(params, testInfo || workerInfo); + async resolveParametersAndRunFunction(fn: Function, testInfo: TestInfo) { + const params = await this.resolveParametersForFunction(fn, testInfo); + return fn(params, testInfo); } - async setupFixtureForRegistration(registration: FixtureRegistration, workerInfo: WorkerInfo, testInfo: TestInfo | undefined): Promise { + async setupFixtureForRegistration(registration: FixtureRegistration, testInfo: TestInfo): Promise { if (registration.scope === 'test') this.testScopeClean = false; @@ -295,7 +296,7 @@ export class FixtureRunner { fixture = new Fixture(this, registration); this.instanceForId.set(registration.id, fixture); - await fixture.setup(workerInfo, testInfo); + await fixture.setup(testInfo); return fixture; } diff --git a/packages/playwright-test/src/index.ts b/packages/playwright-test/src/index.ts index eb8f46b976..fc772b95a4 100644 --- a/packages/playwright-test/src/index.ts +++ b/packages/playwright-test/src/index.ts @@ -423,7 +423,7 @@ export const test = _baseTest.extend({ })); // 7. Cleanup created contexts when we know it's safe - this will produce nice error message. - if (hookType(testInfo) === 'beforeAll' && testInfo.status === 'timedOut') { + if (testInfo.status === 'timedOut' && testInfo.errors.some(error => error.message?.match(/Timeout of \d+ms exceeded in beforeAll hook./))) { const anyContext = leftoverContexts[0]; const pendingCalls = anyContext ? formatPendingCalls((anyContext as any)._connection.pendingProtocolCalls()) : ''; await Promise.all(leftoverContexts.filter(c => createdContexts.has(c)).map(c => c.close())); @@ -519,9 +519,10 @@ function formatStackFrame(frame: StackFrame) { } function hookType(testInfo: TestInfo): 'beforeAll' | 'afterAll' | undefined { - if (testInfo.title.startsWith('beforeAll')) + const impl = testInfo as import('./testInfo').TestInfoImpl; + if (impl._currentRunnable?.type === 'beforeAll') return 'beforeAll'; - if (testInfo.title.startsWith('afterAll')) + if (impl._currentRunnable?.type === 'afterAll') return 'afterAll'; } diff --git a/packages/playwright-test/src/ipc.ts b/packages/playwright-test/src/ipc.ts index 55e1830c0b..91d49ab332 100644 --- a/packages/playwright-test/src/ipc.ts +++ b/packages/playwright-test/src/ipc.ts @@ -76,6 +76,7 @@ export type RunPayload = { export type DonePayload = { fatalErrors: TestError[]; + skipRemaining: boolean; }; export type TestOutputPayload = { diff --git a/packages/playwright-test/src/project.ts b/packages/playwright-test/src/project.ts index 50df921218..c9c871d4be 100644 --- a/packages/playwright-test/src/project.ts +++ b/packages/playwright-test/src/project.ts @@ -52,10 +52,8 @@ export class ProjectImpl { for (const parent of parents) { if (parent._use.length) pool = new FixturePool(parent._use, pool, parent._isDescribe); - for (const hook of parent._eachHooks) + for (const hook of parent._hooks) pool.validateFunction(hook.fn, hook.type + ' hook', hook.location); - for (const hook of parent.hooks) - pool.validateFunction(hook.fn, hook._type + ' hook', hook.location); for (const modifier of parent._modifiers) pool.validateFunction(modifier.fn, modifier.type + ' modifier', modifier.location); } @@ -94,15 +92,6 @@ export class ProjectImpl { } if (!to._entries.length) return false; - for (const hook of from.hooks) { - const clone = hook._clone(); - clone.retries = 1; - clone._pool = this.buildPool(hook); - clone._projectIndex = this.index; - clone._id = `${hook._ordinalInFile}@${hook._requireFile}#run${this.index}-repeat${repeatEachIndex}`; - clone.repeatEachIndex = repeatEachIndex; - to._addAllHook(clone); - } return true; } diff --git a/packages/playwright-test/src/reporters/line.ts b/packages/playwright-test/src/reporters/line.ts index 4b089e2232..f5d1083f4c 100644 --- a/packages/playwright-test/src/reporters/line.ts +++ b/packages/playwright-test/src/reporters/line.ts @@ -61,8 +61,7 @@ class LineReporter extends BaseReporter { override onTestEnd(test: TestCase, result: TestResult) { super.onTestEnd(test, result); - if (!test.title.startsWith('beforeAll') && !test.title.startsWith('afterAll')) - ++this._current; + ++this._current; const retriesSuffix = this.totalTestCount < this._current ? ` (retries)` : ``; const title = `[${this._current}/${this.totalTestCount}]${retriesSuffix} ${formatTestTitle(this.config, test)}`; const suffix = result.retry ? ` (retry #${result.retry})` : ''; diff --git a/packages/playwright-test/src/test.ts b/packages/playwright-test/src/test.ts index 121b1d6f7b..39f1985e47 100644 --- a/packages/playwright-test/src/test.ts +++ b/packages/playwright-test/src/test.ts @@ -17,7 +17,7 @@ import type { FixturePool } from './fixtures'; import * as reporterTypes from '../types/testReporter'; import type { TestTypeImpl } from './testType'; -import { Annotations, FixturesWithLocation, Location, TestCaseType } from './types'; +import { Annotation, FixturesWithLocation, Location } from './types'; import { FullProject } from './types'; class Base { @@ -45,10 +45,9 @@ export class Suite extends Base implements reporterTypes.Suite { _use: FixturesWithLocation[] = []; _isDescribe = false; _entries: (Suite | TestCase)[] = []; - hooks: TestCase[] = []; - _eachHooks: { type: 'beforeEach' | 'afterEach', fn: Function, location: Location }[] = []; + _hooks: { type: 'beforeEach' | 'afterEach' | 'beforeAll' | 'afterAll', fn: Function, location: Location }[] = []; _timeout: number | undefined; - _annotations: Annotations = []; + _annotations: Annotation[] = []; _modifiers: Modifier[] = []; _parallelMode: 'default' | 'serial' | 'parallel' = 'default'; _projectConfig: FullProject | undefined; @@ -66,11 +65,6 @@ export class Suite extends Base implements reporterTypes.Suite { this._entries.push(suite); } - _addAllHook(hook: TestCase) { - hook.parent = this; - this.hooks.push(hook); - } - allTests(): TestCase[] { const result: TestCase[] = []; const visit = (suite: Suite) => { @@ -107,7 +101,7 @@ export class Suite extends Base implements reporterTypes.Suite { suite.location = this.location; suite._requireFile = this._requireFile; suite._use = this._use.slice(); - suite._eachHooks = this._eachHooks.slice(); + suite._hooks = this._hooks.slice(); suite._timeout = this._timeout; suite._annotations = this._annotations.slice(); suite._modifiers = this._modifiers.slice(); @@ -130,11 +124,10 @@ export class TestCase extends Base implements reporterTypes.TestCase { expectedStatus: reporterTypes.TestStatus = 'passed'; timeout = 0; - annotations: Annotations = []; + annotations: Annotation[] = []; retries = 0; repeatEachIndex = 0; - _type: TestCaseType; _ordinalInFile: number; _testType: TestTypeImpl; _id = ''; @@ -142,9 +135,8 @@ export class TestCase extends Base implements reporterTypes.TestCase { _pool: FixturePool | undefined; _projectIndex = 0; - constructor(type: TestCaseType, title: string, fn: Function, ordinalInFile: number, testType: TestTypeImpl, location: Location) { + constructor(title: string, fn: Function, ordinalInFile: number, testType: TestTypeImpl, location: Location) { super(title); - this._type = type; this.fn = fn; this._ordinalInFile = ordinalInFile; this._testType = testType; @@ -174,7 +166,7 @@ export class TestCase extends Base implements reporterTypes.TestCase { } _clone(): TestCase { - const test = new TestCase(this._type, this.title, this.fn, this._ordinalInFile, this._testType, this.location); + const test = new TestCase(this.title, this.fn, this._ordinalInFile, this._testType, this.location); test._only = this._only; test._requireFile = this._requireFile; test.expectedStatus = this.expectedStatus; diff --git a/packages/playwright-test/src/testInfo.ts b/packages/playwright-test/src/testInfo.ts index 5a00f5eaf1..399ca57f00 100644 --- a/packages/playwright-test/src/testInfo.ts +++ b/packages/playwright-test/src/testInfo.ts @@ -25,8 +25,13 @@ import { WorkerInitParams } from './ipc'; import { Loader } from './loader'; import { ProjectImpl } from './project'; import { TestCase } from './test'; -import { Annotations, TestStepInternal } from './types'; -import { addSuffixToFilePath, formatLocation, getContainedPath, monotonicTime, sanitizeForFilePath, serializeError, trimLongString } from './util'; +import { Annotation, TestStepInternal, Location } from './types'; +import { addSuffixToFilePath, getContainedPath, monotonicTime, sanitizeForFilePath, serializeError, trimLongString } from './util'; + +type RunnableDescription = { + type: 'test' | 'beforeAll' | 'afterAll' | 'beforeEach' | 'afterEach' | 'slow' | 'skip' | 'fail' | 'fixme' | 'teardown'; + location?: Location; +}; export class TestInfoImpl implements TestInfo { private _projectImpl: ProjectImpl; @@ -36,6 +41,7 @@ export class TestInfoImpl implements TestInfo { readonly _startTime: number; readonly _startWallTime: number; private _hasHardError: boolean = false; + _currentRunnable: RunnableDescription | undefined; // ------------ TestInfo fields ------------ readonly repeatEachIndex: number; @@ -52,7 +58,7 @@ export class TestInfoImpl implements TestInfo { readonly fn: Function; expectedStatus: TestStatus; duration: number = 0; - readonly annotations: Annotations = []; + readonly annotations: Annotation[] = []; readonly attachments: TestInfo['attachments'] = []; status: TestStatus = 'passed'; readonly stdout: TestInfo['stdout'] = []; @@ -116,7 +122,7 @@ export class TestInfoImpl implements TestInfo { const relativeTestFilePath = path.relative(this.project.testDir, test._requireFile.replace(/\.(spec|test)\.(js|ts|mjs)$/, '')); const sanitizedRelativePath = relativeTestFilePath.replace(process.platform === 'win32' ? new RegExp('\\\\', 'g') : new RegExp('/', 'g'), '-'); - const fullTitleWithoutSpec = test.titlePath().slice(1).join(' ') + (test._type === 'test' ? '' : '-worker' + this.workerIndex); + const fullTitleWithoutSpec = test.titlePath().slice(1).join(' '); let testOutputDir = trimLongString(sanitizedRelativePath + '-' + sanitizeForFilePath(fullTitleWithoutSpec)); if (uniqueProjectNamePathSegment) @@ -170,13 +176,15 @@ export class TestInfoImpl implements TestInfo { // Do not overwrite existing failure upon hook/teardown timeout. if (this.status === 'passed') { this.status = 'timedOut'; - if (this._test._type === 'test') { - this.errors.push({ message: colors.red(`Timeout of ${this.timeout}ms exceeded.`) }); - } else { - // Include location for the hook to distinguish between multiple hooks. - const message = colors.red(`Timeout of ${this.timeout}ms exceeded in ${this._test._type} hook.`); - this.errors.push({ message: message, stack: message + `\n at ${formatLocation(this._test.location)}.` }); - } + const title = titleForRunnable(this._currentRunnable); + const suffix = title ? ` in ${title}` : ''; + const message = colors.red(`Timeout of ${this.timeout}ms exceeded${suffix}.`); + const location = this._currentRunnable?.location; + this.errors.push({ + message, + // Include location for hooks and modifiers to distinguish between them. + stack: location ? message + `\n at ${location.file}:${location.line}:${location.column}` : undefined, + }); } } this.duration = monotonicTime() - this._startTime; @@ -282,3 +290,24 @@ export class TestInfoImpl implements TestInfo { class SkipError extends Error { } + +function titleForRunnable(runnable: RunnableDescription | undefined): string { + if (!runnable) + return ''; + switch (runnable.type) { + case 'test': + return ''; + case 'beforeAll': + case 'beforeEach': + case 'afterAll': + case 'afterEach': + return runnable.type + ' hook'; + case 'teardown': + return 'fixtures teardown'; + case 'skip': + case 'slow': + case 'fixme': + case 'fail': + return runnable.type + ' modifier'; + } +} diff --git a/packages/playwright-test/src/testType.ts b/packages/playwright-test/src/testType.ts index 720decb19f..5277343187 100644 --- a/packages/playwright-test/src/testType.ts +++ b/packages/playwright-test/src/testType.ts @@ -81,7 +81,7 @@ export class TestTypeImpl { private _createTest(type: 'default' | 'only' | 'skip' | 'fixme', location: Location, title: string, fn: Function) { throwIfRunningInsideJest(); const suite = this._ensureCurrentSuite(location, 'test()'); - const test = new TestCase('test', title, fn, nextOrdinalInFile(suite._requireFile), this, location); + const test = new TestCase(title, fn, nextOrdinalInFile(suite._requireFile), this, location); test._requireFile = suite._requireFile; suite._addTest(test); @@ -130,15 +130,7 @@ export class TestTypeImpl { private _hook(name: 'beforeEach' | 'afterEach' | 'beforeAll' | 'afterAll', location: Location, fn: Function) { const suite = this._ensureCurrentSuite(location, `test.${name}()`); - if (name === 'beforeAll' || name === 'afterAll') { - const sameTypeCount = suite.hooks.filter(hook => hook._type === name).length; - const suffix = sameTypeCount ? String(sameTypeCount) : ''; - const hook = new TestCase(name, name + suffix, fn, nextOrdinalInFile(suite._requireFile), this, location); - hook._requireFile = suite._requireFile; - suite._addAllHook(hook); - } else { - suite._eachHooks.push({ type: name, fn, location }); - } + suite._hooks.push({ type: name, fn, location }); } private _configure(location: Location, options: { mode?: 'parallel' | 'serial' }) { diff --git a/packages/playwright-test/src/types.ts b/packages/playwright-test/src/types.ts index bcb8a6f91a..0e1838fa5a 100644 --- a/packages/playwright-test/src/types.ts +++ b/packages/playwright-test/src/types.ts @@ -23,7 +23,7 @@ export type FixturesWithLocation = { fixtures: Fixtures; location: Location; }; -export type Annotations = { type: string, description?: string }[]; +export type Annotation = { type: string, description?: string }; export interface TestStepInternal { complete(error?: Error | TestError): void; @@ -33,5 +33,3 @@ export interface TestStepInternal { forceNoParent: boolean; location?: Location; } - -export type TestCaseType = 'beforeAll' | 'afterAll' | 'test'; diff --git a/packages/playwright-test/src/worker.ts b/packages/playwright-test/src/worker.ts index 4e4e8e00a6..7928e0966f 100644 --- a/packages/playwright-test/src/worker.ts +++ b/packages/playwright-test/src/worker.ts @@ -84,7 +84,7 @@ process.on('message', async message => { } if (message.method === 'run') { const runPayload = message.params as RunPayload; - await workerRunner!.run(runPayload); + await workerRunner!.runTestGroup(runPayload); } }); diff --git a/packages/playwright-test/src/workerRunner.ts b/packages/playwright-test/src/workerRunner.ts index 19b39884c1..d1ad385f61 100644 --- a/packages/playwright-test/src/workerRunner.ts +++ b/packages/playwright-test/src/workerRunner.ts @@ -18,15 +18,15 @@ import rimraf from 'rimraf'; import util from 'util'; import colors from 'colors/safe'; import { EventEmitter } from 'events'; -import { serializeError, formatLocation } from './util'; -import { TestBeginPayload, TestEndPayload, RunPayload, TestEntry, DonePayload, WorkerInitParams, StepBeginPayload, StepEndPayload, TeardownErrorsPayload } from './ipc'; +import { serializeError } from './util'; +import { TestBeginPayload, TestEndPayload, RunPayload, DonePayload, WorkerInitParams, StepBeginPayload, StepEndPayload, TeardownErrorsPayload } from './ipc'; import { setCurrentTestInfo } from './globals'; import { Loader } from './loader'; -import { Modifier, Suite, TestCase } from './test'; -import { Annotations, TestError, TestInfo, TestStepInternal, WorkerInfo } from './types'; +import { Suite, TestCase } from './test'; +import { Annotation, TestError, TestStepInternal } from './types'; import { ProjectImpl } from './project'; import { FixtureRunner } from './fixtures'; -import { raceAgainstTimeout } from 'playwright-core/lib/utils/async'; +import { ManualPromise, raceAgainstTimeout } from 'playwright-core/lib/utils/async'; import { TestInfoImpl } from './testInfo'; const removeFolderAsync = util.promisify(rimraf); @@ -35,15 +35,25 @@ export class WorkerRunner extends EventEmitter { private _params: WorkerInitParams; private _loader!: Loader; private _project!: ProjectImpl; - private _workerInfo!: WorkerInfo; private _fixtureRunner: FixtureRunner; - private _failedTest: TestInfoImpl | undefined; + // Accumulated fatal errors that cannot be attributed to a test. private _fatalErrors: TestError[] = []; - private _entries = new Map(); + // Whether we should skip running remaining tests in the group because + // of a setup error, usually beforeAll hook. + private _skipRemainingTests = false; + // The stage of the full cleanup. Once "finished", we can safely stop running anything. + private _didRunFullCleanup = false; + // Whether the worker was requested to stop. private _isStopped = false; - private _runFinished = Promise.resolve(); + // This promise resolves once the single "run test group" call finishes. + private _runFinished = new ManualPromise(); _currentTest: TestInfoImpl | null = null; + // Dynamic annotations originated by modifiers with a callback, e.g. `test.skip(() => true)`. + private _extraSuiteAnnotations = new Map(); + // Suites that had their beforeAll hooks, but not afterAll hooks executed. + // These suites still need afterAll hooks to be executed for the proper cleanup. + private _activeSuites = new Set(); constructor(params: WorkerInitParams) { super(); @@ -100,7 +110,7 @@ export class WorkerRunner extends EventEmitter { const isExpectError = (error instanceof Error) && !!(error as any).matcherResult; const isCurrentTestExpectedToFail = this._currentTest?.expectedStatus === 'failed'; const shouldConsiderAsTestError = isExpectError || !isCurrentTestExpectedToFail; - if (this._currentTest && this._currentTest._test._type === 'test' && shouldConsiderAsTestError) { + if (this._currentTest && shouldConsiderAsTestError) { this._currentTest._failWithError(serializeError(error), true /* isHardError */); } else { // No current test - fatal error. @@ -116,102 +126,43 @@ export class WorkerRunner extends EventEmitter { this._loader = await Loader.deserialize(this._params.loader); this._project = this._loader.projects()[this._params.projectIndex]; - - this._workerInfo = { - workerIndex: this._params.workerIndex, - parallelIndex: this._params.parallelIndex, - project: this._project.config, - config: this._loader.fullConfig(), - }; } - async run(runPayload: RunPayload) { - let runFinishedCallback = () => {}; - this._runFinished = new Promise(f => runFinishedCallback = f); + async runTestGroup(runPayload: RunPayload) { + this._runFinished = new ManualPromise(); try { - this._entries = new Map(runPayload.entries.map(e => [ e.testId, e ])); + const entries = new Map(runPayload.entries.map(e => [ e.testId, e ])); await this._loadIfNeeded(); const fileSuite = await this._loader.loadTestFile(runPayload.file, 'worker'); const suite = this._project.cloneFileSuite(fileSuite, this._params.repeatEachIndex, test => { - if (!this._entries.has(test._id)) + if (!entries.has(test._id)) return false; return true; }); if (suite) { - const firstPool = suite.allTests()[0]._pool!; - this._fixtureRunner.setPool(firstPool); - await this._runSuite(suite, []); + this._extraSuiteAnnotations = new Map(); + this._activeSuites = new Set(); + this._didRunFullCleanup = false; + const tests = suite.allTests().filter(test => entries.has(test._id)); + for (let i = 0; i < tests.length; i++) + await this._runTest(tests[i], entries.get(tests[i]._id)!.retry, tests[i + 1]); } - if (this._failedTest) - await this._teardownScopes(); } catch (e) { // In theory, we should run above code without any errors. // However, in the case we screwed up, or loadTestFile failed in the worker // but not in the runner, let's do a fatal error. this.unhandledError(e); } finally { - if (this._failedTest) { - // Now that we did run all hooks and teared down scopes, we can - // report the failure, possibly with any error details revealed by teardown. - this.emit('testEnd', buildTestEndPayload(this._failedTest)); - } this._reportDone(); - runFinishedCallback(); + this._runFinished.resolve(); } } - private async _runSuite(suite: Suite, annotations: Annotations) { - // When stopped, do not run a suite. But if we have started running the suite with hooks, - // always finish the hooks. - if (this._isStopped) + private async _runTest(test: TestCase, retry: number, nextTest: TestCase | undefined) { + // Do not run tests after full cleanup, because we are entirely done. + if (this._isStopped && this._didRunFullCleanup) return; - annotations = annotations.concat(suite._annotations); - const allSkipped = suite.allTests().every(test => { - const runEntry = this._entries.get(test._id); - return !runEntry || test.expectedStatus === 'skipped'; - }); - if (allSkipped) { - // This avoids running beforeAll/afterAll hooks. - annotations.push({ type: 'skip' }); - } - - for (const beforeAllModifier of suite._modifiers) { - if (!this._fixtureRunner.dependsOnWorkerFixturesOnly(beforeAllModifier.fn, beforeAllModifier.location)) - continue; - // TODO: separate timeout for beforeAll modifiers? - const result = await raceAgainstTimeout(() => this._fixtureRunner.resolveParametersAndRunFunction(beforeAllModifier.fn, this._workerInfo, undefined), this._project.config.timeout); - if (result.timedOut) { - this._fatalErrors.push(serializeError(new Error(`Timeout of ${this._project.config.timeout}ms exceeded while running ${beforeAllModifier.type} modifier\n at ${formatLocation(beforeAllModifier.location)}`))); - this.stop(); - } else if (!!result.result) { - annotations.push({ type: beforeAllModifier.type, description: beforeAllModifier.description }); - } - } - - for (const hook of suite.hooks) { - if (hook._type !== 'beforeAll') - continue; - const firstTest = suite.allTests()[0]; - await this._runTestOrAllHook(hook, annotations, this._entries.get(firstTest._id)?.retry || 0); - } - for (const entry of suite._entries) { - if (entry instanceof Suite) { - await this._runSuite(entry, annotations); - } else { - const runEntry = this._entries.get(entry._id); - if (runEntry && !this._isStopped) - await this._runTestOrAllHook(entry, annotations, runEntry.retry); - } - } - for (const hook of suite.hooks) { - if (hook._type !== 'afterAll') - continue; - await this._runTestOrAllHook(hook, annotations, 0); - } - } - - private async _runTestOrAllHook(test: TestCase, annotations: Annotations, retry: number) { let lastStepId = 0; const testInfo = new TestInfoImpl(this._loader, this._params, test, retry, data => { const stepId = `${data.category}@${data.title}@${++lastStepId}`; @@ -247,16 +198,7 @@ export class WorkerRunner extends EventEmitter { return step; }); - // Inherit test.setTimeout() from parent suites. - for (let suite: Suite | undefined = test.parent; suite; suite = suite.parent) { - if (suite._timeout !== undefined) { - testInfo.setTimeout(suite._timeout); - break; - } - } - - // Process annotations defined on parent suites. - for (const annotation of annotations) { + const processAnnotation = (annotation: Annotation) => { testInfo.annotations.push(annotation); switch (annotation.type) { case 'fixme': @@ -271,11 +213,35 @@ export class WorkerRunner extends EventEmitter { testInfo.setTimeout(testInfo.timeout * 3); break; } + }; + + if (!this._isStopped) { + // Update the fixture pool - it may differ between tests, but only in test-scoped fixtures. + this._fixtureRunner.setPool(test._pool!); + } + + const suites = getSuites(test); + const reversedSuites = suites.slice().reverse(); + + // Inherit test.setTimeout() from parent suites, deepest has the priority. + for (const suite of reversedSuites) { + if (suite._timeout !== undefined) { + testInfo.setTimeout(suite._timeout); + break; + } + } + + // Process existing annotations defined on parent suites. + for (const suite of suites) { + for (const annotation of suite._annotations) + processAnnotation(annotation); + const extraAnnotations = this._extraSuiteAnnotations.get(suite) || []; + for (const annotation of extraAnnotations) + processAnnotation(annotation); } this._currentTest = testInfo; setCurrentTestInfo(testInfo); - this.emit('testBegin', buildTestBeginPayload(testInfo)); if (testInfo.expectedStatus === 'skipped') { @@ -284,98 +250,201 @@ export class WorkerRunner extends EventEmitter { return; } - // Update the fixture pool - it may differ between tests, but only in test-scoped fixtures. - this._fixtureRunner.setPool(test._pool!); + // Assume beforeAll failed until we actually finish it successfully. + let didFailBeforeAll = true; + let shouldRunAfterEachHooks = false; - await testInfo._runWithTimeout(() => this._runTestWithBeforeHooks(test, testInfo)); + await testInfo._runWithTimeout(async () => { + if (this._isStopped) { + // Getting here means that worker is requested to stop, but was not able to + // run full cleanup yet. Skip the test, but run the cleanup. + testInfo.status = 'skipped'; + didFailBeforeAll = false; + return; + } + + const beforeHooksStep = testInfo._addStep({ + category: 'hook', + title: 'Before Hooks', + canHaveChildren: true, + forceNoParent: true + }); + + // Note: wrap all preparation steps together, because failure in any of them + // prevents further setup and/or test from running. + const maybeError = await testInfo._runFn(async () => { + // Run "beforeAll" modifiers on parent suites, unless already run during previous tests. + for (const suite of suites) { + if (this._extraSuiteAnnotations.has(suite)) + continue; + const extraAnnotations: Annotation[] = []; + this._extraSuiteAnnotations.set(suite, extraAnnotations); + await this._runModifiersForSuite(suite, testInfo, 'worker', extraAnnotations); + } + + // Run "beforeAll" hooks, unless already run during previous tests. + for (const suite of suites) + await this._runBeforeAllHooksForSuite(suite, testInfo); + // Running "beforeAll" succeeded! + didFailBeforeAll = false; + + // Run "beforeEach" modifiers. + for (const suite of suites) + await this._runModifiersForSuite(suite, testInfo, 'test'); + + // Run "beforeEach" hooks. Once started with "beforeEach", we must run all "afterEach" hooks as well. + shouldRunAfterEachHooks = true; + await this._runEachHooksForSuites(suites, 'beforeEach', testInfo); + + // Setup fixtures required by the test. + testInfo._currentRunnable = { type: 'test' }; + const params = await this._fixtureRunner.resolveParametersForFunction(test.fn, testInfo); + beforeHooksStep.complete(); // Report fixture hooks step as completed. + + // Now run the test itself. + const fn = test.fn; // Extract a variable to get a better stack trace ("myTest" vs "TestCase.myTest [as fn]"). + await fn(params, testInfo); + }, 'allowSkips'); + + beforeHooksStep.complete(maybeError); // Second complete is a no-op. + }); + + if (didFailBeforeAll) { + // 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._skipRemainingTests = true; + } + + const afterHooksStep = testInfo._addStep({ + category: 'hook', + title: 'After Hooks', + canHaveChildren: true, + forceNoParent: true + }); + let firstAfterHooksError: TestError | undefined; if (testInfo.status === 'timedOut') { // A timed-out test gets a full additional timeout to run after hooks. testInfo._timeoutRunner.resetTimeout(testInfo.timeout); } - await testInfo._runWithTimeout(() => this._runAfterHooks(test, testInfo)); + 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. - this._currentTest = null; - setCurrentTestInfo(null); + // Run "afterEach" hooks, unless we failed at beforeAll stage. + if (shouldRunAfterEachHooks) { + const afterEachError = await testInfo._runFn(() => this._runEachHooksForSuites(reversedSuites, 'afterEach', testInfo)); + firstAfterHooksError = firstAfterHooksError || afterEachError; + } + + // Run "afterAll" hooks for suites that are not shared with the next test. + const nextSuites = new Set(getSuites(nextTest)); + for (const suite of reversedSuites) { + if (!nextSuites.has(suite)) { + const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo); + firstAfterHooksError = firstAfterHooksError || afterAllError; + } + } + + // Teardown test-scoped fixtures. + testInfo._currentRunnable = { type: 'teardown' }; + const testScopeError = await testInfo._runFn(() => this._fixtureRunner.teardownScope('test')); + firstAfterHooksError = firstAfterHooksError || testScopeError; + }); const isFailure = testInfo.status !== 'skipped' && testInfo.status !== testInfo.expectedStatus; - if (isFailure) { - // Delay reporting testEnd result until after teardownScopes is done. - this._failedTest = testInfo; - if (test._type !== 'test') { - // beforeAll/afterAll hook failure skips any remaining tests in the worker. - this._fatalErrors.push(...testInfo.errors); - } - this.stop(); - } else { - this.emit('testEnd', buildTestEndPayload(testInfo)); + if (isFailure) + this._isStopped = true; + + if (this._isStopped) { + // Run all remaining "afterAll" hooks and teardown all fixtures when worker is shutting down. + // Mark as "cleaned up" early to avoid running cleanup twice. + this._didRunFullCleanup = true; + + // Give it more time for the full cleanup. + testInfo._timeoutRunner.resetTimeout(this._project.config.timeout); + await testInfo._runWithTimeout(async () => { + for (const suite of reversedSuites) { + const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo); + firstAfterHooksError = firstAfterHooksError || afterAllError; + } + testInfo._currentRunnable = { type: 'teardown' }; + const testScopeError = await testInfo._runFn(() => this._fixtureRunner.teardownScope('test')); + firstAfterHooksError = firstAfterHooksError || testScopeError; + const workerScopeError = await testInfo._runFn(() => this._fixtureRunner.teardownScope('worker')); + firstAfterHooksError = firstAfterHooksError || workerScopeError; + }); } + afterHooksStep.complete(firstAfterHooksError); + this._currentTest = null; + setCurrentTestInfo(null); + this.emit('testEnd', buildTestEndPayload(testInfo)); + const preserveOutput = this._loader.fullConfig().preserveOutput === 'always' || (this._loader.fullConfig().preserveOutput === 'failures-only' && isFailure); if (!preserveOutput) await removeFolderAsync(testInfo.outputDir).catch(e => {}); } - private async _runTestWithBeforeHooks(test: TestCase, testInfo: TestInfoImpl) { - const step = testInfo._addStep({ - category: 'hook', - title: 'Before Hooks', - canHaveChildren: true, - forceNoParent: true - }); - const maybeError = await testInfo._runFn(async () => { - if (test._type === 'test') { - const beforeEachModifiers: Modifier[] = []; - for (let s: Suite | undefined = test.parent; s; s = s.parent) { - const modifiers = s._modifiers.filter(modifier => !this._fixtureRunner.dependsOnWorkerFixturesOnly(modifier.fn, modifier.location)); - beforeEachModifiers.push(...modifiers.reverse()); - } - beforeEachModifiers.reverse(); - for (const modifier of beforeEachModifiers) { - const result = await this._fixtureRunner.resolveParametersAndRunFunction(modifier.fn, this._workerInfo, testInfo); - testInfo[modifier.type](!!result, modifier.description!); - } - await this._runHooks(test.parent!, 'beforeEach', testInfo); - } - - const params = await this._fixtureRunner.resolveParametersForFunction(test.fn, this._workerInfo, testInfo); - step.complete(); // Report fixture hooks step as completed. - const fn = test.fn; // Extract a variable to get a better stack trace ("myTest" vs "TestCase.myTest [as fn]"). - await fn(params, testInfo); - }, 'allowSkips'); - step.complete(maybeError); // Second complete is a no-op. - } - - private async _runAfterHooks(test: TestCase, testInfo: TestInfoImpl) { - const step = testInfo._addStep({ - category: 'hook', - title: 'After Hooks', - canHaveChildren: true, - forceNoParent: true - }); - - let teardownError1: TestError | undefined; - if (test._type === 'test') - teardownError1 = await testInfo._runFn(() => this._runHooks(test.parent!, 'afterEach', testInfo)); - // Continue teardown even after the failure. - - const teardownError2 = await testInfo._runFn(() => this._fixtureRunner.teardownScope('test')); - step.complete(teardownError1 || teardownError2); - } - - private async _runHooks(suite: Suite, type: 'beforeEach' | 'afterEach', testInfo: TestInfo) { - const all = []; - for (let s: Suite | undefined = suite; s; s = s.parent) { - const funcs = s._eachHooks.filter(e => e.type === type).map(e => e.fn); - all.push(...funcs.reverse()); + private async _runModifiersForSuite(suite: Suite, testInfo: TestInfoImpl, scope: 'worker' | 'test', extraAnnotations?: Annotation[]) { + for (const modifier of suite._modifiers) { + const actualScope = this._fixtureRunner.dependsOnWorkerFixturesOnly(modifier.fn, modifier.location) ? 'worker' : 'test'; + if (actualScope !== scope) + continue; + testInfo._currentRunnable = { type: modifier.type, location: modifier.location }; + const result = await this._fixtureRunner.resolveParametersAndRunFunction(modifier.fn, testInfo); + if (result && extraAnnotations) + extraAnnotations.push({ type: modifier.type, description: modifier.description }); + testInfo[modifier.type](!!result, modifier.description); } - if (type === 'beforeEach') - all.reverse(); - let error: Error | undefined; - for (const hook of all) { + } + + private async _runBeforeAllHooksForSuite(suite: Suite, testInfo: TestInfoImpl) { + if (this._activeSuites.has(suite)) + return; + this._activeSuites.add(suite); + let beforeAllError: Error | undefined; + for (const hook of suite._hooks) { + if (hook.type !== 'beforeAll') + continue; try { - await this._fixtureRunner.resolveParametersAndRunFunction(hook, this._workerInfo, testInfo); + testInfo._currentRunnable = { type: 'beforeAll', location: hook.location }; + await this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo); + } catch (e) { + // Always run all the hooks, and capture the first error. + beforeAllError = beforeAllError || e; + } + } + if (beforeAllError) + throw beforeAllError; + } + + private async _runAfterAllHooksForSuite(suite: Suite, testInfo: TestInfoImpl) { + if (!this._activeSuites.has(suite)) + return; + this._activeSuites.delete(suite); + let firstError: TestError | undefined; + for (const hook of suite._hooks) { + if (hook.type !== 'afterAll') + continue; + const afterAllError = await testInfo._runFn(async () => { + testInfo._currentRunnable = { type: 'afterAll', location: hook.location }; + await this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo); + }); + firstError = firstError || afterAllError; + } + return firstError; + } + + private async _runEachHooksForSuites(suites: Suite[], type: 'beforeEach' | 'afterEach', testInfo: TestInfoImpl) { + const hooks = suites.map(suite => suite._hooks.filter(hook => hook.type === type)).flat(); + let error: Error | undefined; + for (const hook of hooks) { + try { + testInfo._currentRunnable = { type, location: hook.location }; + await this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo); } catch (e) { // Always run all the hooks, and capture the first error. error = error || e; @@ -386,10 +455,10 @@ export class WorkerRunner extends EventEmitter { } private _reportDone() { - const donePayload: DonePayload = { fatalErrors: this._fatalErrors }; + const donePayload: DonePayload = { fatalErrors: this._fatalErrors, skipRemaining: this._skipRemainingTests }; this.emit('done', donePayload); this._fatalErrors = []; - this._failedTest = undefined; + this._skipRemainingTests = false; } } @@ -417,3 +486,11 @@ function buildTestEndPayload(testInfo: TestInfoImpl): TestEndPayload { })) }; } + +function getSuites(test: TestCase | undefined): Suite[] { + const suites: Suite[] = []; + for (let suite: Suite | undefined = test?.parent; suite; suite = suite.parent) + suites.push(suite); + suites.reverse(); // Put root suite first. + return suites; +} diff --git a/tests/playwright-test/fixture-errors.spec.ts b/tests/playwright-test/fixture-errors.spec.ts index 03ba3d5ea4..a0b457299b 100644 --- a/tests/playwright-test/fixture-errors.spec.ts +++ b/tests/playwright-test/fixture-errors.spec.ts @@ -453,7 +453,26 @@ test('should not report fixture teardown error twice', async ({ runInlineTest }) expect(result.failed).toBe(1); expect(result.output).toContain('Error: Oh my error'); expect(stripAnsi(result.output)).toContain(`throw new Error('Oh my error')`); - expect(countTimes(result.output, 'Oh my error')).toBe(2); + expect(countTimes(stripAnsi(result.output), 'Oh my error')).toBe(2); +}); + +test('should not report fixture teardown timeout twice', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.ts': ` + const test = pwt.test.extend({ + fixture: async ({ }, use) => { + await use(); + await new Promise(() => {}); + }, + }); + test('good', async ({ fixture }) => { + }); + `, + }, { reporter: 'list', timeout: 1000 }); + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + expect(result.output).toContain('while shutting down environment'); + expect(countTimes(result.output, 'while shutting down environment')).toBe(1); }); test('should handle fixture teardown error after test timeout and continue', async ({ runInlineTest }) => { diff --git a/tests/playwright-test/fixtures.spec.ts b/tests/playwright-test/fixtures.spec.ts index 59c8cd1bb7..4f93658d16 100644 --- a/tests/playwright-test/fixtures.spec.ts +++ b/tests/playwright-test/fixtures.spec.ts @@ -316,23 +316,23 @@ test('automatic fixtures should work', async ({ runInlineTest }) => { }); test.beforeEach(async ({}) => { expect(counterWorker).toBe(1); - expect(counterTest === 2 || counterTest === 3).toBe(true); + expect(counterTest === 1 || counterTest === 2).toBe(true); }); test('test 1', async ({}) => { expect(counterWorker).toBe(1); - expect(counterTest).toBe(2); + expect(counterTest).toBe(1); }); test('test 2', async ({}) => { expect(counterWorker).toBe(1); - expect(counterTest).toBe(3); + expect(counterTest).toBe(2); }); test.afterEach(async ({}) => { expect(counterWorker).toBe(1); - expect(counterTest === 2 || counterTest === 3).toBe(true); + expect(counterTest === 1 || counterTest === 2).toBe(true); }); test.afterAll(async ({}) => { expect(counterWorker).toBe(1); - expect(counterTest).toBe(4); + expect(counterTest).toBe(2); }); ` }); diff --git a/tests/playwright-test/hooks.spec.ts b/tests/playwright-test/hooks.spec.ts index de1d31c06c..55c0595f1c 100644 --- a/tests/playwright-test/hooks.spec.ts +++ b/tests/playwright-test/hooks.spec.ts @@ -63,14 +63,10 @@ test('hooks should work with fixtures', async ({ runInlineTest }) => { '+w', '+t', 'beforeAll-17-42', - '-t', - '+t', - 'beforeEach-17-43', - 'test-17-43', - 'afterEach-17-43', - '-t', - '+t', - 'afterAll-17-44', + 'beforeEach-17-42', + 'test-17-42', + 'afterEach-17-42', + 'afterAll-17-42', '-t', '+t', ]); @@ -95,13 +91,13 @@ test('afterEach failure should not prevent other hooks and fixtures teardown', a 'a.test.js': ` const { test } = require('./helper'); test.describe('suite', () => { - test.afterEach(async () => { - console.log('afterEach1'); - }); test.afterEach(async () => { console.log('afterEach2'); throw new Error('afterEach2'); }); + test.afterEach(async () => { + console.log('afterEach1'); + }); test('one', async ({foo}) => { console.log('test'); expect(true).toBe(true); @@ -313,7 +309,7 @@ test('beforeAll hook should get retry index of the first test', async ({ runInli ]); }); -test('afterAll exception should fail the run', async ({ runInlineTest }) => { +test('afterAll exception should fail the test', async ({ runInlineTest }) => { const result = await runInlineTest({ 'a.test.js': ` const { test } = pwt; @@ -325,7 +321,8 @@ test('afterAll exception should fail the run', async ({ runInlineTest }) => { `, }); expect(result.exitCode).toBe(1); - expect(result.passed).toBe(1); + expect(result.passed).toBe(0); + expect(result.failed).toBe(1); expect(result.output).toContain('From the afterAll'); }); @@ -370,13 +367,17 @@ test('beforeAll failure should prevent the test, but not afterAll', async ({ run test.afterAll(() => { console.log('\\n%%afterAll'); }); + test('failed', () => { + console.log('\\n%%test1'); + }); test('skipped', () => { - console.log('\\n%%test'); + console.log('\\n%%test2'); }); `, }); expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); + expect(result.skipped).toBe(1); expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ '%%beforeAll', '%%afterAll', @@ -454,7 +455,7 @@ test('afterAll error should not mask beforeAll', async ({ runInlineTest }) => { expect(result.output).toContain('from beforeAll'); }); -test('beforeAll timeout should be reported', async ({ runInlineTest }) => { +test('beforeAll timeout should be reported and prevent more tests', async ({ runInlineTest }) => { const result = await runInlineTest({ 'a.test.js': ` const { test } = pwt; @@ -465,41 +466,61 @@ test('beforeAll timeout should be reported', async ({ runInlineTest }) => { test.afterAll(() => { console.log('\\n%%afterAll'); }); + test('failed', () => { + console.log('\\n%%test1'); + }); test('skipped', () => { - console.log('\\n%%test'); + console.log('\\n%%test2'); }); `, }, { timeout: 1000 }); expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); + expect(result.skipped).toBe(1); expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ '%%beforeAll', '%%afterAll', ]); expect(result.output).toContain('Timeout of 1000ms exceeded in beforeAll hook.'); + expect(result.output).toContain(`a.test.js:6:12`); + expect(stripAnsi(result.output)).toContain(`> 6 | test.beforeAll(async () => {`); }); -test('afterAll timeout should be reported', async ({ runInlineTest }, testInfo) => { +test('afterAll timeout should be reported, run other afterAll hooks, and continue testing', async ({ runInlineTest }, testInfo) => { const result = await runInlineTest({ 'a.test.js': ` const { test } = pwt; - test.afterAll(async () => { - console.log('\\n%%afterAll'); - await new Promise(f => setTimeout(f, 5000)); + test.describe('suite', () => { + test.afterAll(async () => { + console.log('\\n%%afterAll1'); + await new Promise(f => setTimeout(f, 5000)); + }); + test('runs', () => { + console.log('\\n%%test1'); + }); }); - test('runs', () => { - console.log('\\n%%test'); + test.afterAll(async () => { + console.log('\\n%%afterAll2'); + }); + test('does not run', () => { + console.log('\\n%%test2'); }); `, }, { timeout: 1000 }); expect(result.exitCode).toBe(1); expect(result.passed).toBe(1); + expect(result.failed).toBe(1); + expect(result.skipped).toBe(0); expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ - '%%test', - '%%afterAll', + '%%test1', + '%%afterAll1', + '%%afterAll2', + '%%test2', + '%%afterAll2', ]); expect(result.output).toContain('Timeout of 1000ms exceeded in afterAll hook.'); - expect(result.output).toContain(`at a.test.js:6:12`); + expect(result.output).toContain(`a.test.js:7:14`); + expect(stripAnsi(result.output)).toContain(`> 7 | test.afterAll(async () => {`); }); test('beforeAll and afterAll timeouts at the same time should be reported', async ({ runInlineTest }) => { @@ -606,13 +627,46 @@ test('should not hang and report results when worker process suddenly exits duri const result = await runInlineTest({ 'a.spec.js': ` const { test } = pwt; - test('passed', () => {}); + test('failing due to afterall', () => {}); test.afterAll(() => { process.exit(0); }); ` }, { reporter: 'line' }); expect(result.exitCode).toBe(1); - expect(result.passed).toBe(1); + expect(result.passed).toBe(0); + expect(result.failed).toBe(1); expect(result.output).toContain('Worker process exited unexpectedly'); - expect(stripAnsi(result.output)).toContain('[1/1] a.spec.js:6:7 › passed'); - expect(stripAnsi(result.output)).toContain('[1/1] a.spec.js:7:12 › afterAll'); + expect(stripAnsi(result.output)).toContain('[1/1] a.spec.js:6:7 › failing due to afterall'); +}); + +test('unhandled rejection during beforeAll should be reported and prevent more tests', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.test.js': ` + const { test } = pwt; + test.beforeAll(async () => { + console.log('\\n%%beforeAll'); + Promise.resolve().then(() => { + throw new Error('Oh my'); + }); + await new Promise(f => setTimeout(f, 100)); + }); + test.afterAll(() => { + console.log('\\n%%afterAll'); + }); + test('failed', () => { + console.log('\\n%%test1'); + }); + test('skipped', () => { + console.log('\\n%%test2'); + }); + `, + }); + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + expect(result.skipped).toBe(1); + expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ + '%%beforeAll', + '%%afterAll', + ]); + expect(result.output).toContain('Error: Oh my'); + expect(stripAnsi(result.output)).toContain(`> 9 | throw new Error('Oh my');`); }); diff --git a/tests/playwright-test/playwright.artifacts.spec.ts b/tests/playwright-test/playwright.artifacts.spec.ts index a48788ed56..080c4dc92d 100644 --- a/tests/playwright-test/playwright.artifacts.spec.ts +++ b/tests/playwright-test/playwright.artifacts.spec.ts @@ -46,6 +46,7 @@ const testFiles = { }); test.afterAll(async () => { + await page.setContent('Reset!'); await page.close(); }); @@ -145,10 +146,6 @@ test('should work with screenshot: on', async ({ runInlineTest }, testInfo) => { ' test-failed-1.png', 'artifacts-persistent-passing', ' test-finished-1.png', - 'artifacts-shared-afterAll-worker0', - ' test-finished-1.png', - 'artifacts-shared-beforeAll-worker0', - ' test-finished-1.png', 'artifacts-shared-shared-failing', ' test-failed-1.png', 'artifacts-shared-shared-passing', @@ -214,10 +211,6 @@ test('should work with trace: on', async ({ runInlineTest }, testInfo) => { ' trace.zip', 'artifacts-persistent-passing', ' trace.zip', - 'artifacts-shared-afterAll-worker0', - ' trace.zip', - 'artifacts-shared-beforeAll-worker0', - ' trace.zip', 'artifacts-shared-shared-failing', ' trace.zip', 'artifacts-shared-shared-passing', @@ -277,8 +270,6 @@ test('should work with trace: on-first-retry', async ({ runInlineTest }, testInf ' trace.zip', 'artifacts-persistent-failing-retry1', ' trace.zip', - 'artifacts-shared-beforeAll-worker1-retry1', - ' trace.zip', 'artifacts-shared-shared-failing-retry1', ' trace.zip', 'artifacts-two-contexts-failing-retry1', diff --git a/tests/playwright-test/reporter.spec.ts b/tests/playwright-test/reporter.spec.ts index f1fc09bca3..68a830039b 100644 --- a/tests/playwright-test/reporter.spec.ts +++ b/tests/playwright-test/reporter.spec.ts @@ -343,15 +343,11 @@ test('should report api steps', async ({ runInlineTest }) => { `%% end {\"title\":\"browserContext.close\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"After Hooks\",\"category\":\"hook\",\"steps\":[{\"title\":\"apiRequestContext.dispose\",\"category\":\"pw:api\"},{\"title\":\"browserContext.close\",\"category\":\"pw:api\"}]}`, `%% begin {\"title\":\"Before Hooks\",\"category\":\"hook\"}`, - `%% end {\"title\":\"Before Hooks\",\"category\":\"hook\"}`, `%% begin {\"title\":\"browser.newPage\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"browser.newPage\",\"category\":\"pw:api\"}`, `%% begin {\"title\":\"page.setContent\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"page.setContent\",\"category\":\"pw:api\"}`, - `%% begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`, - `%% end {\"title\":\"After Hooks\",\"category\":\"hook\"}`, - `%% begin {\"title\":\"Before Hooks\",\"category\":\"hook\"}`, - `%% end {\"title\":\"Before Hooks\",\"category\":\"hook\"}`, + `%% end {\"title\":\"Before Hooks\",\"category\":\"hook\",\"steps\":[{\"title\":\"browser.newPage\",\"category\":\"pw:api\"},{\"title\":\"page.setContent\",\"category\":\"pw:api\"}]}`, `%% begin {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, `%% begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`, @@ -361,13 +357,9 @@ test('should report api steps', async ({ runInlineTest }) => { `%% begin {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, `%% begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`, - `%% end {\"title\":\"After Hooks\",\"category\":\"hook\"}`, - `%% begin {\"title\":\"Before Hooks\",\"category\":\"hook\"}`, - `%% end {\"title\":\"Before Hooks\",\"category\":\"hook\"}`, `%% begin {\"title\":\"page.close\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"page.close\",\"category\":\"pw:api\"}`, - `%% begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`, - `%% end {\"title\":\"After Hooks\",\"category\":\"hook\"}`, + `%% end {\"title\":\"After Hooks\",\"category\":\"hook\",\"steps\":[{\"title\":\"page.close\",\"category\":\"pw:api\"}]}`, ]); }); diff --git a/tests/playwright-test/test-modifiers.spec.ts b/tests/playwright-test/test-modifiers.spec.ts index 8c2af4deea..d85b93c79b 100644 --- a/tests/playwright-test/test-modifiers.spec.ts +++ b/tests/playwright-test/test-modifiers.spec.ts @@ -330,6 +330,38 @@ test('modifier timeout should be reported', async ({ runInlineTest }) => { }, { timeout: 2000 }); expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); - expect(result.output).toContain('Error: Timeout of 2000ms exceeded while running skip modifier'); + expect(result.output).toContain('Timeout of 2000ms exceeded in skip modifier.'); expect(stripAnsi(result.output)).toContain('6 | test.skip(async () => new Promise(() => {}));'); }); + +test('should not run hooks if modifier throws', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.test.ts': ` + const { test } = pwt; + test.skip(() => { + console.log('%%modifier'); + throw new Error('Oh my'); + }); + test.beforeAll(() => { + console.log('%%beforeEach'); + }); + test.beforeEach(() => { + console.log('%%beforeEach'); + }); + test.afterEach(() => { + console.log('%%afterEach'); + }); + test.afterAll(() => { + console.log('%%beforeEach'); + }); + test('skipped1', () => { + console.log('%%skipped1'); + }); + `, + }); + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ + '%%modifier', + ]); +}); diff --git a/tests/playwright-test/test-output-dir.spec.ts b/tests/playwright-test/test-output-dir.spec.ts index 0c8f976408..0e2a887b15 100644 --- a/tests/playwright-test/test-output-dir.spec.ts +++ b/tests/playwright-test/test-output-dir.spec.ts @@ -83,53 +83,27 @@ test('should include repeat token', async ({ runInlineTest }) => { expect(result.passed).toBe(3); }); -test('should be unique for beforeAll and afterAll hooks', async ({ runInlineTest }, testInfo) => { +test('should be unique for beforeAll hook from different workers', async ({ runInlineTest }, testInfo) => { const result = await runInlineTest({ 'a.spec.js': ` const { test } = pwt; test.beforeAll(({}, testInfo) => { console.log('\\n%%' + testInfo.outputDir); }); - test.beforeAll(({}, testInfo) => { - console.log('\\n%%' + testInfo.outputDir); + test('fails', ({}, testInfo) => { + expect(1).toBe(2); }); - test.afterAll(({}, testInfo) => { - console.log('\\n%%' + testInfo.outputDir); - }); - test.afterAll(({}, testInfo) => { - console.log('\\n%%' + testInfo.outputDir); - }); - test.describe('suite', () => { - test.beforeAll(({}, testInfo) => { - console.log('\\n%%' + testInfo.outputDir); - }); - test.afterAll(({}, testInfo) => { - console.log('\\n%%' + testInfo.outputDir); - }); - test('fails', ({}, testInfo) => { - expect(1).toBe(2); - }); - test('passes', ({}, testInfo) => { - }); + test('passes', ({}, testInfo) => { }); ` - }); + }, { retries: '1' }); expect(result.exitCode).toBe(1); expect(result.passed).toBe(1); expect(result.failed).toBe(1); expect(result.output.split('\n').filter(x => x.startsWith('%%'))).toEqual([ - `%%${testInfo.outputPath('test-results', 'a-beforeAll-worker0')}`, - `%%${testInfo.outputPath('test-results', 'a-beforeAll1-worker0')}`, - `%%${testInfo.outputPath('test-results', 'a-suite-beforeAll-worker0')}`, - `%%${testInfo.outputPath('test-results', 'a-suite-afterAll-worker0')}`, - `%%${testInfo.outputPath('test-results', 'a-afterAll-worker0')}`, - `%%${testInfo.outputPath('test-results', 'a-afterAll1-worker0')}`, - `%%${testInfo.outputPath('test-results', 'a-beforeAll-worker1')}`, - `%%${testInfo.outputPath('test-results', 'a-beforeAll1-worker1')}`, - `%%${testInfo.outputPath('test-results', 'a-suite-beforeAll-worker1')}`, - `%%${testInfo.outputPath('test-results', 'a-suite-afterAll-worker1')}`, - `%%${testInfo.outputPath('test-results', 'a-afterAll-worker1')}`, - `%%${testInfo.outputPath('test-results', 'a-afterAll1-worker1')}`, + `%%${testInfo.outputPath('test-results', 'a-fails')}`, + `%%${testInfo.outputPath('test-results', 'a-fails-retry1')}`, + `%%${testInfo.outputPath('test-results', 'a-passes')}`, ]); });