From 62e4e80599efc00a799a389ee4be76c74c9f172b Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 28 Jul 2022 14:46:21 -0700 Subject: [PATCH] feat(test runner): show the number of fatal errors at the end (#15975) --- .../playwright-test/src/reporters/base.ts | 13 ++++-- packages/playwright-test/src/reporters/dot.ts | 10 ++++- .../playwright-test/src/reporters/line.ts | 14 +++++- .../playwright-test/src/reporters/list.ts | 43 ++++++++++++------- packages/playwright-test/src/worker.ts | 4 +- packages/playwright-test/src/workerRunner.ts | 23 +++++++--- tests/playwright-test/fixture-errors.spec.ts | 6 +-- tests/playwright-test/reporter-base.spec.ts | 28 ++++++++++++ 8 files changed, 108 insertions(+), 33 deletions(-) diff --git a/packages/playwright-test/src/reporters/base.ts b/packages/playwright-test/src/reporters/base.ts index 87fbab24ad..c2501edd0b 100644 --- a/packages/playwright-test/src/reporters/base.ts +++ b/packages/playwright-test/src/reporters/base.ts @@ -42,6 +42,7 @@ type TestSummary = { unexpected: TestCase[]; flaky: TestCase[]; failuresToPrint: TestCase[]; + fatalErrors: TestError[]; }; export class BaseReporter implements Reporter { @@ -54,6 +55,7 @@ export class BaseReporter implements Reporter { private monotonicStartTime: number = 0; private _omitFailures: boolean; private readonly _ttyWidthForTest: number; + private _fatalErrors: TestError[] = []; constructor(options: { omitFailures?: boolean } = {}) { this._omitFailures = options.omitFailures || false; @@ -96,7 +98,7 @@ export class BaseReporter implements Reporter { } onError(error: TestError) { - console.log('\n' + formatError(this.config, error, colors.enabled).message); + this._fatalErrors.push(error); } async onEnd(result: FullResult) { @@ -116,7 +118,7 @@ export class BaseReporter implements Reporter { protected generateStartingMessage() { const jobs = Math.min(this.config.workers, this.config._testGroupsCount); const shardDetails = this.config.shard ? `, shard ${this.config.shard.current} of ${this.config.shard.total}` : ''; - return `\nRunning ${this.totalTestCount} test${this.totalTestCount > 1 ? 's' : ''} using ${jobs} worker${jobs > 1 ? 's' : ''}${shardDetails}`; + return `\nRunning ${this.totalTestCount} test${this.totalTestCount !== 1 ? 's' : ''} using ${jobs} worker${jobs !== 1 ? 's' : ''}${shardDetails}`; } protected getSlowTests(): [string, number][] { @@ -129,8 +131,10 @@ export class BaseReporter implements Reporter { return fileDurations.filter(([,duration]) => duration > threshold).slice(0, count); } - protected generateSummaryMessage({ skipped, expected, unexpected, flaky }: TestSummary) { + protected generateSummaryMessage({ skipped, expected, unexpected, flaky, fatalErrors }: TestSummary) { const tokens: string[] = []; + if (fatalErrors.length) + tokens.push(colors.red(` ${fatalErrors.length} fatal ${fatalErrors.length === 1 ? 'error' : 'errors'}`)); if (unexpected.length) { tokens.push(colors.red(` ${unexpected.length} failed`)); for (const test of unexpected) @@ -179,7 +183,8 @@ export class BaseReporter implements Reporter { skippedWithError, unexpected, flaky, - failuresToPrint + failuresToPrint, + fatalErrors: this._fatalErrors, }; } diff --git a/packages/playwright-test/src/reporters/dot.ts b/packages/playwright-test/src/reporters/dot.ts index 3f12a168c0..849211fd96 100644 --- a/packages/playwright-test/src/reporters/dot.ts +++ b/packages/playwright-test/src/reporters/dot.ts @@ -15,8 +15,8 @@ */ import { colors } from 'playwright-core/lib/utilsBundle'; -import { BaseReporter } from './base'; -import type { FullResult, TestCase, TestResult, FullConfig, Suite } from '../../types/testReporter'; +import { BaseReporter, formatError } from './base'; +import type { FullResult, TestCase, TestResult, FullConfig, Suite, TestError } from '../../types/testReporter'; class DotReporter extends BaseReporter { private _counter = 0; @@ -64,6 +64,12 @@ class DotReporter extends BaseReporter { } } + override onError(error: TestError): void { + super.onError(error); + console.log('\n' + formatError(this.config, error, colors.enabled).message); + this._counter = 0; + } + override async onEnd(result: FullResult) { await super.onEnd(result); process.stdout.write('\n'); diff --git a/packages/playwright-test/src/reporters/line.ts b/packages/playwright-test/src/reporters/line.ts index 914847734e..88ca4af977 100644 --- a/packages/playwright-test/src/reporters/line.ts +++ b/packages/playwright-test/src/reporters/line.ts @@ -15,8 +15,8 @@ */ import { colors } from 'playwright-core/lib/utilsBundle'; -import { BaseReporter, formatFailure, formatTestTitle } from './base'; -import type { FullConfig, TestCase, Suite, TestResult, FullResult, TestStep } from '../../types/testReporter'; +import { BaseReporter, formatError, formatFailure, formatTestTitle } from './base'; +import type { FullConfig, TestCase, Suite, TestResult, FullResult, TestStep, TestError } from '../../types/testReporter'; class LineReporter extends BaseReporter { private _current = 0; @@ -100,6 +100,16 @@ class LineReporter extends BaseReporter { process.stdout.write(`\u001B[1A\u001B[2K${prefix + this.fitToScreen(title, prefix)}\n`); } + override onError(error: TestError): void { + super.onError(error); + + const message = formatError(this.config, error, colors.enabled).message + '\n\n'; + if (!process.env.PW_TEST_DEBUG_REPORTERS) + process.stdout.write(`\u001B[1A\u001B[2K`); + process.stdout.write(message); + console.log(); + } + override async onEnd(result: FullResult) { if (!process.env.PW_TEST_DEBUG_REPORTERS) process.stdout.write(`\u001B[1A\u001B[2K`); diff --git a/packages/playwright-test/src/reporters/list.ts b/packages/playwright-test/src/reporters/list.ts index 86c53a411e..25c3ecb3a1 100644 --- a/packages/playwright-test/src/reporters/list.ts +++ b/packages/playwright-test/src/reporters/list.ts @@ -16,8 +16,8 @@ /* eslint-disable no-console */ import { colors, ms as milliseconds } from 'playwright-core/lib/utilsBundle'; -import { BaseReporter, formatTestTitle, stripAnsiEscapes } from './base'; -import type { FullConfig, FullResult, Suite, TestCase, TestResult, TestStep } from '../../types/testReporter'; +import { BaseReporter, formatError, formatTestTitle, stripAnsiEscapes } from './base'; +import type { FullConfig, FullResult, Suite, TestCase, TestError, TestResult, TestStep } from '../../types/testReporter'; // Allow it in the Visual Studio Code Terminal and the new Windows Terminal const DOES_NOT_SUPPORT_UTF8_IN_TERMINAL = process.platform === 'win32' && process.env.TERM_PROGRAM !== 'vscode' && !process.env.WT_SESSION; @@ -47,11 +47,8 @@ class ListReporter extends BaseReporter { } onTestBegin(test: TestCase, result: TestResult) { - if (this._liveTerminal && this._needNewLine) { - this._needNewLine = false; - process.stdout.write('\n'); - this._lastRow++; - } + if (this._liveTerminal) + this._maybeWriteNewLine(); this._resultIndex.set(result, this._resultIndex.size + 1); this._testRows.set(test, this._lastRow++); if (this._liveTerminal) { @@ -87,15 +84,26 @@ class ListReporter extends BaseReporter { this._updateTestLine(test, colors.dim(formatTestTitle(this.config, test, step.parent)) + this._retrySuffix(result), this._testPrefix(result, '')); } - private _dumpToStdio(test: TestCase | undefined, chunk: string | Buffer, stream: NodeJS.WriteStream) { - if (this.config.quiet) - return; - const text = chunk.toString('utf-8'); + private _maybeWriteNewLine() { + if (this._needNewLine) { + this._needNewLine = false; + process.stdout.write('\n'); + } + } + + private _updateLineCountAndNewLineFlagForOutput(text: string) { this._needNewLine = text[text.length - 1] !== '\n'; if (this._liveTerminal) { const newLineCount = text.split('\n').length - 1; this._lastRow += newLineCount; } + } + + private _dumpToStdio(test: TestCase | undefined, chunk: string | Buffer, stream: NodeJS.WriteStream) { + if (this.config.quiet) + return; + const text = chunk.toString('utf-8'); + this._updateLineCountAndNewLineFlagForOutput(text); stream.write(chunk); } @@ -124,10 +132,7 @@ class ListReporter extends BaseReporter { if (this._liveTerminal) { this._updateTestLine(test, text, prefix); } else { - if (this._needNewLine) { - this._needNewLine = false; - process.stdout.write('\n'); - } + this._maybeWriteNewLine(); process.stdout.write(prefix + text); process.stdout.write('\n'); } @@ -170,6 +175,14 @@ class ListReporter extends BaseReporter { process.stdout.write(testRow + ' : ' + prefix + line + '\n'); } + override onError(error: TestError): void { + super.onError(error); + this._maybeWriteNewLine(); + const message = formatError(this.config, error, colors.enabled).message + '\n'; + this._updateLineCountAndNewLineFlagForOutput(message); + process.stdout.write(message); + } + override async onEnd(result: FullResult) { await super.onEnd(result); process.stdout.write('\n'); diff --git a/packages/playwright-test/src/worker.ts b/packages/playwright-test/src/worker.ts index a4b96a7ef5..96bc36c20a 100644 --- a/packages/playwright-test/src/worker.ts +++ b/packages/playwright-test/src/worker.ts @@ -99,7 +99,9 @@ async function gracefullyCloseAndExit() { await stopProfiling(workerIndex); } catch (e) { try { - const payload: TeardownErrorsPayload = { fatalErrors: [serializeError(e)] }; + const error = serializeError(e); + workerRunner.appendWorkerTeardownDiagnostics(error); + const payload: TeardownErrorsPayload = { fatalErrors: [error] }; process.send!({ method: 'teardownErrors', params: payload }); } catch { } diff --git a/packages/playwright-test/src/workerRunner.ts b/packages/playwright-test/src/workerRunner.ts index b704aca2a4..31151b8a1c 100644 --- a/packages/playwright-test/src/workerRunner.ts +++ b/packages/playwright-test/src/workerRunner.ts @@ -86,15 +86,13 @@ export class WorkerRunner extends EventEmitter { await this._loadIfNeeded(); await this._teardownScopes(); if (this._fatalErrors.length) { - const diagnostics = this._createWorkerTeardownDiagnostics(); - if (diagnostics) - this._fatalErrors.unshift(diagnostics); + this.appendWorkerTeardownDiagnostics(this._fatalErrors[this._fatalErrors.length - 1]); const payload: TeardownErrorsPayload = { fatalErrors: this._fatalErrors }; this.emit('teardownErrors', payload); } } - private _createWorkerTeardownDiagnostics(): TestError | undefined { + appendWorkerTeardownDiagnostics(error: TestError) { if (!this._lastRunningTests.length) return; const count = this._totalRunningTests === 1 ? '1 test' : `${this._totalRunningTests} tests`; @@ -102,10 +100,23 @@ export class WorkerRunner extends EventEmitter { if (this._lastRunningTests.length < this._totalRunningTests) lastMessage = `, last ${this._lastRunningTests.length} tests were`; const message = [ - colors.red(`Worker teardown error. This worker ran ${count}${lastMessage}:`), + '', + '', + colors.red(`Failed worker ran ${count}${lastMessage}:`), ...this._lastRunningTests.map(testInfo => formatTestTitle(testInfo._test, testInfo.project.name)), ].join('\n'); - return { message }; + if (error.message) { + if (error.stack) { + let index = error.stack.indexOf(error.message); + if (index !== -1) { + index += error.message.length; + error.stack = error.stack.substring(0, index) + message + error.stack.substring(index); + } + } + error.message += message; + } else if (error.value) { + error.value += message; + } } private async _teardownScopes() { diff --git a/tests/playwright-test/fixture-errors.spec.ts b/tests/playwright-test/fixture-errors.spec.ts index 5d63304297..adbe892718 100644 --- a/tests/playwright-test/fixture-errors.spec.ts +++ b/tests/playwright-test/fixture-errors.spec.ts @@ -517,7 +517,9 @@ test('should report worker fixture teardown with debug info', async ({ runInline expect(result.exitCode).toBe(1); expect(result.passed).toBe(20); expect(stripAnsi(result.output)).toContain([ - 'Worker teardown error. This worker ran 20 tests, last 10 tests were:', + 'Worker teardown timeout of 1000ms exceeded while tearing down "fixture".', + '', + 'Failed worker ran 20 tests, last 10 tests were:', 'a.spec.ts:12:9 › good10', 'a.spec.ts:12:9 › good11', 'a.spec.ts:12:9 › good12', @@ -528,8 +530,6 @@ test('should report worker fixture teardown with debug info', async ({ runInline 'a.spec.ts:12:9 › good17', 'a.spec.ts:12:9 › good18', 'a.spec.ts:12:9 › good19', - '', - 'Worker teardown timeout of 1000ms exceeded while tearing down "fixture".', ].join('\n')); }); diff --git a/tests/playwright-test/reporter-base.spec.ts b/tests/playwright-test/reporter-base.spec.ts index 7322fe3a37..9e91136af2 100644 --- a/tests/playwright-test/reporter-base.spec.ts +++ b/tests/playwright-test/reporter-base.spec.ts @@ -290,3 +290,31 @@ test('should not crash on undefined body with manual attachments', async ({ runI expect(result.failed).toBe(1); expect(result.exitCode).toBe(1); }); + +test('should report fatal errors at the end', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.ts': ` + const test = pwt.test.extend({ + fixture: [async ({ }, use) => { + await use(); + throw new Error('oh my!'); + }, { scope: 'worker' }], + }); + test('good', async ({ fixture }) => { + }); + `, + 'b.spec.ts': ` + const test = pwt.test.extend({ + fixture: [async ({ }, use) => { + await use(); + throw new Error('oh my!'); + }, { scope: 'worker' }], + }); + test('good', async ({ fixture }) => { + }); + `, + }, { reporter: 'list' }); + expect(result.exitCode).toBe(1); + expect(result.passed).toBe(2); + expect(stripAnsi(result.output)).toContain('2 fatal errors'); +});