From 22ead7b5ab4918b774791f5173468ce6e3ea4327 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 13 Jul 2022 15:11:56 -0700 Subject: [PATCH] Revert "fix(test runner): handle istty in line reporter (#15455)" (#15633) This reverts commit 767babc3a225f9dec7efbcfef1bd073bcb053d93. --- .../playwright-test/src/reporters/base.ts | 3 - .../playwright-test/src/reporters/line.ts | 34 +-- .../playwright-test/src/reporters/list.ts | 12 +- tests/playwright-test/hooks.spec.ts | 2 +- .../playwright-test-fixtures.ts | 4 - tests/playwright-test/reporter-json.spec.ts | 2 +- tests/playwright-test/reporter-line.spec.ts | 232 ++++-------------- tests/playwright-test/reporter-list.spec.ts | 6 +- 8 files changed, 74 insertions(+), 221 deletions(-) diff --git a/packages/playwright-test/src/reporters/base.ts b/packages/playwright-test/src/reporters/base.ts index 81807e7863..87fbab24ad 100644 --- a/packages/playwright-test/src/reporters/base.ts +++ b/packages/playwright-test/src/reporters/base.ts @@ -20,7 +20,6 @@ import path from 'path'; import type { FullConfig, TestCase, Suite, TestResult, TestError, Reporter, FullResult, TestStep, Location } from '../../types/testReporter'; import type { FullConfigInternal } from '../types'; import { codeFrameColumns } from '../babelBundle'; -import { getAsBooleanFromENV } from 'playwright-core/lib/utils'; export type TestResultOutput = { chunk: string | Buffer, type: 'stdout' | 'stderr' }; export const kOutputSymbol = Symbol('output'); @@ -55,12 +54,10 @@ export class BaseReporter implements Reporter { private monotonicStartTime: number = 0; private _omitFailures: boolean; private readonly _ttyWidthForTest: number; - readonly liveTerminal: boolean; constructor(options: { omitFailures?: boolean } = {}) { this._omitFailures = options.omitFailures || false; this._ttyWidthForTest = parseInt(process.env.PWTEST_TTY_WIDTH || '', 10); - this.liveTerminal = process.stdout.isTTY || getAsBooleanFromENV('PLAYWRIGHT_LIVE_TERMINAL'); } onBegin(config: FullConfig, suite: Suite) { diff --git a/packages/playwright-test/src/reporters/line.ts b/packages/playwright-test/src/reporters/line.ts index f3485117cc..373251f062 100644 --- a/packages/playwright-test/src/reporters/line.ts +++ b/packages/playwright-test/src/reporters/line.ts @@ -18,14 +18,10 @@ import { colors } from 'playwright-core/lib/utilsBundle'; import { BaseReporter, formatFailure, formatTestTitle } from './base'; import type { FullConfig, TestCase, Suite, TestResult, FullResult, TestStep } from '../../types/testReporter'; -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; printsToStdio() { return true; @@ -34,8 +30,7 @@ class LineReporter extends BaseReporter { override onBegin(config: FullConfig, suite: Suite) { super.onBegin(config, suite); console.log(this.generateStartingMessage()); - if (this.liveTerminal) - console.log(); + console.log(); } override onStdOut(chunk: string | Buffer, test?: TestCase, result?: TestResult) { @@ -51,8 +46,8 @@ class LineReporter extends BaseReporter { private _dumpToStdio(test: TestCase | undefined, chunk: string | Buffer, stream: NodeJS.WriteStream) { if (this.config.quiet) return; - if (this.liveTerminal) - stream.write(lineUp + erase); + if (!process.env.PW_TEST_DEBUG_REPORTERS) + stream.write(`\u001B[1A\u001B[2K`); if (test && this._lastTest !== test) { // Write new header for the output. const title = colors.gray(formatTestTitle(this.config, test)); @@ -85,8 +80,8 @@ class LineReporter extends BaseReporter { override onTestEnd(test: TestCase, result: TestResult) { super.onTestEnd(test, result); if (!this.willRetry(test) && (test.outcome() === 'flaky' || test.outcome() === 'unexpected')) { - if (this.liveTerminal) - process.stdout.write(lineUp + erase); + if (!process.env.PW_TEST_DEBUG_REPORTERS) + process.stdout.write(`\u001B[1A\u001B[2K`); console.log(formatFailure(this.config, test, { index: ++this._failures }).message); @@ -95,27 +90,20 @@ class LineReporter extends BaseReporter { } private _updateLine(test: TestCase, result: TestResult, step?: TestStep) { - // 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 + prefix + this.fitToScreen(title, prefix) + '\n'); - } else { - if (percent !== this._lastPercent) - process.stdout.write(`[${percent}%] ${title}\n`); - } - this._lastPercent = percent; + if (process.env.PW_TEST_DEBUG_REPORTERS) + process.stdout.write(`${prefix + title}\n`); + else + process.stdout.write(`\u001B[1A\u001B[2K${prefix + this.fitToScreen(title, prefix)}\n`); } override async onEnd(result: FullResult) { + if (!process.env.PW_TEST_DEBUG_REPORTERS) + process.stdout.write(`\u001B[1A\u001B[2K`); await super.onEnd(result); - if (this.liveTerminal) - process.stdout.write(lineUp + erase); - else - 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 8797d78088..7d7789413c 100644 --- a/packages/playwright-test/src/reporters/list.ts +++ b/packages/playwright-test/src/reporters/list.ts @@ -28,9 +28,11 @@ class ListReporter extends BaseReporter { private _lastRow = 0; private _testRows = new Map(); private _needNewLine = false; + private readonly _liveTerminal: string | boolean | undefined; constructor(options: { omitFailures?: boolean } = {}) { super(options); + this._liveTerminal = process.stdout.isTTY || !!process.env.PWTEST_TTY_WIDTH; } printsToStdio() { @@ -44,7 +46,7 @@ class ListReporter extends BaseReporter { } onTestBegin(test: TestCase, result: TestResult) { - if (this.liveTerminal) { + if (this._liveTerminal) { if (this._needNewLine) { this._needNewLine = false; process.stdout.write('\n'); @@ -68,7 +70,7 @@ class ListReporter extends BaseReporter { } onStepBegin(test: TestCase, result: TestResult, step: TestStep) { - if (!this.liveTerminal) + if (!this._liveTerminal) return; if (step.category !== 'test.step') return; @@ -76,7 +78,7 @@ class ListReporter extends BaseReporter { } onStepEnd(test: TestCase, result: TestResult, step: TestStep) { - if (!this.liveTerminal) + if (!this._liveTerminal) return; if (step.category !== 'test.step') return; @@ -88,7 +90,7 @@ class ListReporter extends BaseReporter { return; const text = chunk.toString('utf-8'); this._needNewLine = text[text.length - 1] !== '\n'; - if (this.liveTerminal) { + if (this._liveTerminal) { const newLineCount = text.split('\n').length - 1; this._lastRow += newLineCount; } @@ -117,7 +119,7 @@ class ListReporter extends BaseReporter { text += this._retrySuffix(result) + colors.dim(` (${milliseconds(result.duration)})`); } - if (this.liveTerminal) { + if (this._liveTerminal) { this._updateTestLine(test, text, prefix); } else { if (this._needNewLine) { diff --git a/tests/playwright-test/hooks.spec.ts b/tests/playwright-test/hooks.spec.ts index 78a2a55d6a..8e4a4eb940 100644 --- a/tests/playwright-test/hooks.spec.ts +++ b/tests/playwright-test/hooks.spec.ts @@ -631,7 +631,7 @@ test('should not hang and report results when worker process suddenly exits duri test('failing due to afterall', () => {}); test.afterAll(() => { process.exit(0); }); ` - }, { reporter: 'line' }, { PLAYWRIGHT_LIVE_TERMINAL: '1' }); + }, { reporter: 'line' }); expect(result.exitCode).toBe(1); expect(result.passed).toBe(0); expect(result.failed).toBe(1); diff --git a/tests/playwright-test/playwright-test-fixtures.ts b/tests/playwright-test/playwright-test-fixtures.ts index 7443e4728e..44858cc90a 100644 --- a/tests/playwright-test/playwright-test-fixtures.ts +++ b/tests/playwright-test/playwright-test-fixtures.ts @@ -284,10 +284,6 @@ export function stripAnsi(str: string): string { return str.replace(asciiRegex, ''); } -export function trimLineEnds(text: string): string { - return text.split('\n').map(line => line.trimEnd()).join('\n'); -} - export function countTimes(s: string, sub: string): number { let result = 0; for (let index = 0; index !== -1;) { diff --git a/tests/playwright-test/reporter-json.spec.ts b/tests/playwright-test/reporter-json.spec.ts index 8fdc3b2f11..2702c008c0 100644 --- a/tests/playwright-test/reporter-json.spec.ts +++ b/tests/playwright-test/reporter-json.spec.ts @@ -230,7 +230,7 @@ test('should add line in addition to file json without CI', async ({ runInlineTe expect(1).toBe(1); }); `, - }, { reporter: '' }, { PLAYWRIGHT_LIVE_TERMINAL: '1' }); + }, { reporter: '' }, { PW_TEST_DEBUG_REPORTERS: '1' }); expect(result.exitCode).toBe(0); 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 f1af9f6048..56ba9efc96 100644 --- a/tests/playwright-test/reporter-line.spec.ts +++ b/tests/playwright-test/reporter-line.spec.ts @@ -14,203 +14,73 @@ * limitations under the License. */ -import { test, expect, stripAnsi, trimLineEnds } from './playwright-test-fixtures'; +import { test, expect, stripAnsi } from './playwright-test-fixtures'; -test('should work with tty', async ({ runInlineTest }, testInfo) => { +test('render unexpected after retry', async ({ runInlineTest }) => { const result = await runInlineTest({ 'a.test.js': ` const { test } = pwt; - test.skip('skipped test', async ({}) => { - }); - test('flaky test', async ({}, testInfo) => { - expect(testInfo.retry).toBe(1); - }); - test('passing test', async ({}) => { - }); - test('failing test', async ({}) => { + test('one', async ({}) => { expect(1).toBe(0); }); `, - }, { retries: '1', reporter: 'line', workers: '1' }, { - PLAYWRIGHT_LIVE_TERMINAL: '1', - FORCE_COLOR: '0', - PW_TEST_DEBUG_REPORTERS: '1', - }); + }, { retries: 3, reporter: 'line' }); + const text = stripAnsi(result.output); + expect(text).toContain('[1/1] a.test.js:6:7 › one'); + expect(text).toContain('[2/1] (retries) a.test.js:6:7 › one (retry #1)'); + expect(text).toContain('[3/1] (retries) a.test.js:6:7 › one (retry #2)'); + expect(text).toContain('[4/1] (retries) a.test.js:6:7 › one (retry #3)'); + expect(text).toContain('1 failed'); + expect(text).toContain('1) a.test'); + expect(text).not.toContain('2) a.test'); + expect(text).toContain('Retry #1 ----'); + expect(text).toContain('Retry #2 ----'); + expect(text).toContain('Retry #3 ----'); expect(result.exitCode).toBe(1); - 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 - - Expected: 1 - Received: 0 - - 7 | }); - 8 | test('flaky test', async ({}, testInfo) => { - > 9 | expect(testInfo.retry).toBe(1); - | ^ - 10 | }); - 11 | test('passing test', async ({}) => { - 12 | }); - - at ${testInfo.outputPath('a.test.js')}:9:32 - - -[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 - - Expected: 0 - Received: 1 - - 12 | }); - 13 | test('failing test', async ({}) => { - > 14 | expect(1).toBe(0); - | ^ - 15 | }); - 16 | - - at ${testInfo.outputPath('a.test.js')}:14:19 - - Retry #1 --------------------------------------------------------------------------------------- - - Error: expect(received).toBe(expected) // Object.is equality - - Expected: 0 - Received: 1 - - 12 | }); - 13 | test('failing test', async ({}) => { - > 14 | expect(1).toBe(0); - | ^ - 15 | }); - 16 | - - at ${testInfo.outputPath('a.test.js')}:14:19 - - - - 1 failed - a.test.js:13:7 › failing test ================================================================== - 1 flaky - a.test.js:8:7 › flaky test ===================================================================== - 1 skipped - 1 passed`)); }); -test('should work with non-tty', async ({ runInlineTest }, testInfo) => { +test('render flaky', async ({ runInlineTest }) => { const result = await runInlineTest({ 'a.test.js': ` const { test } = pwt; - test.skip('skipped test', async ({}) => { - }); - test('flaky test', async ({}, testInfo) => { - expect(testInfo.retry).toBe(1); - }); - test('passing test', async ({}) => { - }); - test('failing test', async ({}) => { - expect(1).toBe(0); + test('one', async ({}, testInfo) => { + expect(testInfo.retry).toBe(3); }); `, - }, { retries: '1', reporter: 'line', workers: '1' }, { - FORCE_COLOR: '0', - PW_TEST_DEBUG_REPORTERS: '1', - }); - expect(result.exitCode).toBe(1); - expect(trimLineEnds(result.output)).toContain(trimLineEnds(`Running 4 tests using 1 worker -[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 - - Expected: 1 - Received: 0 - - 7 | }); - 8 | test('flaky test', async ({}, testInfo) => { - > 9 | expect(testInfo.retry).toBe(1); - | ^ - 10 | }); - 11 | test('passing test', async ({}) => { - 12 | }); - - at ${testInfo.outputPath('a.test.js')}:9:32 - - -[99%] a.test.js:11:7 › passing test - 2) a.test.js:13:7 › failing test ================================================================= - - Error: expect(received).toBe(expected) // Object.is equality - - Expected: 0 - Received: 1 - - 12 | }); - 13 | test('failing test', async ({}) => { - > 14 | expect(1).toBe(0); - | ^ - 15 | }); - 16 | - - at ${testInfo.outputPath('a.test.js')}:14:19 - - Retry #1 --------------------------------------------------------------------------------------- - - Error: expect(received).toBe(expected) // Object.is equality - - Expected: 0 - Received: 1 - - 12 | }); - 13 | test('failing test', async ({}) => { - > 14 | expect(1).toBe(0); - | ^ - 15 | }); - 16 | - - at ${testInfo.outputPath('a.test.js')}:14:19 - - -[100%] - - 1 failed - a.test.js:13:7 › failing test ================================================================== - 1 flaky - a.test.js:8:7 › flaky test ===================================================================== - 1 skipped - 1 passed`)); -}); - -test('should spare status updates in non-tty mode', async ({ runInlineTest }) => { - const result = await runInlineTest({ - 'a.test.js': ` - const { test } = pwt; - for (let i = 0; i < 300; i++) { - test('test' + i, () => {}); - } - `, - }, { reporter: 'line', workers: '1' }, { - FORCE_COLOR: '0', - PW_TEST_DEBUG_REPORTERS: '1', - }); + }, { retries: 3, reporter: 'line' }); + const text = stripAnsi(result.output); + expect(text).toContain('1 flaky'); expect(result.exitCode).toBe(0); - 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')); +}); + +test('should print flaky failures', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.ts': ` + const { test } = pwt; + test('foobar', async ({}, testInfo) => { + expect(testInfo.retry).toBe(1); + }); + ` + }, { retries: '1', reporter: 'line' }); + expect(result.exitCode).toBe(0); + expect(result.flaky).toBe(1); + expect(stripAnsi(result.output)).toContain('expect(testInfo.retry).toBe(1)'); +}); + +test('should work on CI', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.test.js': ` + const { test } = pwt; + test('one', async ({}) => { + expect(1).toBe(0); + }); + `, + }, { reporter: 'line' }, { CI: '1' }); + const text = stripAnsi(result.output); + expect(text).toContain('[1/1] a.test.js:6:7 › one'); + expect(text).toContain('1 failed'); + expect(text).toContain('1) a.test'); + expect(result.exitCode).toBe(1); }); test('should print output', async ({ runInlineTest }) => { diff --git a/tests/playwright-test/reporter-list.spec.ts b/tests/playwright-test/reporter-list.spec.ts index b7af07b2a0..d3a1d06d36 100644 --- a/tests/playwright-test/reporter-list.spec.ts +++ b/tests/playwright-test/reporter-list.spec.ts @@ -66,7 +66,7 @@ test('render steps', async ({ runInlineTest }) => { }); }); `, - }, { reporter: 'list' }, { PW_TEST_DEBUG_REPORTERS: '1', PLAYWRIGHT_LIVE_TERMINAL: '1' }); + }, { reporter: 'list' }, { PW_TEST_DEBUG_REPORTERS: '1', PWTEST_TTY_WIDTH: '80' }); const text = stripAnsi(result.output); const lines = text.split('\n').filter(l => l.startsWith('0 :')); lines.pop(); // Remove last item that contains [v] and time in ms. @@ -94,7 +94,7 @@ test('render retries', async ({ runInlineTest }) => { expect(testInfo.retry).toBe(1); }); `, - }, { reporter: 'list', retries: '1' }, { PW_TEST_DEBUG_REPORTERS: '1', PLAYWRIGHT_LIVE_TERMINAL: '1' }); + }, { reporter: 'list', retries: '1' }, { PW_TEST_DEBUG_REPORTERS: '1', PWTEST_TTY_WIDTH: '80' }); const text = stripAnsi(result.output); const lines = text.split('\n').filter(l => l.startsWith('0 :') || l.startsWith('1 :')).map(l => l.replace(/[\dm]+s/, 'XXms')); @@ -123,7 +123,7 @@ test('should truncate long test names', async ({ runInlineTest }) => { test.skip('skipped very long name', async () => { }); `, - }, { reporter: 'list', retries: 0 }, { PLAYWRIGHT_LIVE_TERMINAL: '1', PWTEST_TTY_WIDTH: 50 }); + }, { reporter: 'list', retries: 0 }, { PWTEST_TTY_WIDTH: 50 }); expect(result.exitCode).toBe(1); const lines = stripAnsi(result.output).split('\n').slice(3, 11);