From 51c3ea55ed7a7ad735d41962ba4cf5a4cc4cdbde Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 13 Jul 2022 13:54:26 -0700 Subject: [PATCH] Revert "feat(line reporter): show testing progress (#15547)" (#15631) This reverts commit e76d9b3b2828620e36827047dd714ef037b75fdc. --- docs/src/test-reporters-js.md | 5 +- .../playwright-test/src/reporters/base.ts | 37 +----------- .../playwright-test/src/reporters/line.ts | 48 ++++++++------- .../playwright-test/src/reporters/list.ts | 3 +- tests/playwright-test/hooks.spec.ts | 2 +- tests/playwright-test/reporter-json.spec.ts | 2 +- tests/playwright-test/reporter-line.spec.ts | 58 ++++++++----------- 7 files changed, 54 insertions(+), 101 deletions(-) diff --git a/docs/src/test-reporters-js.md b/docs/src/test-reporters-js.md index 25bb363168..7a558d5fe2 100644 --- a/docs/src/test-reporters-js.md +++ b/docs/src/test-reporters-js.md @@ -151,7 +151,7 @@ Running 124 tests using 6 workers ### Line reporter -Line reporter is more concise than the list reporter. It uses two lines to report currently running test and testing progress, and prints failures when they occur. Line reporter is useful for large test suites where it shows the progress but does not spam the output by listing all the tests. +Line reporter is more concise than the list reporter. It uses a single line to report last finished test, and prints failures when they occur. Line reporter is useful for large test suites where it shows the progress but does not spam the output by listing all the tests. ```bash npx playwright test --reporter=line @@ -190,8 +190,7 @@ Running 124 tests using 6 workers Expected: 1 Received: 0 -example.spec.ts:8:3 › should navigate to playwright.dev -[23/124] Passed: 20 Flaky: 0 Failed: 0 Skipped: 2 (12s) +[23/124] gitignore.spec.ts - should respect nested .gitignore ``` ### Dot reporter diff --git a/packages/playwright-test/src/reporters/base.ts b/packages/playwright-test/src/reporters/base.ts index 97a65a0946..81807e7863 100644 --- a/packages/playwright-test/src/reporters/base.ts +++ b/packages/playwright-test/src/reporters/base.ts @@ -49,13 +49,13 @@ export class BaseReporter implements Reporter { duration = 0; config!: FullConfigInternal; suite!: Suite; + totalTestCount = 0; result!: FullResult; private fileDurations = new Map(); private monotonicStartTime: number = 0; private _omitFailures: boolean; private readonly _ttyWidthForTest: number; readonly liveTerminal: boolean; - readonly stats = { started: 0, skipped: 0, completed: 0, total: 0, passed: 0, failed: 0, flaky: 0 }; constructor(options: { omitFailures?: boolean } = {}) { this._omitFailures = options.omitFailures || false; @@ -67,7 +67,7 @@ export class BaseReporter implements Reporter { this.monotonicStartTime = monotonicTime(); this.config = config as FullConfigInternal; this.suite = suite; - this.stats.total = suite.allTests().length; + this.totalTestCount = suite.allTests().length; } onStdOut(chunk: string | Buffer, test?: TestCase, result?: TestResult) { @@ -85,21 +85,7 @@ export class BaseReporter implements Reporter { (result as any)[kOutputSymbol].push(output); } - onTestBegin(test: TestCase, result: TestResult) { - this.stats.started++; - } - onTestEnd(test: TestCase, result: TestResult) { - this.stats.completed++; - if (!this.willRetry(test)) { - switch (test.outcome()) { - case 'skipped': this.stats.skipped++; break; - case 'expected': this.stats.passed++; break; - case 'unexpected': this.stats.failed++; break; - case 'flaky': this.stats.flaky++; break; - } - } - // Ignore any tests that are run in parallel. for (let suite: Suite | undefined = test.parent; suite; suite = suite.parent) { if ((suite as any)._parallelMode === 'parallel') @@ -133,24 +119,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.stats.total} test${this.stats.total > 1 ? 's' : ''} using ${jobs} worker${jobs > 1 ? 's' : ''}${shardDetails}`; - } - - protected generateStatsMessage(mode: 'started' | 'completed', done: boolean) { - // Do not report 100% until done. - const count = this.stats[mode]; - const percent = Math.min(done ? 100 : 99, Math.round(count / this.stats.total * 100)); - const maxExpected = done ? this.stats.total : this.stats.total - (mode === 'started' ? 0 : 1); - const retriesSuffix = count > maxExpected ? `+retries` : ``; - const message = [ - `[${count}/${this.stats.total}${retriesSuffix}]`, - `${(this.stats.passed ? colors.green : colors.gray)('Passed: ' + this.stats.passed)}`, - `${(this.stats.flaky ? colors.red : colors.gray)('Flaky: ' + this.stats.flaky)}`, - `${(this.stats.failed ? colors.red : colors.gray)('Failed: ' + this.stats.failed)}`, - `${(this.stats.skipped ? colors.yellow : colors.gray)('Skipped: ' + this.stats.skipped)}`, - colors.gray(process.env.PW_TEST_DEBUG_REPORTERS ? `(XXms)` : `(${milliseconds((monotonicTime() - this.monotonicStartTime) | 0)})`), - ].join(' '); - return { percent, message }; + return `\nRunning ${this.totalTestCount} test${this.totalTestCount > 1 ? 's' : ''} using ${jobs} worker${jobs > 1 ? 's' : ''}${shardDetails}`; } protected getSlowTests(): [string, number][] { diff --git a/packages/playwright-test/src/reporters/line.ts b/packages/playwright-test/src/reporters/line.ts index e5a6b8538f..f3485117cc 100644 --- a/packages/playwright-test/src/reporters/line.ts +++ b/packages/playwright-test/src/reporters/line.ts @@ -22,6 +22,7 @@ const lineUp = process.env.PW_TEST_DEBUG_REPORTERS ? '' : '\u001B[1A'; const erase = process.env.PW_TEST_DEBUG_REPORTERS ? '' : '\u001B[2K'; class LineReporter extends BaseReporter { + private _current = 0; private _failures = 0; private _lastTest: TestCase | undefined; private _lastPercent = -1; @@ -34,31 +35,27 @@ class LineReporter extends BaseReporter { super.onBegin(config, suite); console.log(this.generateStartingMessage()); if (this.liveTerminal) - console.log('\n'); + console.log(); } override onStdOut(chunk: string | Buffer, test?: TestCase, result?: TestResult) { super.onStdOut(chunk, test, result); - this._dumpToStdio(test, result, chunk, process.stdout); + this._dumpToStdio(test, chunk, process.stdout); } override onStdErr(chunk: string | Buffer, test?: TestCase, result?: TestResult) { super.onStdErr(chunk, test, result); - this._dumpToStdio(test, result, chunk, process.stderr); + this._dumpToStdio(test, chunk, process.stderr); } - private _retrySuffix(result: TestResult | undefined) { - return result?.retry ? colors.yellow(` (retry #${result.retry})`) : ''; - } - - private _dumpToStdio(test: TestCase | undefined, result: TestResult | undefined, chunk: string | Buffer, stream: NodeJS.WriteStream) { + private _dumpToStdio(test: TestCase | undefined, chunk: string | Buffer, stream: NodeJS.WriteStream) { if (this.config.quiet) return; if (this.liveTerminal) - stream.write(lineUp + erase + lineUp + erase); + stream.write(lineUp + erase); if (test && this._lastTest !== test) { // Write new header for the output. - const title = colors.gray(formatTestTitle(this.config, test)) + this._retrySuffix(result); + const title = colors.gray(formatTestTitle(this.config, test)); stream.write(this.fitToScreen(title) + `\n`); this._lastTest = test; } @@ -67,12 +64,11 @@ class LineReporter extends BaseReporter { if (chunk[chunk.length - 1] !== '\n') console.log(); - if (this.liveTerminal) - console.log('\n'); + console.log(); } - override onTestBegin(test: TestCase, result: TestResult) { - super.onTestBegin(test, result); + onTestBegin(test: TestCase, result: TestResult) { + ++this._current; this._updateLine(test, result, undefined); } @@ -90,34 +86,36 @@ class LineReporter extends BaseReporter { super.onTestEnd(test, result); if (!this.willRetry(test) && (test.outcome() === 'flaky' || test.outcome() === 'unexpected')) { if (this.liveTerminal) - process.stdout.write(lineUp + erase + lineUp + erase); + process.stdout.write(lineUp + erase); console.log(formatFailure(this.config, test, { index: ++this._failures }).message); console.log(); - if (this.liveTerminal) - process.stdout.write(this.fitToScreen(this.generateStatsMessage('started', false).message) + '\n'); } } private _updateLine(test: TestCase, result: TestResult, step?: TestStep) { - const stats = this.generateStatsMessage('started', false); - const title = formatTestTitle(this.config, test, step) + this._retrySuffix(result); + // Do not report 100% until done. + const percent = Math.min(99, Math.round(this._current / this.totalTestCount * 100)); + const retriesPrefix = this.totalTestCount < this._current ? ` (retries)` : ``; + const prefix = `[${this._current}/${this.totalTestCount}]${retriesPrefix} `; + const currentRetrySuffix = result.retry ? colors.yellow(` (retry #${result.retry})`) : ''; + const title = formatTestTitle(this.config, test, step) + currentRetrySuffix; if (this.liveTerminal) { - process.stdout.write(lineUp + erase + lineUp + erase + this.fitToScreen(title) + '\n' + this.fitToScreen(stats.message) + '\n'); + process.stdout.write(lineUp + erase + prefix + this.fitToScreen(title, prefix) + '\n'); } else { - if (stats.percent !== this._lastPercent) - process.stdout.write(this.fitToScreen(stats.message) + '\n'); + if (percent !== this._lastPercent) + process.stdout.write(`[${percent}%] ${title}\n`); } - this._lastPercent = stats.percent; + this._lastPercent = percent; } override async onEnd(result: FullResult) { await super.onEnd(result); if (this.liveTerminal) - process.stdout.write(lineUp + erase + lineUp + erase); + process.stdout.write(lineUp + erase); else - process.stdout.write(this.fitToScreen(this.generateStatsMessage('started', true).message) + '\n'); + process.stdout.write(`[100%]\n`); this.epilogue(false); } } diff --git a/packages/playwright-test/src/reporters/list.ts b/packages/playwright-test/src/reporters/list.ts index 984bffa2eb..8797d78088 100644 --- a/packages/playwright-test/src/reporters/list.ts +++ b/packages/playwright-test/src/reporters/list.ts @@ -43,8 +43,7 @@ class ListReporter extends BaseReporter { console.log(); } - override onTestBegin(test: TestCase, result: TestResult) { - super.onTestBegin(test, result); + onTestBegin(test: TestCase, result: TestResult) { if (this.liveTerminal) { if (this._needNewLine) { this._needNewLine = false; diff --git a/tests/playwright-test/hooks.spec.ts b/tests/playwright-test/hooks.spec.ts index 15c0269a36..78a2a55d6a 100644 --- a/tests/playwright-test/hooks.spec.ts +++ b/tests/playwright-test/hooks.spec.ts @@ -636,7 +636,7 @@ test('should not hang and report results when worker process suddenly exits duri expect(result.passed).toBe(0); expect(result.failed).toBe(1); expect(result.output).toContain('Worker process exited unexpectedly'); - expect(stripAnsi(result.output)).toContain('a.spec.js:6:7 › failing due to 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 }) => { diff --git a/tests/playwright-test/reporter-json.spec.ts b/tests/playwright-test/reporter-json.spec.ts index 358c637852..8fdc3b2f11 100644 --- a/tests/playwright-test/reporter-json.spec.ts +++ b/tests/playwright-test/reporter-json.spec.ts @@ -232,6 +232,6 @@ test('should add line in addition to file json without CI', async ({ runInlineTe `, }, { reporter: '' }, { PLAYWRIGHT_LIVE_TERMINAL: '1' }); expect(result.exitCode).toBe(0); - expect(stripAnsi(result.output)).toContain('a.test.js:6:7 › one'); + expect(stripAnsi(result.output)).toContain('[1/1] a.test.js:6:7 › one'); expect(fs.existsSync(testInfo.outputPath('a.json'))).toBeTruthy(); }); diff --git a/tests/playwright-test/reporter-line.spec.ts b/tests/playwright-test/reporter-line.spec.ts index 9ea087298c..f1af9f6048 100644 --- a/tests/playwright-test/reporter-line.spec.ts +++ b/tests/playwright-test/reporter-line.spec.ts @@ -37,13 +37,12 @@ test('should work with tty', async ({ runInlineTest }, testInfo) => { PW_TEST_DEBUG_REPORTERS: '1', }); expect(result.exitCode).toBe(1); - expect(trimLineEnds(result.output)).toContain(trimLineEnds(`a.test.js:6:12 › skipped test -[1/4] Passed: 0 Flaky: 0 Failed: 0 Skipped: 0 (XXms) -a.test.js:8:7 › flaky test -[2/4] Passed: 0 Flaky: 0 Failed: 0 Skipped: 1 (XXms) -a.test.js:8:7 › flaky test (retry #1) -[3/4] Passed: 0 Flaky: 0 Failed: 0 Skipped: 1 (XXms) - 1) a.test.js:8:7 › flaky test ==================================================================== + expect(trimLineEnds(result.output)).toContain(trimLineEnds(`Running 4 tests using 1 worker + +[1/4] a.test.js:6:12 › skipped test +[2/4] a.test.js:8:7 › flaky test +[3/4] a.test.js:8:7 › flaky test (retry #1) + 1) a.test.js:8:7 › flaky test ==================================================================== Error: expect(received).toBe(expected) // Object.is equality @@ -61,14 +60,10 @@ test('should work with tty', async ({ runInlineTest }, testInfo) => { at ${testInfo.outputPath('a.test.js')}:9:32 -[3/4] Passed: 0 Flaky: 1 Failed: 0 Skipped: 1 (XXms) -a.test.js:11:7 › passing test -[4/4] Passed: 0 Flaky: 1 Failed: 0 Skipped: 1 (XXms) -a.test.js:13:7 › failing test -[5/4+retries] Passed: 1 Flaky: 1 Failed: 0 Skipped: 1 (XXms) -a.test.js:13:7 › failing test (retry #1) -[6/4+retries] Passed: 1 Flaky: 1 Failed: 0 Skipped: 1 (XXms) - 2) a.test.js:13:7 › failing test ================================================================= +[4/4] a.test.js:11:7 › passing test +[5/4] (retries) a.test.js:13:7 › failing test +[6/4] (retries) a.test.js:13:7 › failing test (retry #1) + 2) a.test.js:13:7 › failing test ================================================================= Error: expect(received).toBe(expected) // Object.is equality @@ -101,8 +96,7 @@ test('should work with tty', async ({ runInlineTest }, testInfo) => { at ${testInfo.outputPath('a.test.js')}:14:19 -[6/4+retries] Passed: 1 Flaky: 1 Failed: 1 Skipped: 1 (XXms) - + 1 failed a.test.js:13:7 › failing test ================================================================== 1 flaky @@ -132,9 +126,9 @@ test('should work with non-tty', async ({ runInlineTest }, testInfo) => { }); expect(result.exitCode).toBe(1); expect(trimLineEnds(result.output)).toContain(trimLineEnds(`Running 4 tests using 1 worker -[1/4] Passed: 0 Flaky: 0 Failed: 0 Skipped: 0 (XXms) -[2/4] Passed: 0 Flaky: 0 Failed: 0 Skipped: 1 (XXms) -[3/4] Passed: 0 Flaky: 0 Failed: 0 Skipped: 1 (XXms) +[25%] a.test.js:6:12 › skipped test +[50%] a.test.js:8:7 › flaky test +[75%] a.test.js:8:7 › flaky test (retry #1) 1) a.test.js:8:7 › flaky test ==================================================================== Error: expect(received).toBe(expected) // Object.is equality @@ -153,7 +147,7 @@ test('should work with non-tty', async ({ runInlineTest }, testInfo) => { at ${testInfo.outputPath('a.test.js')}:9:32 -[4/4] Passed: 0 Flaky: 1 Failed: 0 Skipped: 1 (XXms) +[99%] a.test.js:11:7 › passing test 2) a.test.js:13:7 › failing test ================================================================= Error: expect(received).toBe(expected) // Object.is equality @@ -187,7 +181,7 @@ test('should work with non-tty', async ({ runInlineTest }, testInfo) => { at ${testInfo.outputPath('a.test.js')}:14:19 -[6/4+retries] Passed: 1 Flaky: 1 Failed: 1 Skipped: 1 (XXms) +[100%] 1 failed a.test.js:13:7 › failing test ================================================================== @@ -210,10 +204,10 @@ test('should spare status updates in non-tty mode', async ({ runInlineTest }) => PW_TEST_DEBUG_REPORTERS: '1', }); expect(result.exitCode).toBe(0); - const lines = [`Running 300 tests using 1 worker`, `[1/300] Passed: 0 Flaky: 0 Failed: 0 Skipped: 0 (XXms)`]; - for (let i = 0; i < 99; i++) - lines.push(`[${3 * i + 2}/300] Passed: ${3 * i + 1} Flaky: 0 Failed: 0 Skipped: 0 (XXms)`); - lines.push('[300/300] Passed: 300 Flaky: 0 Failed: 0 Skipped: 0 (XXms)'); + const lines = [`Running 300 tests using 1 worker`, `[0%] a.test.js:7:9 › test0`]; + for (let i = 1; i <= 99; i++) + lines.push(`[${i}%] a.test.js:7:9 › test${3 * i - 2}`); + lines.push('[100%]'); lines.push(''); lines.push(' 300 passed'); expect(trimLineEnds(result.output)).toContain(lines.join('\n')); @@ -228,21 +222,15 @@ test('should print output', async ({ runInlineTest }) => { process.stdout.write('two'); console.log('full-line'); }); - test('one more', async ({}, testInfo) => { - console.log('yay'); - }); ` - }, { reporter: 'line' }, { PW_TEST_DEBUG_REPORTERS: '1' }); + }, { reporter: 'line' }); expect(result.exitCode).toBe(0); expect(stripAnsi(result.output)).toContain([ - '[1/2] Passed: 0 Flaky: 0 Failed: 0 Skipped: 0 (XXms)', 'a.spec.ts:6:7 › foobar', 'one', + '', 'two', + '', 'full-line', - '[2/2] Passed: 1 Flaky: 0 Failed: 0 Skipped: 0 (XXms)', - 'a.spec.ts:11:7 › one more', - 'yay', - '[2/2] Passed: 2 Flaky: 0 Failed: 0 Skipped: 0 (XXms)', ].join('\n')); });