From cb14de7a5b2716be7e225be1fca707ff97d7d77b Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Tue, 7 Nov 2023 14:09:40 -0800 Subject: [PATCH] chore: do not use ansi outsite of TTY (#27979) Fixes https://github.com/microsoft/playwright/issues/27891 --- packages/playwright/src/reporters/base.ts | 33 ++++++++---- packages/playwright/src/reporters/dot.ts | 3 +- packages/playwright/src/reporters/html.ts | 4 +- .../src/reporters/internalReporter.ts | 3 +- packages/playwright/src/reporters/line.ts | 3 +- packages/playwright/src/reporters/list.ts | 53 ++++++++++--------- 6 files changed, 57 insertions(+), 42 deletions(-) diff --git a/packages/playwright/src/reporters/base.ts b/packages/playwright/src/reporters/base.ts index 15bb85159f..7e94cb2f6a 100644 --- a/packages/playwright/src/reporters/base.ts +++ b/packages/playwright/src/reporters/base.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { colors, ms as milliseconds, parseStackTraceLine } from 'playwright-core/lib/utilsBundle'; +import { colors as realColors, ms as milliseconds, parseStackTraceLine } from 'playwright-core/lib/utilsBundle'; import path from 'path'; import type { FullConfig, TestCase, Suite, TestResult, TestError, FullResult, TestStep, Location } from '../../types/testReporter'; import type { SuitePrivate } from '../../types/reporterPrivate'; @@ -45,6 +45,28 @@ type TestSummary = { fatalErrors: TestError[]; }; +export const isTTY = !!process.env.PWTEST_TTY_WIDTH || process.stdout.isTTY; +export const ttyWidth = process.env.PWTEST_TTY_WIDTH ? parseInt(process.env.PWTEST_TTY_WIDTH, 10) : process.stdout.columns || 0; +let useColors = isTTY; +if (process.env.DEBUG_COLORS === '0' + || process.env.DEBUG_COLORS === 'false' + || process.env.FORCE_COLOR === '0' + || process.env.FORCE_COLOR === 'false') + useColors = false; +else if (process.env.DEBUG_COLORS || process.env.FORCE_COLOR) + useColors = true; + +export const colors = useColors ? realColors : { + bold: (t: string) => t, + cyan: (t: string) => t, + dim: (t: string) => t, + gray: (t: string) => t, + green: (t: string) => t, + red: (t: string) => t, + yellow: (t: string) => t, + enabled: false, +}; + export class BaseReporter implements ReporterV2 { config!: FullConfig; suite!: Suite; @@ -52,13 +74,11 @@ export class BaseReporter implements ReporterV2 { result!: FullResult; private fileDurations = new Map(); private _omitFailures: boolean; - private readonly _ttyWidthForTest: number; private _fatalErrors: TestError[] = []; private _failureCount: number = 0; constructor(options: { omitFailures?: boolean } = {}) { this._omitFailures = options.omitFailures || false; - this._ttyWidthForTest = parseInt(process.env.PWTEST_TTY_WIDTH || '', 10); } version(): 'v2' { @@ -128,12 +148,7 @@ export class BaseReporter implements ReporterV2 { return true; } - protected ttyWidth() { - return this._ttyWidthForTest || process.stdout.columns || 0; - } - protected fitToScreen(line: string, prefix?: string): string { - const ttyWidth = this.ttyWidth(); if (!ttyWidth) { // Guard against the case where we cannot determine available width. return line; @@ -473,7 +488,7 @@ export function formatError(error: TestError, highlightCode: boolean): ErrorDeta export function separator(text: string = ''): string { if (text) text += ' '; - const columns = Math.min(100, process.stdout?.columns || 100); + const columns = Math.min(100, ttyWidth || 100); return text + colors.dim('─'.repeat(Math.max(0, columns - text.length))); } diff --git a/packages/playwright/src/reporters/dot.ts b/packages/playwright/src/reporters/dot.ts index 71599ff352..05d2da1e4e 100644 --- a/packages/playwright/src/reporters/dot.ts +++ b/packages/playwright/src/reporters/dot.ts @@ -14,8 +14,7 @@ * limitations under the License. */ -import { colors } from 'playwright-core/lib/utilsBundle'; -import { BaseReporter, formatError } from './base'; +import { colors, BaseReporter, formatError } from './base'; import type { FullResult, TestCase, TestResult, Suite, TestError } from '../../types/testReporter'; class DotReporter extends BaseReporter { diff --git a/packages/playwright/src/reporters/html.ts b/packages/playwright/src/reporters/html.ts index 731ed0be3c..ddac659269 100644 --- a/packages/playwright/src/reporters/html.ts +++ b/packages/playwright/src/reporters/html.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { colors, open } from 'playwright-core/lib/utilsBundle'; +import { open } from 'playwright-core/lib/utilsBundle'; import { MultiMap, getPackageManagerExecCommand } from 'playwright-core/lib/utils'; import fs from 'fs'; import path from 'path'; @@ -25,7 +25,7 @@ import { codeFrameColumns } from '../transform/babelBundle'; import type { FullResult, FullConfig, Location, Suite, TestCase as TestCasePublic, TestResult as TestResultPublic, TestStep as TestStepPublic, TestError } from '../../types/testReporter'; import type { SuitePrivate } from '../../types/reporterPrivate'; import { HttpServer, assert, calculateSha1, copyFileAndMakeWritable, gracefullyProcessExitDoNotHang, removeFolders, sanitizeForFilePath } from 'playwright-core/lib/utils'; -import { formatError, formatResultFailure, stripAnsiEscapes } from './base'; +import { colors, formatError, formatResultFailure, stripAnsiEscapes } from './base'; import { resolveReporterOutputPath } from '../util'; import type { Metadata } from '../../types/test'; import type { ZipFile } from 'playwright-core/lib/zipBundle'; diff --git a/packages/playwright/src/reporters/internalReporter.ts b/packages/playwright/src/reporters/internalReporter.ts index fefb089b0b..02959ab11b 100644 --- a/packages/playwright/src/reporters/internalReporter.ts +++ b/packages/playwright/src/reporters/internalReporter.ts @@ -15,11 +15,10 @@ */ import fs from 'fs'; -import { colors } from 'playwright-core/lib/utilsBundle'; import { codeFrameColumns } from '../transform/babelBundle'; import type { FullConfig, TestCase, TestError, TestResult, FullResult, TestStep } from '../../types/testReporter'; import { Suite } from '../common/test'; -import { prepareErrorStack, relativeFilePath } from './base'; +import { colors, prepareErrorStack, relativeFilePath } from './base'; import type { ReporterV2 } from './reporterV2'; import { monotonicTime } from 'playwright-core/lib/utils'; diff --git a/packages/playwright/src/reporters/line.ts b/packages/playwright/src/reporters/line.ts index 66345aa879..a44f29d02e 100644 --- a/packages/playwright/src/reporters/line.ts +++ b/packages/playwright/src/reporters/line.ts @@ -14,8 +14,7 @@ * limitations under the License. */ -import { colors } from 'playwright-core/lib/utilsBundle'; -import { BaseReporter, formatError, formatFailure, formatTestTitle } from './base'; +import { colors, BaseReporter, formatError, formatFailure, formatTestTitle } from './base'; import type { TestCase, Suite, TestResult, FullResult, TestStep, TestError } from '../../types/testReporter'; class LineReporter extends BaseReporter { diff --git a/packages/playwright/src/reporters/list.ts b/packages/playwright/src/reporters/list.ts index 71998c49d9..11f6a8b755 100644 --- a/packages/playwright/src/reporters/list.ts +++ b/packages/playwright/src/reporters/list.ts @@ -14,8 +14,8 @@ * limitations under the License. */ -import { colors, ms as milliseconds } from 'playwright-core/lib/utilsBundle'; -import { BaseReporter, formatError, formatTestTitle, stepSuffix, stripAnsiEscapes } from './base'; +import { ms as milliseconds } from 'playwright-core/lib/utilsBundle'; +import { colors, BaseReporter, formatError, formatTestTitle, isTTY, stepSuffix, stripAnsiEscapes, ttyWidth } from './base'; import type { FullResult, Suite, TestCase, TestError, TestResult, TestStep } from '../../types/testReporter'; // Allow it in the Visual Studio Code Terminal and the new Windows Terminal @@ -31,13 +31,11 @@ class ListReporter extends BaseReporter { private _resultIndex = new Map(); private _stepIndex = new Map(); private _needNewLine = false; - private readonly _liveTerminal: string | boolean | undefined; private _printSteps: boolean; constructor(options: { omitFailures?: boolean, printSteps?: boolean } = {}) { super(options); - this._printSteps = options.printSteps || !!process.env.PW_TEST_DEBUG_REPORTERS_PRINT_STEPS; - this._liveTerminal = process.stdout.isTTY || !!process.env.PWTEST_TTY_WIDTH; + this._printSteps = isTTY && (options.printSteps || !!process.env.PW_TEST_DEBUG_REPORTERS_PRINT_STEPS); } override printsToStdio() { @@ -55,16 +53,15 @@ class ListReporter extends BaseReporter { override onTestBegin(test: TestCase, result: TestResult) { super.onTestBegin(test, result); - if (this._liveTerminal) - this._maybeWriteNewLine(); - this._resultIndex.set(result, String(this._resultIndex.size + 1)); - if (this._liveTerminal) { - this._testRows.set(test, this._lastRow); - const index = this._resultIndex.get(result)!; - const prefix = this._testPrefix(index, ''); - const line = colors.dim(formatTestTitle(this.config, test)) + this._retrySuffix(result); - this._appendLine(line, prefix); - } + if (!isTTY) + return; + this._maybeWriteNewLine(); + const index = String(this._resultIndex.size + 1); + this._resultIndex.set(result, index); + this._testRows.set(test, this._lastRow); + const prefix = this._testPrefix(index, ''); + const line = colors.dim(formatTestTitle(this.config, test)) + this._retrySuffix(result); + this._appendLine(line, prefix); } override onStdOut(chunk: string | Buffer, test?: TestCase, result?: TestResult) { @@ -81,9 +78,9 @@ class ListReporter extends BaseReporter { super.onStepBegin(test, result, step); if (step.category !== 'test.step') return; - const testIndex = this._resultIndex.get(result)!; + const testIndex = this._resultIndex.get(result) || ''; if (!this._printSteps) { - if (this._liveTerminal) + if (isTTY) this._updateLine(this._testRows.get(test)!, colors.dim(formatTestTitle(this.config, test, step)) + this._retrySuffix(result), this._testPrefix(testIndex, '')); return; } @@ -93,9 +90,8 @@ class ListReporter extends BaseReporter { const stepIndex = `${testIndex}.${ordinal}`; this._stepIndex.set(step, stepIndex); - if (this._liveTerminal) + if (isTTY) { this._maybeWriteNewLine(); - if (this._liveTerminal) { this._stepRows.set(step, this._lastRow); const prefix = this._testPrefix(stepIndex, ''); const line = test.title + colors.dim(stepSuffix(step)); @@ -108,9 +104,9 @@ class ListReporter extends BaseReporter { if (step.category !== 'test.step') return; - const testIndex = this._resultIndex.get(result)!; + const testIndex = this._resultIndex.get(result) || ''; if (!this._printSteps) { - if (this._liveTerminal) + if (isTTY) this._updateLine(this._testRows.get(test)!, colors.dim(formatTestTitle(this.config, test, step.parent)) + this._retrySuffix(result), this._testPrefix(testIndex, '')); return; } @@ -137,8 +133,7 @@ class ListReporter extends BaseReporter { private _updateLineCountAndNewLineFlagForOutput(text: string) { this._needNewLine = text[text.length - 1] !== '\n'; - const ttyWidth = this.ttyWidth(); - if (!this._liveTerminal || ttyWidth === 0) + if (!ttyWidth) return; for (const ch of text) { if (ch === '\n') { @@ -168,7 +163,15 @@ class ListReporter extends BaseReporter { const title = formatTestTitle(this.config, test); let prefix = ''; let text = ''; - const index = this._resultIndex.get(result)!; + + // In TTY mode test index is incremented in onTestStart + // and in non-TTY mode it is incremented onTestEnd. + let index = this._resultIndex.get(result); + if (!index) { + index = String(this._resultIndex.size + 1); + this._resultIndex.set(result, index); + } + if (result.status === 'skipped') { prefix = this._testPrefix(index, colors.green('-')); // Do not show duration for skipped. @@ -189,7 +192,7 @@ class ListReporter extends BaseReporter { } private _updateOrAppendLine(row: number, text: string, prefix: string) { - if (this._liveTerminal) { + if (isTTY) { this._updateLine(row, text, prefix); } else { this._maybeWriteNewLine();