From 91672595f2ed772f26dc7b6fbd08e7cce28cd7cc Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 24 Feb 2022 12:39:28 -0800 Subject: [PATCH] fix(reporters): normalize usage of isTTY, env.CI and debug env variables (#12295) - `stdout.isTTY` controls whether list reporter updates lines or just adds them; - `env.CI` is used in a few places to affect the defaults: - whether to open interactive html; - default reporter dot/line; - default terminal reporter added to non-terminal reporters; - `env.PWTEST_SKIP_TEST_OUTPUT` is removed; - `env.PW_TEST_DEBUG_REPORTERS` is introduced specifically for tests. --- .../playwright-test/src/reporters/base.ts | 2 +- .../playwright-test/src/reporters/html.ts | 2 +- .../playwright-test/src/reporters/line.ts | 8 ++--- .../playwright-test/src/reporters/list.ts | 4 +-- packages/playwright-test/src/runner.ts | 2 +- tests/playwright-test/config.spec.ts | 2 +- tests/playwright-test/golden.spec.ts | 6 ++-- .../playwright-test-fixtures.ts | 6 ++-- tests/playwright-test/reporter-html.spec.ts | 36 +++++++++---------- tests/playwright-test/reporter-json.spec.ts | 21 +++++++++-- tests/playwright-test/reporter-line.spec.ts | 4 +-- tests/playwright-test/reporter-list.spec.ts | 6 ++-- tests/playwright-test/stdio.spec.ts | 2 +- 13 files changed, 59 insertions(+), 42 deletions(-) diff --git a/packages/playwright-test/src/reporters/base.ts b/packages/playwright-test/src/reporters/base.ts index 987f677298..12c4d24910 100644 --- a/packages/playwright-test/src/reporters/base.ts +++ b/packages/playwright-test/src/reporters/base.ts @@ -108,7 +108,7 @@ export class BaseReporter implements Reporter { } protected fitToScreen(line: string, suffix?: string): string { - const ttyWidth = this._ttyWidthForTest || (process.env.PWTEST_SKIP_TEST_OUTPUT ? 80 : process.stdout.columns || 0); + const ttyWidth = this._ttyWidthForTest || process.stdout.columns || 0; if (!ttyWidth) { // Guard against the case where we cannot determine available width. return line; diff --git a/packages/playwright-test/src/reporters/html.ts b/packages/playwright-test/src/reporters/html.ts index 0ddf74ff1e..095b3d92a2 100644 --- a/packages/playwright-test/src/reporters/html.ts +++ b/packages/playwright-test/src/reporters/html.ts @@ -149,7 +149,7 @@ class HtmlReporter implements Reporter { const builder = new HtmlBuilder(reportFolder); const { ok, singleTestId } = await builder.build(reports); - if (process.env.PWTEST_SKIP_TEST_OUTPUT || process.env.CI) + if (process.env.CI) return; const shouldOpen = this._open === 'always' || (!ok && this._open === 'on-failure'); diff --git a/packages/playwright-test/src/reporters/line.ts b/packages/playwright-test/src/reporters/line.ts index d3c8971459..4b089e2232 100644 --- a/packages/playwright-test/src/reporters/line.ts +++ b/packages/playwright-test/src/reporters/line.ts @@ -46,7 +46,7 @@ class LineReporter extends BaseReporter { private _dumpToStdio(test: TestCase | undefined, chunk: string | Buffer, stream: NodeJS.WriteStream) { if (this.config.quiet) return; - if (!process.env.PWTEST_SKIP_TEST_OUTPUT) + if (!process.env.PW_TEST_DEBUG_REPORTERS) stream.write(`\u001B[1A\u001B[2K`); if (test && this._lastTest !== test) { // Write new header for the output. @@ -66,13 +66,13 @@ class LineReporter extends BaseReporter { 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})` : ''; - if (process.env.PWTEST_SKIP_TEST_OUTPUT) + if (process.env.PW_TEST_DEBUG_REPORTERS) process.stdout.write(`${title + suffix}\n`); else process.stdout.write(`\u001B[1A\u001B[2K${this.fitToScreen(title, suffix) + colors.yellow(suffix)}\n`); if (!this.willRetry(test) && (test.outcome() === 'flaky' || test.outcome() === 'unexpected')) { - if (!process.env.PWTEST_SKIP_TEST_OUTPUT) + if (!process.env.PW_TEST_DEBUG_REPORTERS) process.stdout.write(`\u001B[1A\u001B[2K`); console.log(formatFailure(this.config, test, { index: ++this._failures @@ -82,7 +82,7 @@ class LineReporter extends BaseReporter { } override async onEnd(result: FullResult) { - if (!process.env.PWTEST_SKIP_TEST_OUTPUT) + if (!process.env.PW_TEST_DEBUG_REPORTERS) process.stdout.write(`\u001B[1A\u001B[2K`); await super.onEnd(result); this.epilogue(false); diff --git a/packages/playwright-test/src/reporters/list.ts b/packages/playwright-test/src/reporters/list.ts index 553a07df49..5e63c5a98b 100644 --- a/packages/playwright-test/src/reporters/list.ts +++ b/packages/playwright-test/src/reporters/list.ts @@ -33,7 +33,7 @@ class ListReporter extends BaseReporter { constructor(options: { omitFailures?: boolean } = {}) { super(options); - this._liveTerminal = process.stdout.isTTY || process.env.PWTEST_SKIP_TEST_OUTPUT || !!process.env.PWTEST_TTY_WIDTH; + this._liveTerminal = process.stdout.isTTY || !!process.env.PWTEST_TTY_WIDTH; } printsToStdio() { @@ -129,7 +129,7 @@ class ListReporter extends BaseReporter { } private _updateTestLine(test: TestCase, line: string, suffix: string) { - if (process.env.PWTEST_SKIP_TEST_OUTPUT) + if (process.env.PW_TEST_DEBUG_REPORTERS) this._updateTestLineForTest(test, line, suffix); else this._updateTestLineForTTY(test, line, suffix); diff --git a/packages/playwright-test/src/runner.ts b/packages/playwright-test/src/runner.ts index 36db82dad6..ad3b53f29e 100644 --- a/packages/playwright-test/src/runner.ts +++ b/packages/playwright-test/src/runner.ts @@ -128,7 +128,7 @@ export class Runner { if (list) reporters.unshift(new ListModeReporter()); else - reporters.unshift(process.stdout.isTTY && !process.env.CI ? new LineReporter({ omitFailures: true }) : new DotReporter({ omitFailures: true })); + reporters.unshift(!process.env.CI ? new LineReporter({ omitFailures: true }) : new DotReporter({ omitFailures: true })); } return new Multiplexer(reporters); } diff --git a/tests/playwright-test/config.spec.ts b/tests/playwright-test/config.spec.ts index 58f26b914f..e418b8c21d 100644 --- a/tests/playwright-test/config.spec.ts +++ b/tests/playwright-test/config.spec.ts @@ -386,7 +386,7 @@ test('should work with undefined values and base', async ({ runInlineTest }) => expect(testInfo.config.updateSnapshots).toBe('missing'); }); ` - }, {}, { CI: '1' }); + }); expect(result.exitCode).toBe(0); expect(result.passed).toBe(1); diff --git a/tests/playwright-test/golden.spec.ts b/tests/playwright-test/golden.spec.ts index 3f9aba99cc..ddebe109d3 100644 --- a/tests/playwright-test/golden.spec.ts +++ b/tests/playwright-test/golden.spec.ts @@ -214,7 +214,7 @@ test('should write missing expectations locally twice and continue', async ({ ru expect(stackLines.length).toBe(0); }); -test('shouldn\'t write missing expectations locally for negated matcher', async ({ runInlineTest }, testInfo) => { +test('should not write missing expectations for negated matcher', async ({ runInlineTest }, testInfo) => { const result = await runInlineTest({ ...files, 'a.spec.js': ` @@ -223,7 +223,7 @@ test('shouldn\'t write missing expectations locally for negated matcher', async expect('Hello world').not.toMatchSnapshot('snapshot.txt'); }); ` - }, {}, { CI: '' }); + }); expect(result.exitCode).toBe(1); const snapshotOutputPath = testInfo.outputPath('a.spec.js-snapshots/snapshot.txt'); @@ -690,7 +690,7 @@ test('should write missing expectations with sanitized snapshot name', async ({ expect('Hello world').toMatchSnapshot('../../snapshot!.txt'); }); ` - }, {}, { CI: '' }); + }); expect(result.exitCode).toBe(1); const snapshotOutputPath = testInfo.outputPath('a.spec.js-snapshots/-snapshot-.txt'); diff --git a/tests/playwright-test/playwright-test-fixtures.ts b/tests/playwright-test/playwright-test-fixtures.ts index 6a02d0aae8..8f0fc97537 100644 --- a/tests/playwright-test/playwright-test-fixtures.ts +++ b/tests/playwright-test/playwright-test-fixtures.ts @@ -120,17 +120,17 @@ async function runPlaywrightTest(childProcess: CommonFixtures['childProcess'], b ...process.env, PLAYWRIGHT_JSON_OUTPUT_NAME: reportFile, PWTEST_CACHE_DIR: cacheDir, - PWTEST_SKIP_TEST_OUTPUT: '1', - ...env, + CI: undefined, + PW_TEST_HTML_REPORT_OPEN: undefined, PLAYWRIGHT_DOCKER: undefined, PW_GRID: undefined, - PW_TEST_HTML_REPORT_OPEN: undefined, PW_TEST_REPORTER: undefined, PW_TEST_REPORTER_WS_ENDPOINT: undefined, PW_TEST_SOURCE_TRANSFORM: undefined, PW_TEST_SOURCE_TRANSFORM_SCOPE: undefined, PW_OUT_OF_PROCESS_DRIVER: undefined, NODE_OPTIONS: undefined, + ...env, }, cwd: baseDir, }); diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts index c617d9dd13..0a213067b0 100644 --- a/tests/playwright-test/reporter-html.spec.ts +++ b/tests/playwright-test/reporter-html.spec.ts @@ -51,7 +51,7 @@ test('should generate report', async ({ runInlineTest, showReport, page }) => { expect(testInfo.retry).toBe(1); }); `, - }, { reporter: 'dot,html', retries: 1 }); + }, { reporter: 'dot,html', retries: 1 }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); await showReport(); @@ -80,7 +80,7 @@ test('should not throw when attachment is missing', async ({ runInlineTest, page testInfo.attachments.push({ name: 'screenshot', path: screenshot, contentType: 'image/png' }); }); `, - }, { reporter: 'dot,html' }); + }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(0); expect(result.passed).toBe(1); @@ -108,7 +108,7 @@ test('should include image diff', async ({ runInlineTest, page, showReport }) => await expect(screenshot).toMatchSnapshot('expected.png'); }); `, - }, { reporter: 'dot,html' }); + }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); @@ -145,7 +145,7 @@ test('should not include image diff with non-images', async ({ runInlineTest, pa await expect(screenshot).toMatchSnapshot('expected'); }); `, - }, { reporter: 'dot,html' }); + }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); @@ -173,7 +173,7 @@ test('should include screenshot on failure', async ({ runInlineTest, page, showR await expect(true).toBeFalsy(); }); `, - }, { reporter: 'dot,html' }); + }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); @@ -196,7 +196,7 @@ test('should include stdio', async ({ runInlineTest, page, showReport }) => { await expect(true).toBeFalsy(); }); `, - }, { reporter: 'dot,html' }); + }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); @@ -216,7 +216,7 @@ test('should highlight error', async ({ runInlineTest, page, showReport }) => { await expect(true).toBeFalsy(); }); `, - }, { reporter: 'dot,html' }); + }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); @@ -236,7 +236,7 @@ test('should show trace source', async ({ runInlineTest, page, showReport }) => await page.evaluate('2 + 2'); }); `, - }, { reporter: 'dot,html' }); + }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(0); expect(result.passed).toBe(1); @@ -269,7 +269,7 @@ test('should show trace title', async ({ runInlineTest, page, showReport }) => { await page.evaluate('2 + 2'); }); `, - }, { reporter: 'dot,html' }); + }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(0); expect(result.passed).toBe(1); @@ -293,7 +293,7 @@ test('should show multi trace source', async ({ runInlineTest, page, server, sho await request.dispose(); }); `, - }, { reporter: 'dot,html' }); + }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(0); expect(result.passed).toBe(1); @@ -328,7 +328,7 @@ test('should show timed out steps', async ({ runInlineTest, page, showReport }) }); }); `, - }, { reporter: 'dot,html' }); + }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(1); expect(result.passed).toBe(0); @@ -350,7 +350,7 @@ test('should render annotations', async ({ runInlineTest, page, showReport }) => test.skip(true, 'I am not interested in this test'); }); `, - }, { reporter: 'dot,html' }); + }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(0); expect(result.skipped).toBe(1); @@ -389,7 +389,7 @@ test('should render text attachments as text', async ({ runInlineTest, page, sho }); }); `, - }, { reporter: 'dot,html' }); + }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(0); await showReport(); @@ -417,7 +417,7 @@ test('should strikethough textual diff', async ({ runInlineTest, showReport, pag expect('new').toMatchSnapshot('snapshot.txt'); }); ` - }, { reporter: 'dot,html' }); + }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(1); await showReport(); await page.click('text="is a test"'); @@ -442,7 +442,7 @@ test('should strikethough textual diff with commonalities', async ({ runInlineTe expect('newcommon').toMatchSnapshot('snapshot.txt'); }); ` - }, { reporter: 'dot,html' }); + }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(1); await showReport(); await page.click('text="is a test"'); @@ -460,7 +460,7 @@ test('should differentiate repeat-each test cases', async ({ runInlineTest, show throw new Error('ouch'); }); ` - }, { 'reporter': 'dot,html', 'repeat-each': 3 }); + }, { 'reporter': 'dot,html', 'repeat-each': 3 }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(1); await showReport(); @@ -485,7 +485,7 @@ test('should group similar / loop steps', async ({ runInlineTest, showReport, pa expect(2).toEqual(2); }); ` - }, { 'reporter': 'dot,html' }); + }, { 'reporter': 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(0); await showReport(); @@ -504,7 +504,7 @@ test('open tests from required file', async ({ runInlineTest, showReport, page } test('sample', async ({}) => { expect(2).toBe(2); }); `, 'a.spec.js': `require('./inner')` - }, { 'reporter': 'dot,html' }); + }, { 'reporter': 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' }); expect(result.exitCode).toBe(0); await showReport(); await expect(page.locator('text=a.spec.js')).toBeVisible(); diff --git a/tests/playwright-test/reporter-json.spec.ts b/tests/playwright-test/reporter-json.spec.ts index fa2ed96c46..98a49274ca 100644 --- a/tests/playwright-test/reporter-json.spec.ts +++ b/tests/playwright-test/reporter-json.spec.ts @@ -202,7 +202,7 @@ test('should have error position in results', async ({ expect(result.report.suites[0].specs[0].tests[0].results[0].errorLocation.column).toBe(23); }); -test('should add dot in addition to file json', async ({ runInlineTest }, testInfo) => { +test('should add dot in addition to file json with CI', async ({ runInlineTest }, testInfo) => { const result = await runInlineTest({ 'playwright.config.ts': ` module.exports = { reporter: [['json', { outputFile: 'a.json' }]] }; @@ -213,8 +213,25 @@ test('should add dot in addition to file json', async ({ runInlineTest }, testIn expect(1).toBe(1); }); `, - }, { reporter: '' }); + }, { reporter: '' }, { CI: '1' }); expect(result.exitCode).toBe(0); expect(stripAnsi(result.output)).toContain('·'); expect(fs.existsSync(testInfo.outputPath('a.json'))).toBeTruthy(); }); + +test('should add line in addition to file json without CI', async ({ runInlineTest }, testInfo) => { + const result = await runInlineTest({ + 'playwright.config.ts': ` + module.exports = { reporter: [['json', { outputFile: 'a.json' }]] }; + `, + 'a.test.js': ` + const { test } = pwt; + test('one', async ({}) => { + expect(1).toBe(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 8febdb5d04..982185cb84 100644 --- a/tests/playwright-test/reporter-line.spec.ts +++ b/tests/playwright-test/reporter-line.spec.ts @@ -67,7 +67,7 @@ test('should print flaky failures', async ({ runInlineTest }) => { expect(stripAnsi(result.output)).toContain('expect(testInfo.retry).toBe(1)'); }); -test('should work without tty', async ({ runInlineTest }) => { +test('should work on CI', async ({ runInlineTest }) => { const result = await runInlineTest({ 'a.test.js': ` const { test } = pwt; @@ -75,7 +75,7 @@ test('should work without tty', async ({ runInlineTest }) => { expect(1).toBe(0); }); `, - }, { reporter: 'line' }, { PWTEST_TTY_WIDTH: '0', PWTEST_SKIP_TEST_OUTPUT: undefined }); + }, { 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'); diff --git a/tests/playwright-test/reporter-list.spec.ts b/tests/playwright-test/reporter-list.spec.ts index a060da3441..6725021f98 100644 --- a/tests/playwright-test/reporter-list.spec.ts +++ b/tests/playwright-test/reporter-list.spec.ts @@ -63,7 +63,7 @@ test('render steps', async ({ runInlineTest }) => { }); }); `, - }, { reporter: 'list' }); + }, { 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. @@ -91,7 +91,7 @@ test('render retries', async ({ runInlineTest }) => { expect(testInfo.retry).toBe(1); }); `, - }, { reporter: 'list', retries: '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')); const positiveStatusMarkPrefix = process.platform === 'win32' ? 'ok' : '✓ '; @@ -121,7 +121,7 @@ test('should truncate long test names', async ({ runInlineTest }) => { test.skip('skipped very long name', async () => { }); `, - }, { reporter: 'list', retries: 0 }, { PWTEST_TTY_WIDTH: 50, PWTEST_SKIP_TEST_OUTPUT: undefined }); + }, { reporter: 'list', retries: 0 }, { PWTEST_TTY_WIDTH: 50 }); const text = stripAnsi(result.output); const positiveStatusMarkPrefix = process.platform === 'win32' ? 'ok' : '✓ '; const negativateStatusMarkPrefix = process.platform === 'win32' ? 'x ' : '✘ '; diff --git a/tests/playwright-test/stdio.spec.ts b/tests/playwright-test/stdio.spec.ts index 434e1988f8..51bfbf36cb 100644 --- a/tests/playwright-test/stdio.spec.ts +++ b/tests/playwright-test/stdio.spec.ts @@ -73,6 +73,6 @@ test('should ignore stdio when quiet', async ({ runInlineTest }) => { console.error('\\n%% stderr in a test'); }); ` - }, { reporter: 'list' }, { PWTEST_SKIP_TEST_OUTPUT: '' }); + }, { reporter: 'list' }); expect(result.output).not.toContain('%%'); });