From 0d54afab9c5e4bafff185c7ea7f8fff0013bf59a Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 15 Dec 2021 10:39:49 -0800 Subject: [PATCH] feat(test runner): show beforeAll/afterAll hooks similar to tests (#10923) Reporters now get notified about hooks start/end/steps. --- docs/src/test-reporter-api/class-reporter.md | 12 ++--- docs/src/test-reporter-api/class-suite.md | 5 ++ packages/html-reporter/src/reportView.tsx | 2 +- packages/html-reporter/src/testFileView.tsx | 2 +- packages/playwright-test/src/dispatcher.ts | 28 ++++++++-- packages/playwright-test/src/project.ts | 7 ++- .../playwright-test/src/reporters/html.ts | 26 ++++++--- .../playwright-test/src/reporters/json.ts | 2 + .../playwright-test/src/reporters/line.ts | 3 +- packages/playwright-test/src/reporters/raw.ts | 2 + packages/playwright-test/src/test.ts | 10 ++-- packages/playwright-test/src/testType.ts | 17 +++--- packages/playwright-test/src/types.ts | 2 + packages/playwright-test/src/workerRunner.ts | 42 +++++++-------- .../playwright-test/types/testReporter.d.ts | 17 +++--- tests/playwright-test/hooks.spec.ts | 54 +++++++++++++++++-- tests/playwright-test/reporter-html.spec.ts | 35 ++++++++++++ tests/playwright-test/reporter.spec.ts | 14 +++++ tests/playwright-test/retry.spec.ts | 2 +- tests/playwright-test/stdio.spec.ts | 15 +++++- .../overrides-testReporter.d.ts | 1 + 21 files changed, 227 insertions(+), 71 deletions(-) diff --git a/docs/src/test-reporter-api/class-reporter.md b/docs/src/test-reporter-api/class-reporter.md index 4bc25e4e57..6e50bf30ff 100644 --- a/docs/src/test-reporter-api/class-reporter.md +++ b/docs/src/test-reporter-api/class-reporter.md @@ -144,7 +144,7 @@ Output chunk. ### param: Reporter.onStdErr.test - `test` <[void]|[TestCase]> -Test that was running. Note that output may happen when to test is running, in which case this will be [void]. +Test that was running. Note that output may happen when no test is running, in which case this will be [void]. ### param: Reporter.onStdErr.result - `result` <[void]|[TestResult]> @@ -164,7 +164,7 @@ Output chunk. ### param: Reporter.onStdOut.test - `test` <[void]|[TestCase]> -Test that was running. Note that output may happen when to test is running, in which case this will be [void]. +Test that was running. Note that output may happen when no test is running, in which case this will be [void]. ### param: Reporter.onStdOut.result - `result` <[void]|[TestResult]> @@ -178,7 +178,7 @@ Called when a test step started in the worker process. ### param: Reporter.onStepBegin.test - `test` <[TestCase]> -Test that has been started. +Test that the step belongs to. ### param: Reporter.onStepBegin.result - `result` <[TestResult]> @@ -188,7 +188,7 @@ Result of the test run, this object gets populated while the test runs. ### param: Reporter.onStepBegin.step - `step` <[TestStep]> -Test step instance. +Test step instance that has started. ## method: Reporter.onStepEnd @@ -197,7 +197,7 @@ Called when a test step finished in the worker process. ### param: Reporter.onStepEnd.test - `test` <[TestCase]> -Test that has been finished. +Test that the step belongs to. ### param: Reporter.onStepEnd.result - `result` <[TestResult]> @@ -207,7 +207,7 @@ Result of the test run. ### param: Reporter.onStepEnd.step - `step` <[TestStep]> -Test step instance. +Test step instance that has finished. ## method: Reporter.onTestBegin diff --git a/docs/src/test-reporter-api/class-suite.md b/docs/src/test-reporter-api/class-suite.md index b0b3d74c9e..510be9a59f 100644 --- a/docs/src/test-reporter-api/class-suite.md +++ b/docs/src/test-reporter-api/class-suite.md @@ -24,6 +24,11 @@ Reporter is given a root suite in the [`method: Reporter.onBegin`] method. Returns the list of all test cases in this suite and its descendants, as opposite to [`property: Suite.tests`]. +## property: Suite.hooks +- type: <[Array]<[TestCase]>> + +`beforeAll` and `afterAll` hooks in the suite. Note that other hooks such as `beforeEach` and `afterEach` are reported as the steps within the test. + ## property: Suite.location - type: <[void]|[Location]> diff --git a/packages/html-reporter/src/reportView.tsx b/packages/html-reporter/src/reportView.tsx index 6cdb20d432..8a28e61236 100644 --- a/packages/html-reporter/src/reportView.tsx +++ b/packages/html-reporter/src/reportView.tsx @@ -72,7 +72,7 @@ const TestCaseViewLoader: React.FC<{ if (!fileId) return; const file = await report.entry(`${fileId}.json`) as TestFile; - for (const t of file.tests) { + for (const t of [...file.tests, ...file.hooks]) { if (t.testId === testId) { setTest(t); break; diff --git a/packages/html-reporter/src/testFileView.tsx b/packages/html-reporter/src/testFileView.tsx index b969a52deb..cf87210903 100644 --- a/packages/html-reporter/src/testFileView.tsx +++ b/packages/html-reporter/src/testFileView.tsx @@ -38,7 +38,7 @@ export const TestFileView: React.FC<{ {msToString(file.stats.duration)} {file.fileName} }> - {file.tests.filter(t => filter.matches(t)).map(test => + {[...file.tests, ...file.hooks].filter(t => filter.matches(t)).map(test =>
{msToString(test.duration)} {report.projectNames.length > 1 && !!test.projectName && diff --git a/packages/playwright-test/src/dispatcher.ts b/packages/playwright-test/src/dispatcher.ts index 95905d72e1..366b38c586 100644 --- a/packages/playwright-test/src/dispatcher.ts +++ b/packages/playwright-test/src/dispatcher.ts @@ -57,8 +57,15 @@ export class Dispatcher { this._reporter = reporter; this._queue = testGroups; for (const group of testGroups) { - 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() }); + } + } + } } } @@ -165,11 +172,14 @@ 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 data = this._testById.get(params.testId)!; const result = data.test._appendTestResult(); data.resultByWorkerIndex.set(worker.workerIndex, { result, stepStack: new Set(), steps: new Map() }); result.workerIndex = worker.workerIndex; @@ -179,6 +189,7 @@ export class Dispatcher { worker.addListener('testBegin', onTestBegin); const onTestEnd = (params: TestEndPayload) => { + runningHookId = undefined; remainingByTestId.delete(params.testId); if (this._hasReachedMaxFailures()) return; @@ -199,7 +210,7 @@ export class Dispatcher { test.annotations = params.annotations; test.timeout = params.timeout; const isFailure = result.status !== 'skipped' && result.status !== test.expectedStatus; - if (isFailure) + if (isFailure && test._type === 'test') failedTestIds.add(params.testId); this._reportTestEnd(test, result); }; @@ -276,6 +287,15 @@ export class Dispatcher { // In case of fatal error, report first remaining test as failing with this error, // and all others as skipped. if (params.fatalError) { + // 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.error = params.fatalError; + result.status = 'failed'; + this._reporter.onTestEnd?.(data.test, result); + } + let first = true; for (const test of remaining) { if (this._hasReachedMaxFailures()) @@ -418,7 +438,7 @@ export class Dispatcher { } private _reportTestEnd(test: TestCase, result: TestResult) { - if (result.status !== 'skipped' && result.status !== test.expectedStatus) + if (test._type === 'test' && result.status !== 'skipped' && result.status !== test.expectedStatus) ++this._failureCount; this._reporter.onTestEnd?.(test, result); const maxFailures = this._loader.fullConfig().maxFailures; diff --git a/packages/playwright-test/src/project.ts b/packages/playwright-test/src/project.ts index a2a2749973..0a4a7ac536 100644 --- a/packages/playwright-test/src/project.ts +++ b/packages/playwright-test/src/project.ts @@ -54,7 +54,7 @@ export class ProjectImpl { pool = new FixturePool(parent._use, pool, parent._isDescribe); for (const hook of parent._eachHooks) pool.validateFunction(hook.fn, hook.type + ' hook', hook.location); - for (const hook of parent._allHooks) + 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,10 +94,13 @@ export class ProjectImpl { } if (!to._entries.length) return false; - for (const hook of from._allHooks) { + 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/html.ts b/packages/playwright-test/src/reporters/html.ts index 27381a8665..47fc3316f8 100644 --- a/packages/playwright-test/src/reporters/html.ts +++ b/packages/playwright-test/src/reporters/html.ts @@ -53,12 +53,14 @@ export type TestFile = { fileId: string; fileName: string; tests: TestCase[]; + hooks: TestCase[]; }; export type TestFileSummary = { fileId: string; fileName: string; tests: TestCaseSummary[]; + hooks: TestCaseSummary[]; stats: Stats; }; @@ -237,18 +239,23 @@ class HtmlBuilder { let fileEntry = data.get(fileId); if (!fileEntry) { fileEntry = { - testFile: { fileId, fileName, tests: [] }, - testFileSummary: { fileId, fileName, tests: [], stats: emptyStats() }, + testFile: { fileId, fileName, tests: [], hooks: [] }, + testFileSummary: { fileId, fileName, tests: [], hooks: [], stats: emptyStats() }, }; data.set(fileId, fileEntry); } const { testFile, testFileSummary } = fileEntry; const testEntries: TestEntry[] = []; - this._processJsonSuite(file, fileId, projectJson.project.name, [], testEntries); + const hookEntries: TestEntry[] = []; + this._processJsonSuite(file, fileId, projectJson.project.name, [], testEntries, hookEntries); for (const test of testEntries) { testFile.tests.push(test.testCase); testFileSummary.tests.push(test.testCaseSummary); } + for (const hook of hookEntries) { + testFile.hooks.push(hook.testCase); + testFileSummary.hooks.push(hook.testCaseSummary); + } } } @@ -271,13 +278,15 @@ class HtmlBuilder { if (!stats.ok) ok = false; - testFileSummary.tests.sort((t1, t2) => { + const testCaseSummaryComparator = (t1: TestCaseSummary, t2: TestCaseSummary) => { const w1 = (t1.outcome === 'unexpected' ? 1000 : 0) + (t1.outcome === 'flaky' ? 1 : 0); const w2 = (t2.outcome === 'unexpected' ? 1000 : 0) + (t2.outcome === 'flaky' ? 1 : 0); if (w2 - w1) return w2 - w1; return t1.location.line - t2.location.line; - }); + }; + testFileSummary.tests.sort(testCaseSummaryComparator); + testFileSummary.hooks.sort(testCaseSummaryComparator); this._addDataFile(fileId + '.json', testFile); } @@ -335,10 +344,11 @@ class HtmlBuilder { this._dataZipFile.addBuffer(Buffer.from(JSON.stringify(data)), fileName); } - private _processJsonSuite(suite: JsonSuite, fileId: string, projectName: string, path: string[], out: TestEntry[]) { + private _processJsonSuite(suite: JsonSuite, fileId: string, projectName: string, path: string[], outTests: TestEntry[], outHooks: TestEntry[]) { const newPath = [...path, suite.title]; - suite.suites.map(s => this._processJsonSuite(s, fileId, projectName, newPath, out)); - suite.tests.forEach(t => out.push(this._createTestEntry(t, projectName, newPath))); + suite.suites.map(s => this._processJsonSuite(s, fileId, projectName, newPath, outTests, outHooks)); + suite.tests.forEach(t => outTests.push(this._createTestEntry(t, projectName, newPath))); + suite.hooks.forEach(t => outHooks.push(this._createTestEntry(t, projectName, newPath))); } private _createTestEntry(test: JsonTestCase, projectName: string, path: string[]): TestEntry { diff --git a/packages/playwright-test/src/reporters/json.ts b/packages/playwright-test/src/reporters/json.ts index 69c4bf9251..8d8553a09a 100644 --- a/packages/playwright-test/src/reporters/json.ts +++ b/packages/playwright-test/src/reporters/json.ts @@ -42,6 +42,7 @@ export interface JSONReportSuite { column: number; line: number; specs: JSONReportSpec[]; + hooks: JSONReportSpec[]; suites?: JSONReportSuite[]; } export interface JSONReportSpec { @@ -207,6 +208,7 @@ class JSONReporter implements Reporter { title: suite.title, ...this._relativeLocation(suite.location), specs: suite.tests.map(test => this._serializeTestSpec(test)), + hooks: suite.hooks.map(test => this._serializeTestSpec(test)), suites: suites.length ? suites : undefined, }; } diff --git a/packages/playwright-test/src/reporters/line.ts b/packages/playwright-test/src/reporters/line.ts index e9168ba743..0a98945fa3 100644 --- a/packages/playwright-test/src/reporters/line.ts +++ b/packages/playwright-test/src/reporters/line.ts @@ -61,7 +61,8 @@ class LineReporter extends BaseReporter { override onTestEnd(test: TestCase, result: TestResult) { super.onTestEnd(test, result); const width = process.env.PWTEST_SKIP_TEST_OUTPUT ? 79 : process.stdout.columns! - 1; - ++this._current; + if (!test.title.startsWith('beforeAll') && !test.title.startsWith('afterAll')) + ++this._current; const retriesSuffix = this.totalTestCount < this._current ? ` (retries)` : ``; const title = `[${this._current}/${this.totalTestCount}]${retriesSuffix} ${formatTestTitle(this.config, test)}`.substring(0, width); if (process.env.PWTEST_SKIP_TEST_OUTPUT) diff --git a/packages/playwright-test/src/reporters/raw.ts b/packages/playwright-test/src/reporters/raw.ts index 2d1a6f1d9e..d1df6fbe52 100644 --- a/packages/playwright-test/src/reporters/raw.ts +++ b/packages/playwright-test/src/reporters/raw.ts @@ -54,6 +54,7 @@ export type JsonSuite = { location?: JsonLocation; suites: JsonSuite[]; tests: JsonTestCase[]; + hooks: JsonTestCase[]; }; export type JsonTestCase = { @@ -188,6 +189,7 @@ class RawReporter { location, suites: suite.suites.map(s => this._serializeSuite(s)), tests: suite.tests.map(t => this._serializeTest(t, fileId)), + hooks: suite.hooks.map(t => this._serializeTest(t, fileId)), }; } diff --git a/packages/playwright-test/src/test.ts b/packages/playwright-test/src/test.ts index 3e666bed71..06c707367b 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 } from './types'; +import { Annotations, FixturesWithLocation, Location, TestCaseType } from './types'; import { FullProject } from './types'; class Base { @@ -45,7 +45,7 @@ export class Suite extends Base implements reporterTypes.Suite { _use: FixturesWithLocation[] = []; _isDescribe = false; _entries: (Suite | TestCase)[] = []; - _allHooks: TestCase[] = []; + hooks: TestCase[] = []; _eachHooks: { type: 'beforeEach' | 'afterEach', fn: Function, location: Location }[] = []; _timeout: number | undefined; _annotations: Annotations = []; @@ -67,7 +67,7 @@ export class Suite extends Base implements reporterTypes.Suite { _addAllHook(hook: TestCase) { hook.parent = this; - this._allHooks.push(hook); + this.hooks.push(hook); } allTests(): TestCase[] { @@ -132,7 +132,7 @@ export class TestCase extends Base implements reporterTypes.TestCase { annotations: Annotations = []; retries = 0; - _type: 'beforeAll' | 'afterAll' | 'test'; + _type: TestCaseType; _ordinalInFile: number; _testType: TestTypeImpl; _id = ''; @@ -141,7 +141,7 @@ export class TestCase extends Base implements reporterTypes.TestCase { _repeatEachIndex = 0; _projectIndex = 0; - constructor(type: 'beforeAll' | 'afterAll' | 'test', title: string, fn: Function, ordinalInFile: number, testType: TestTypeImpl, location: Location) { + constructor(type: TestCaseType, title: string, fn: Function, ordinalInFile: number, testType: TestTypeImpl, location: Location) { super(title); this._type = type; this.fn = fn; diff --git a/packages/playwright-test/src/testType.ts b/packages/playwright-test/src/testType.ts index b7b9e5c720..71633c1ede 100644 --- a/packages/playwright-test/src/testType.ts +++ b/packages/playwright-test/src/testType.ts @@ -21,7 +21,6 @@ import { wrapFunctionWithLocation } from './transform'; import { Fixtures, FixturesWithLocation, Location, TestType } from './types'; import { errorWithLocation, serializeError } from './util'; -const countByFile = new Map(); const testTypeSymbol = Symbol('testType'); export class TestTypeImpl { @@ -69,10 +68,7 @@ export class TestTypeImpl { if (!suite) throw errorWithLocation(location, `test() can only be called in a test file`); - const ordinalInFile = countByFile.get(suite._requireFile) || 0; - countByFile.set(suite._requireFile, ordinalInFile + 1); - - const test = new TestCase('test', title, fn, ordinalInFile, this, location); + const test = new TestCase('test', title, fn, nextOrdinalInFile(suite._requireFile), this, location); test._requireFile = suite._requireFile; suite._addTest(test); @@ -127,9 +123,9 @@ export class TestTypeImpl { if (!suite) throw errorWithLocation(location, `${name} hook can only be called in a test file`); if (name === 'beforeAll' || name === 'afterAll') { - const sameTypeCount = suite._allHooks.filter(hook => hook._type === name).length; + const sameTypeCount = suite.hooks.filter(hook => hook._type === name).length; const suffix = sameTypeCount ? String(sameTypeCount) : ''; - const hook = new TestCase(name, name + suffix, fn, 0, this, location); + const hook = new TestCase(name, name + suffix, fn, nextOrdinalInFile(suite._requireFile), this, location); hook._requireFile = suite._requireFile; suite._addAllHook(hook); } else { @@ -232,4 +228,11 @@ function throwIfRunningInsideJest() { } } +const countByFile = new Map(); +function nextOrdinalInFile(file: string) { + const ordinalInFile = countByFile.get(file) || 0; + countByFile.set(file, ordinalInFile + 1); + return ordinalInFile; +} + export const rootTestType = new TestTypeImpl([]); diff --git a/packages/playwright-test/src/types.ts b/packages/playwright-test/src/types.ts index 2b974694b7..160e7fce34 100644 --- a/packages/playwright-test/src/types.ts +++ b/packages/playwright-test/src/types.ts @@ -37,3 +37,5 @@ export interface TestStepInternal { export interface TestInfoImpl extends TestInfo { _addStep: (data: Omit) => TestStepInternal; } + +export type TestCaseType = 'beforeAll' | 'afterAll' | 'test'; diff --git a/packages/playwright-test/src/workerRunner.ts b/packages/playwright-test/src/workerRunner.ts index f66daa82f6..284cdc24c0 100644 --- a/packages/playwright-test/src/workerRunner.ts +++ b/packages/playwright-test/src/workerRunner.ts @@ -26,7 +26,7 @@ import { TestBeginPayload, TestEndPayload, RunPayload, TestEntry, DonePayload, W import { setCurrentTestInfo } from './globals'; import { Loader } from './loader'; import { Modifier, Suite, TestCase } from './test'; -import { Annotations, TestError, TestInfo, TestInfoImpl, TestStepInternal, WorkerInfo } from './types'; +import { Annotations, TestCaseType, TestError, TestInfo, TestInfoImpl, TestStepInternal, WorkerInfo } from './types'; import { ProjectImpl } from './project'; import { FixtureRunner } from './fixtures'; import { DeadlineRunner, raceAgainstDeadline } from 'playwright-core/lib/utils/async'; @@ -34,7 +34,7 @@ import { calculateFileSha1 } from 'playwright-core/lib/utils/utils'; const removeFolderAsync = util.promisify(rimraf); -type TestData = { testId: string, testInfo: TestInfoImpl, type: 'test' | 'beforeAll' | 'afterAll' }; +type TestData = { testId: string, testInfo: TestInfoImpl, type: TestCaseType }; export class WorkerRunner extends EventEmitter { private _params: WorkerInitParams; @@ -171,7 +171,7 @@ export class WorkerRunner extends EventEmitter { 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.testId, this._failedTest.testInfo)); + this.emit('testEnd', buildTestEndPayload(this._failedTest)); } this._reportDone(); runFinishedCallback(); @@ -199,7 +199,7 @@ export class WorkerRunner extends EventEmitter { annotations.push({ type: beforeAllModifier.type, description: beforeAllModifier.description }); } - for (const hook of suite._allHooks) { + for (const hook of suite.hooks) { if (hook._type !== 'beforeAll') continue; const firstTest = suite.allTests()[0]; @@ -214,7 +214,7 @@ export class WorkerRunner extends EventEmitter { await this._runTestOrAllHook(entry, annotations, runEntry.retry); } } - for (const hook of suite._allHooks) { + for (const hook of suite.hooks) { if (hook._type !== 'afterAll') continue; await this._runTestOrAllHook(hook, annotations, 0); @@ -222,7 +222,6 @@ export class WorkerRunner extends EventEmitter { } private async _runTestOrAllHook(test: TestCase, annotations: Annotations, retry: number) { - const reportEvents = test._type === 'test'; const startTime = monotonicTime(); const startWallTime = Date.now(); let deadlineRunner: DeadlineRunner | undefined; @@ -351,8 +350,7 @@ export class WorkerRunner extends EventEmitter { wallTime: Date.now(), error, }; - if (reportEvents) - this.emit('stepEnd', payload); + this.emit('stepEnd', payload); } }; const hasLocation = data.location && !data.location.file.includes('@playwright'); @@ -365,8 +363,7 @@ export class WorkerRunner extends EventEmitter { location, wallTime: Date.now(), }; - if (reportEvents) - this.emit('stepBegin', payload); + this.emit('stepBegin', payload); return step; }, }; @@ -405,13 +402,11 @@ export class WorkerRunner extends EventEmitter { return testInfo.timeout ? startTime + testInfo.timeout : 0; }; - if (reportEvents) - this.emit('testBegin', buildTestBeginPayload(testId, testInfo, startWallTime)); + this.emit('testBegin', buildTestBeginPayload(testData, startWallTime)); if (testInfo.expectedStatus === 'skipped') { testInfo.status = 'skipped'; - if (reportEvents) - this.emit('testEnd', buildTestEndPayload(testId, testInfo)); + this.emit('testEnd', buildTestEndPayload(testData)); return; } @@ -446,10 +441,10 @@ export class WorkerRunner extends EventEmitter { const isFailure = testInfo.status !== 'skipped' && testInfo.status !== testInfo.expectedStatus; if (isFailure) { - if (test._type === 'test') { - // Delay reporting testEnd result until after teardownScopes is done. - this._failedTest = testData; - } else { + // Delay reporting testEnd result until after teardownScopes is done. + this._failedTest = testData; + if (test._type !== 'test') { + // beforeAll/afterAll hook failure skips any remaining tests in the worker. if (!this._fatalError) this._fatalError = testInfo.error; // Keep any error we have, and add "timeout" message. @@ -457,8 +452,8 @@ export class WorkerRunner extends EventEmitter { this._fatalError = prependToTestError(this._fatalError, colors.red(`Timeout of ${testInfo.timeout}ms exceeded in ${test._type} hook.\n`)); } this.stop(); - } else if (reportEvents) { - this.emit('testEnd', buildTestEndPayload(testId, testInfo)); + } else { + this.emit('testEnd', buildTestEndPayload(testData)); } const preserveOutput = this._loader.fullConfig().preserveOutput === 'always' || @@ -597,14 +592,15 @@ export class WorkerRunner extends EventEmitter { } } -function buildTestBeginPayload(testId: string, testInfo: TestInfo, startWallTime: number): TestBeginPayload { +function buildTestBeginPayload(testData: TestData, startWallTime: number): TestBeginPayload { return { - testId, + testId: testData.testId, startWallTime, }; } -function buildTestEndPayload(testId: string, testInfo: TestInfo): TestEndPayload { +function buildTestEndPayload(testData: TestData): TestEndPayload { + const { testId, testInfo } = testData; return { testId, duration: testInfo.duration, diff --git a/packages/playwright-test/types/testReporter.d.ts b/packages/playwright-test/types/testReporter.d.ts index e0133e3984..7c02b629bb 100644 --- a/packages/playwright-test/types/testReporter.d.ts +++ b/packages/playwright-test/types/testReporter.d.ts @@ -84,6 +84,11 @@ export interface Suite { * listed in the child [suite.suites](https://playwright.dev/docs/api/class-suite#suite-suites). */ tests: TestCase[]; + /** + * `beforeAll` and `afterAll` hooks in the suite. Note that other hooks such as `beforeEach` and `afterEach` are reported + * as the steps within the test. + */ + hooks: TestCase[]; /** * Returns a list of titles from the root down to this suite. */ @@ -379,14 +384,14 @@ export interface Reporter { /** * Called when something has been written to the standard output in the worker process. * @param chunk Output chunk. - * @param test Test that was running. Note that output may happen when to test is running, in which case this will be [void]. + * @param test Test that was running. Note that output may happen when no test is running, in which case this will be [void]. * @param result Result of the test run, this object gets populated while the test runs. */ onStdOut?(chunk: string | Buffer, test?: TestCase, result?: TestResult): void; /** * Called when something has been written to the standard error in the worker process. * @param chunk Output chunk. - * @param test Test that was running. Note that output may happen when to test is running, in which case this will be [void]. + * @param test Test that was running. Note that output may happen when no test is running, in which case this will be [void]. * @param result Result of the test run, this object gets populated while the test runs. */ onStdErr?(chunk: string | Buffer, test?: TestCase, result?: TestResult): void; @@ -398,16 +403,16 @@ export interface Reporter { onTestEnd?(test: TestCase, result: TestResult): void; /** * Called when a test step started in the worker process. - * @param test Test that has been started. + * @param test Test that the step belongs to. * @param result Result of the test run, this object gets populated while the test runs. - * @param step Test step instance. + * @param step Test step instance that has started. */ onStepBegin?(test: TestCase, result: TestResult, step: TestStep): void; /** * Called when a test step finished in the worker process. - * @param test Test that has been finished. + * @param test Test that the step belongs to. * @param result Result of the test run. - * @param step Test step instance. + * @param step Test step instance that has finished. */ onStepEnd?(test: TestCase, result: TestResult, step: TestStep): void; /** diff --git a/tests/playwright-test/hooks.spec.ts b/tests/playwright-test/hooks.spec.ts index 720bc581d6..39a0d5fe5f 100644 --- a/tests/playwright-test/hooks.spec.ts +++ b/tests/playwright-test/hooks.spec.ts @@ -139,26 +139,30 @@ test('beforeEach failure should prevent the test, but not other hooks', async ({ }); test('beforeAll should be run once', async ({ runInlineTest }) => { - const report = await runInlineTest({ + const result = await runInlineTest({ 'a.test.js': ` const { test } = pwt; test.describe('suite1', () => { let counter = 0; test.beforeAll(async () => { - console.log('beforeAll1-' + (++counter)); + console.log('\\n%%beforeAll1-' + (++counter)); }); test.describe('suite2', () => { test.beforeAll(async () => { - console.log('beforeAll2'); + console.log('\\n%%beforeAll2'); }); test('one', async ({}) => { - console.log('test'); + console.log('\\n%%test'); }); }); }); `, }); - expect(report.output).toContain('beforeAll1-1\nbeforeAll2\ntest'); + expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ + '%%beforeAll1-1', + '%%beforeAll2', + '%%test', + ]); }); test('beforeEach should be able to skip a test', async ({ runInlineTest }) => { @@ -567,3 +571,43 @@ test('should report error from fixture teardown when beforeAll times out', async expect(stripAscii(result.output)).toContain('Timeout of 1000ms exceeded in beforeAll hook.'); expect(stripAscii(result.output)).toContain('Error: Oh my!'); }); + +test('should not hang and report results when worker process suddenly exits during afterAll', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.js': ` + const { test } = pwt; + test('passed', () => {}); + test.afterAll(() => { process.exit(0); }); + ` + }, { reporter: 'line' }); + expect(result.exitCode).toBe(1); + expect(result.passed).toBe(1); + expect(result.output).toContain('Worker process exited unexpectedly'); + expect(stripAscii(result.output)).toContain('[1/1] a.spec.js:6:7 › passed'); + expect(stripAscii(result.output)).toContain('[1/1] a.spec.js:7:12 › afterAll'); +}); + +test('same beforeAll running in parallel should be reported correctly', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.js': ` + const { test } = pwt; + test.describe.parallel('', () => { + test.beforeAll(async () => { + await new Promise(f => setTimeout(f, 3000)); + }); + test('test1', () => {}); + test('test2', () => {}); + }); + ` + }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(2); + const hook = result.report.suites[0].suites[0].hooks[0]; + expect(hook.title).toBe('beforeAll'); + expect(hook.tests.length).toBe(1); + expect(hook.tests[0].results.length).toBe(2); + expect(hook.tests[0].results[0].status).toBe('passed'); + expect(hook.tests[0].results[1].status).toBe('passed'); + const workers = [hook.tests[0].results[0].workerIndex, hook.tests[0].results[1].workerIndex]; + expect(workers.sort()).toEqual([0, 1]); +}); diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts index 67600dab7f..524b229150 100644 --- a/tests/playwright-test/reporter-html.spec.ts +++ b/tests/playwright-test/reporter-html.spec.ts @@ -295,6 +295,41 @@ test('should render annotations', async ({ runInlineTest, page, showReport }) => await expect(page.locator('.test-case-annotation')).toHaveText('skip: I am not interested in this test'); }); +test('should render beforeAll/afterAll hooks', async ({ runInlineTest, page, showReport }) => { + const result = await runInlineTest({ + 'a.test.js': ` + const { test } = pwt; + test.beforeAll(async () => { + }); + test.afterAll(async () => { + await test.step('after step', () => { + throw new Error('oh!'); + }); + }); + test('test', async ({}) => { + }); + `, + }, { reporter: 'dot,html' }); + expect(result.exitCode).toBe(1); + expect(result.passed).toBe(1); + + await showReport(); + await expect(page.locator('.subnav-item:has-text("All") .counter')).toHaveText('1'); + await expect(page.locator('.subnav-item:has-text("Passed") .counter')).toHaveText('1'); + await expect(page.locator('.subnav-item:has-text("Failed") .counter')).toHaveText('0'); + await expect(page.locator('.subnav-item:has-text("Flaky") .counter')).toHaveText('0'); + await expect(page.locator('.subnav-item:has-text("Skipped") .counter')).toHaveText('0'); + + await expect(page.locator('text=beforeAll')).toBeVisible(); + await expect(page.locator('.test-file-test:has-text("beforeAll") svg.color-icon-success')).toHaveCount(1); + + await expect(page.locator('text=afterAll')).toBeVisible(); + await expect(page.locator('.test-file-test:has-text("afterAll") svg.color-text-danger')).toHaveCount(1); + + await page.click('text=afterAll'); + await expect(page.locator('.tree-item:has-text("after step") svg.color-text-danger')).toHaveCount(1); +}); + test('should render text attachments as text', async ({ runInlineTest, page, showReport }) => { const result = await runInlineTest({ 'a.test.js': ` diff --git a/tests/playwright-test/reporter.spec.ts b/tests/playwright-test/reporter.spec.ts index 1d95ffb8fe..cf8c3e5e7e 100644 --- a/tests/playwright-test/reporter.spec.ts +++ b/tests/playwright-test/reporter.spec.ts @@ -305,6 +305,14 @@ test('should report api steps', async ({ runInlineTest }) => { `%% end {\"title\":\"After Hooks\",\"category\":\"hook\",\"steps\":[{\"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\"}`, `%% begin {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, `%% begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`, @@ -315,6 +323,12 @@ test('should report api steps', async ({ runInlineTest }) => { `%% 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\"}`, ]); }); diff --git a/tests/playwright-test/retry.spec.ts b/tests/playwright-test/retry.spec.ts index af989a1f0e..5e1dcf0c4b 100644 --- a/tests/playwright-test/retry.spec.ts +++ b/tests/playwright-test/retry.spec.ts @@ -162,7 +162,7 @@ test('should retry beforeAll failure', async ({ runInlineTest }) => { expect(result.passed).toBe(0); expect(result.failed).toBe(1); expect(result.skipped).toBe(1); - expect(stripAscii(result.output).split('\n')[2]).toBe('×°×°F°'); + expect(stripAscii(result.output).split('\n')[2]).toBe('××°F×°FF°'); expect(result.output).toContain('BeforeAll is bugged!'); }); diff --git a/tests/playwright-test/stdio.spec.ts b/tests/playwright-test/stdio.spec.ts index 758641848c..01e931ba69 100644 --- a/tests/playwright-test/stdio.spec.ts +++ b/tests/playwright-test/stdio.spec.ts @@ -39,7 +39,7 @@ test('should get top level stdio', async ({ runInlineTest }) => { ]); }); -test('should get stdio from env afterAll', async ({ runInlineTest }) => { +test('should get stdio from worker fixture teardown', async ({ runInlineTest }) => { const result = await runInlineTest({ 'helper.ts': ` export const test = pwt.test.extend({ @@ -61,6 +61,19 @@ test('should get stdio from env afterAll', async ({ runInlineTest }) => { ]); }); +test('should get stdio from beforeAll and afterAll', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.js': ` + const { test } = pwt; + test.beforeAll(() => console.log('before')); + test('is a test', () => {}); + test.afterAll(() => console.error('after')); + ` + }); + expect(result.report.suites[0].hooks[0].tests[0].results[0].stdout).toEqual([{ text: 'before\n' }]); + expect(result.report.suites[0].hooks[1].tests[0].results[0].stderr).toEqual([{ text: 'after\n' }]); +}); + test('should ignore stdio when quiet', async ({ runInlineTest }) => { const result = await runInlineTest({ 'playwright.config.ts': ` diff --git a/utils/generate_types/overrides-testReporter.d.ts b/utils/generate_types/overrides-testReporter.d.ts index 882718e4f8..bc4a1c9a24 100644 --- a/utils/generate_types/overrides-testReporter.d.ts +++ b/utils/generate_types/overrides-testReporter.d.ts @@ -29,6 +29,7 @@ export interface Suite { location?: Location; suites: Suite[]; tests: TestCase[]; + hooks: TestCase[]; titlePath(): string[]; allTests(): TestCase[]; project(): FullProject | undefined;