From a46aaee6e8de789f0bacd03ea038316a8acd1590 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 22 Jun 2022 17:03:54 -0700 Subject: [PATCH] fix(reporters): truncate long test titles from the start (#15052) Most useful information is at the end - test name, current step, retry. We truncate the repetitive project + suites at the start. --- .../playwright-test/src/reporters/base.ts | 42 +++++++++------- .../playwright-test/src/reporters/line.ts | 11 +++-- .../playwright-test/src/reporters/list.ts | 48 ++++++++++--------- tests/playwright-test/reporter-list.spec.ts | 14 +++--- 4 files changed, 64 insertions(+), 51 deletions(-) diff --git a/packages/playwright-test/src/reporters/base.ts b/packages/playwright-test/src/reporters/base.ts index 2cef3f2c11..3524c9a816 100644 --- a/packages/playwright-test/src/reporters/base.ts +++ b/packages/playwright-test/src/reporters/base.ts @@ -106,13 +106,13 @@ export class BaseReporter implements Reporter { this.result = result; } - protected fitToScreen(line: string, suffix?: string): string { + protected fitToScreen(line: string, prefix?: string): string { const ttyWidth = this._ttyWidthForTest || process.stdout.columns || 0; if (!ttyWidth) { // Guard against the case where we cannot determine available width. return line; } - return fitToWidth(line, ttyWidth, suffix); + return fitToWidth(line, ttyWidth, prefix); } protected generateStartingMessage() { @@ -428,26 +428,34 @@ function monotonicTime(): number { return seconds * 1000 + (nanoseconds / 1000000 | 0); } -const ansiRegex = new RegExp('[\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)|(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-ntqry=><~]))', 'g'); +const ansiRegex = new RegExp('([\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)|(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-ntqry=><~])))', 'g'); export function stripAnsiEscapes(str: string): string { return str.replace(ansiRegex, ''); } -// Leaves enough space for the "suffix" to also fit. -function fitToWidth(line: string, width: number, suffix?: string): string { - const suffixLength = suffix ? stripAnsiEscapes(suffix).length : 0; - width -= suffixLength; +// Leaves enough space for the "prefix" to also fit. +function fitToWidth(line: string, width: number, prefix?: string): string { + const prefixLength = prefix ? stripAnsiEscapes(prefix).length : 0; + width -= prefixLength; if (line.length <= width) return line; - let m; - let ansiLen = 0; - ansiRegex.lastIndex = 0; - while ((m = ansiRegex.exec(line)) !== null) { - const visibleLen = m.index - ansiLen; - if (visibleLen >= width) - break; - ansiLen += m[0].length; + + // Even items are plain text, odd items are control sequences. + const parts = line.split(ansiRegex); + const taken: string[] = []; + for (let i = parts.length - 1; i >= 0; i--) { + if (i % 2) { + // Include all control sequences to preserve formatting. + taken.push(parts[i]); + } else { + let part = parts[i].substring(parts[i].length - width); + if (part.length < parts[i].length && part.length > 0) { + // Add ellipsis if we are truncating. + part = '\u2026' + part.substring(1); + } + taken.push(part); + width -= part.length; + } } - // Truncate and reset all colors. - return line.substr(0, width + ansiLen) + '\u001b[0m'; + return taken.reverse().join(''); } diff --git a/packages/playwright-test/src/reporters/line.ts b/packages/playwright-test/src/reporters/line.ts index 1b0c1dc94c..ebb02ce9a5 100644 --- a/packages/playwright-test/src/reporters/line.ts +++ b/packages/playwright-test/src/reporters/line.ts @@ -62,13 +62,14 @@ class LineReporter extends BaseReporter { override onTestEnd(test: TestCase, result: TestResult) { super.onTestEnd(test, result); ++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})` : ''; + 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) + currentRetrySuffix; if (process.env.PW_TEST_DEBUG_REPORTERS) - process.stdout.write(`${title + suffix}\n`); + process.stdout.write(`${prefix + title}\n`); else - process.stdout.write(`\u001B[1A\u001B[2K${this.fitToScreen(title, suffix) + colors.yellow(suffix)}\n`); + process.stdout.write(`\u001B[1A\u001B[2K${prefix + this.fitToScreen(title, prefix)}\n`); if (!this.willRetry(test) && (test.outcome() === 'flaky' || test.outcome() === 'unexpected')) { if (!process.env.PW_TEST_DEBUG_REPORTERS) diff --git a/packages/playwright-test/src/reporters/list.ts b/packages/playwright-test/src/reporters/list.ts index c94db74aa4..dc7ce77047 100644 --- a/packages/playwright-test/src/reporters/list.ts +++ b/packages/playwright-test/src/reporters/list.ts @@ -52,9 +52,9 @@ class ListReporter extends BaseReporter { process.stdout.write('\n'); this._lastRow++; } - const line = ' ' + colors.gray(formatTestTitle(this.config, test)); - const suffix = this._retrySuffix(result); - process.stdout.write(this.fitToScreen(line, suffix) + suffix + '\n'); + const prefix = ' '; + const line = colors.gray(formatTestTitle(this.config, test)) + this._retrySuffix(result); + process.stdout.write(prefix + this.fitToScreen(line, prefix) + '\n'); } this._testRows.set(test, this._lastRow++); } @@ -74,7 +74,7 @@ class ListReporter extends BaseReporter { return; if (step.category !== 'test.step') return; - this._updateTestLine(test, ' ' + colors.gray(formatTestTitle(this.config, test, step)), this._retrySuffix(result)); + this._updateTestLine(test, colors.gray(formatTestTitle(this.config, test, step)) + this._retrySuffix(result), ' '); } onStepEnd(test: TestCase, result: TestResult, step: TestStep) { @@ -82,7 +82,7 @@ class ListReporter extends BaseReporter { return; if (step.category !== 'test.step') return; - this._updateTestLine(test, ' ' + colors.gray(formatTestTitle(this.config, test, step.parent)), this._retrySuffix(result)); + this._updateTestLine(test, colors.gray(formatTestTitle(this.config, test, step.parent)) + this._retrySuffix(result), ' '); } private _dumpToStdio(test: TestCase | undefined, chunk: string | Buffer, stream: NodeJS.WriteStream) { @@ -100,48 +100,52 @@ class ListReporter extends BaseReporter { override onTestEnd(test: TestCase, result: TestResult) { super.onTestEnd(test, result); - let duration = colors.dim(` (${milliseconds(result.duration)})`); const title = formatTestTitle(this.config, test); + let prefix = ''; let text = ''; if (result.status === 'skipped') { - text = colors.green(' - ') + colors.cyan(title); - duration = ''; // Do not show duration for skipped. + prefix = colors.green(' - '); + // Do not show duration for skipped. + text = colors.cyan(title) + this._retrySuffix(result); } else { const statusMark = (' ' + (result.status === 'passed' ? POSITIVE_STATUS_MARK : NEGATIVE_STATUS_MARK)).padEnd(5); - if (result.status === test.expectedStatus) - text = colors.green(statusMark) + colors.gray(title); - else - text = colors.red(statusMark + title); + if (result.status === test.expectedStatus) { + prefix = colors.green(statusMark); + text = colors.gray(title); + } else { + prefix = colors.red(statusMark); + text = colors.red(title); + } + text += this._retrySuffix(result) + colors.dim(` (${milliseconds(result.duration)})`); } - const suffix = this._retrySuffix(result) + duration; if (this._liveTerminal) { - this._updateTestLine(test, text, suffix); + this._updateTestLine(test, text, prefix); } else { if (this._needNewLine) { this._needNewLine = false; process.stdout.write('\n'); } - process.stdout.write(text + suffix); + process.stdout.write(prefix + text); process.stdout.write('\n'); } } - private _updateTestLine(test: TestCase, line: string, suffix: string) { + private _updateTestLine(test: TestCase, line: string, prefix: string) { if (process.env.PW_TEST_DEBUG_REPORTERS) - this._updateTestLineForTest(test, line, suffix); + this._updateTestLineForTest(test, line, prefix); else - this._updateTestLineForTTY(test, line, suffix); + this._updateTestLineForTTY(test, line, prefix); } - private _updateTestLineForTTY(test: TestCase, line: string, suffix: string) { + private _updateTestLineForTTY(test: TestCase, line: string, prefix: string) { const testRow = this._testRows.get(test)!; // Go up if needed if (testRow !== this._lastRow) process.stdout.write(`\u001B[${this._lastRow - testRow}A`); // Erase line, go to the start process.stdout.write('\u001B[2K\u001B[0G'); - process.stdout.write(this.fitToScreen(line, suffix) + suffix); + process.stdout.write(prefix + this.fitToScreen(line, prefix)); // Go down if needed. if (testRow !== this._lastRow) process.stdout.write(`\u001B[${this._lastRow - testRow}E`); @@ -151,9 +155,9 @@ class ListReporter extends BaseReporter { return (result.retry ? colors.yellow(` (retry #${result.retry})`) : ''); } - private _updateTestLineForTest(test: TestCase, line: string, suffix: string) { + private _updateTestLineForTest(test: TestCase, line: string, prefix: string) { const testRow = this._testRows.get(test)!; - process.stdout.write(testRow + ' : ' + line + suffix + '\n'); + process.stdout.write(testRow + ' : ' + prefix + line + '\n'); } override async onEnd(result: FullResult) { diff --git a/tests/playwright-test/reporter-list.spec.ts b/tests/playwright-test/reporter-list.spec.ts index 4ace6d7838..253ca3f47d 100644 --- a/tests/playwright-test/reporter-list.spec.ts +++ b/tests/playwright-test/reporter-list.spec.ts @@ -113,7 +113,7 @@ test('should truncate long test names', async ({ runInlineTest }) => { `, 'a.test.ts': ` const { test } = pwt; - test('fails very long name', async ({}) => { + test('failure in very long name', async ({}) => { expect(1).toBe(0); }); test('passes', async ({}) => { @@ -126,12 +126,12 @@ test('should truncate long test names', async ({ runInlineTest }) => { }, { reporter: 'list', retries: 0 }, { PWTEST_TTY_WIDTH: 50 }); const text = stripAnsi(result.output); - expect(text).toContain(`${NEGATIVE_STATUS_MARK} [foo] › a.test.ts:6:7 › fails very`); - expect(text).not.toContain(`${NEGATIVE_STATUS_MARK} [foo] › a.test.ts:6:7 › fails very long name (`); + expect(text).toContain(`${NEGATIVE_STATUS_MARK} …st.ts:6:7 › failure in very long name (`); + expect(text).not.toContain(`${NEGATIVE_STATUS_MARK} …est.ts:6:7 › fails very long name (`); expect(text).toContain(`${POSITIVE_STATUS_MARK} [foo] › a.test.ts:9:7 › passes (`); - expect(text).toContain(`${POSITIVE_STATUS_MARK} [foo] › a.test.ts:11:7 › passes 2 long`); - expect(text).not.toContain(`${POSITIVE_STATUS_MARK} [foo] › a.test.ts:11:7 › passes 2 long name (`); - expect(text).toContain(`- [foo] › a.test.ts:13:12 › skipped very long n`); - expect(text).not.toContain(`- [foo] › a.test.ts:13:12 › skipped very long na`); + expect(text).toContain(`${POSITIVE_STATUS_MARK} … › a.test.ts:11:7 › passes 2 long name (`); + expect(text).not.toContain(`${POSITIVE_STATUS_MARK} …] › a.test.ts:11:7 › passes 2 long name (`); + expect(text).toContain(`- …] › a.test.ts:13:12 › skipped very long nam`); + expect(text).not.toContain(`- …o] › a.test.ts:13:12 › skipped very long nam`); expect(result.exitCode).toBe(1); });