cherry-pick(release-1.14): show stdio for failures in terminal report…

…ers (#8187) PR #8150 SHA 44cdda4 Fixes #7900 Co-authored-by: Dmitry Gozman <[email protected]>
microsoft · Aug 13, 2021 · b9a1823 · b9a1823
1 parent cba2f05
commit b9a1823
Show file tree

Hide file tree

Showing 8 changed files with 88 additions and 23 deletions.
diff --git a/src/test/reporters/base.ts b/src/test/reporters/base.ts
@@ -25,28 +25,37 @@ import { FullConfig, TestCase, Suite, TestResult, TestError, Reporter, FullResul
 
 const stackUtils = new StackUtils();
 
+type TestResultOutput = { chunk: string | Buffer, type: 'stdout' | 'stderr' };
+const kOutputSymbol = Symbol('output');
+
 export class BaseReporter implements Reporter  {
   duration = 0;
   config!: FullConfig;
   suite!: Suite;
   result!: FullResult;
   fileDurations = new Map<string, number>();
   monotonicStartTime: number = 0;
+  private printTestOutput = !process.env.PWTEST_SKIP_TEST_OUTPUT;
 
   onBegin(config: FullConfig, suite: Suite) {
     this.monotonicStartTime = monotonicTime();
     this.config = config;
     this.suite = suite;
   }
 
-  onStdOut(chunk: string | Buffer) {
-    if (!this.config.quiet)
-      process.stdout.write(chunk);
+  onStdOut(chunk: string | Buffer, test?: TestCase, result?: TestResult) {
+    this._appendOutput({ chunk, type: 'stdout' }, result);
+  }
+
+  onStdErr(chunk: string | Buffer, test?: TestCase, result?: TestResult) {
+    this._appendOutput({ chunk, type: 'stderr' }, result);
   }
 
-  onStdErr(chunk: string | Buffer) {
-    if (!this.config.quiet)
-      process.stderr.write(chunk);
+  private _appendOutput(output: TestResultOutput, result: TestResult | undefined) {
+    if (!result)
+      return;
+    (result as any)[kOutputSymbol] = (result as any)[kOutputSymbol] || [];
+    (result as any)[kOutputSymbol].push(output);
   }
 
   onTestEnd(test: TestCase, result: TestResult) {
@@ -133,7 +142,7 @@ export class BaseReporter implements Reporter  {
 
   private _printFailures(failures: TestCase[]) {
     failures.forEach((test, index) => {
-      console.log(formatFailure(this.config, test, index + 1));
+      console.log(formatFailure(this.config, test, index + 1, this.printTestOutput));
     });
   }
 
@@ -142,7 +151,7 @@ export class BaseReporter implements Reporter  {
   }
 }
 
-export function formatFailure(config: FullConfig, test: TestCase, index?: number): string {
+export function formatFailure(config: FullConfig, test: TestCase, index?: number, stdio?: boolean): string {
   const tokens: string[] = [];
   tokens.push(formatTestHeader(config, test, '  ', index));
   for (const result of test.results) {
@@ -155,6 +164,17 @@ export function formatFailure(config: FullConfig, test: TestCase, index?: number
       tokens.push(colors.gray(pad(`    Retry #${result.retry}${statusSuffix}`, '-')));
     }
     tokens.push(...resultTokens);
+    const output = ((result as any)[kOutputSymbol] || []) as TestResultOutput[];
+    if (stdio && output.length) {
+      const outputText = output.map(({ chunk, type }) => {
+        const text = chunk.toString('utf8');
+        if (type === 'stderr')
+          return colors.red(stripAnsiEscapes(text));
+        return text;
+      }).join('');
+      tokens.push('');
+      tokens.push(colors.gray(pad('--- Test output', '-')) + '\n\n' + outputText + '\n' + pad('', '-'));
+    }
   }
   tokens.push('');
   return tokens.join('\n');
@@ -219,7 +239,9 @@ function formatError(error: TestError, file?: string) {
 }
 
 function pad(line: string, char: string): string {
-  return line + ' ' + colors.gray(char.repeat(Math.max(0, 100 - line.length - 1)));
+  if (line)
+    line += ' ';
+  return line + colors.gray(char.repeat(Math.max(0, 100 - line.length)));
 }
 
 function indent(lines: string, tab: string) {
@@ -244,6 +266,6 @@ function monotonicTime(): number {
 }
 
 const asciiRegex = 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 stripAscii(str: string): string {
+export function stripAnsiEscapes(str: string): string {
   return str.replace(asciiRegex, '');
 }
diff --git a/src/test/reporters/dot.ts b/src/test/reporters/dot.ts
@@ -21,6 +21,18 @@ import { FullResult, TestCase, TestResult } from '../../../types/testReporter';
 class DotReporter extends BaseReporter {
   private _counter = 0;
 
+  onStdOut(chunk: string | Buffer, test?: TestCase, result?: TestResult) {
+    super.onStdOut(chunk, test, result);
+    if (!this.config.quiet)
+      process.stdout.write(chunk);
+  }
+
+  onStdErr(chunk: string | Buffer, test?: TestCase, result?: TestResult) {
+    super.onStdErr(chunk, test, result);
+    if (!this.config.quiet)
+      process.stderr.write(chunk);
+  }
+
   onTestEnd(test: TestCase, result: TestResult) {
     super.onTestEnd(test, result);
     if (this._counter === 80) {

diff --git a/src/test/reporters/junit.ts b/src/test/reporters/junit.ts
@@ -18,7 +18,7 @@ import fs from 'fs';
 import path from 'path';
 import { FullConfig, FullResult, Reporter, Suite, TestCase } from '../../../types/testReporter';
 import { monotonicTime } from '../util';
-import { formatFailure, formatTestTitle, stripAscii } from './base';
+import { formatFailure, formatTestTitle, stripAnsiEscapes } from './base';
 
 class JUnitReporter implements Reporter {
   private config!: FullConfig;
@@ -142,7 +142,7 @@ class JUnitReporter implements Reporter {
           message: `${path.basename(test.location.file)}:${test.location.line}:${test.location.column} ${test.title}`,
           type: 'FAILURE',
         },
-        text: stripAscii(formatFailure(this.config, test))
+        text: stripAnsiEscapes(formatFailure(this.config, test))
       });
     }
     for (const result of test.results) {

diff --git a/src/test/reporters/line.ts b/src/test/reporters/line.ts
@@ -30,11 +30,13 @@ class LineReporter extends BaseReporter {
     console.log();
   }
 
-  onStdOut(chunk: string | Buffer, test?: TestCase) {
+  onStdOut(chunk: string | Buffer, test?: TestCase, result?: TestResult) {
+    super.onStdOut(chunk, test, result);
     this._dumpToStdio(test, chunk, process.stdout);
   }
 
-  onStdErr(chunk: string | Buffer, test?: TestCase) {
+  onStdErr(chunk: string | Buffer, test?: TestCase, result?: TestResult) {
+    super.onStdErr(chunk, test, result);
     this._dumpToStdio(test, chunk, process.stderr);
   }
 

diff --git a/src/test/reporters/list.ts b/src/test/reporters/list.ts
@@ -27,7 +27,6 @@ const POSITIVE_STATUS_MARK = DOES_NOT_SUPPORT_UTF8_IN_TERMINAL ? 'ok' : '✓';
 const NEGATIVE_STATUS_MARK = DOES_NOT_SUPPORT_UTF8_IN_TERMINAL ? 'x' : '✘';
 
 class ListReporter extends BaseReporter {
-  private _failure = 0;
   private _lastRow = 0;
   private _testRows = new Map<TestCase, number>();
   private _needNewLine = false;
@@ -44,16 +43,18 @@ class ListReporter extends BaseReporter {
         process.stdout.write('\n');
         this._lastRow++;
       }
-      process.stdout.write('     ' + colors.gray(formatTestTitle(this.config, test) + ': ') + '\n');
+      process.stdout.write('     ' + colors.gray(formatTestTitle(this.config, test)) + '\n');
     }
     this._testRows.set(test, this._lastRow++);
   }
 
-  onStdOut(chunk: string | Buffer, test?: TestCase) {
+  onStdOut(chunk: string | Buffer, test?: TestCase, result?: TestResult) {
+    super.onStdOut(chunk, test, result);
     this._dumpToStdio(test, chunk, process.stdout);
   }
 
-  onStdErr(chunk: string | Buffer, test?: TestCase) {
+  onStdErr(chunk: string | Buffer, test?: TestCase, result?: TestResult) {
+    super.onStdErr(chunk, test, result);
     this._dumpToStdio(test, chunk, process.stdout);
   }
 
@@ -76,13 +77,13 @@ class ListReporter extends BaseReporter {
     const title = formatTestTitle(this.config, test);
     let text = '';
     if (result.status === 'skipped') {
-      text = colors.green('  - ') + colors.cyan(title);
+      text = colors.green('  -  ') + colors.cyan(title);
     } else {
       const statusMark = ('  ' + (result.status === 'passed' ? POSITIVE_STATUS_MARK : NEGATIVE_STATUS_MARK)).padEnd(5);
       if (result.status === test.expectedStatus)
         text = '\u001b[2K\u001b[0G' + colors.green(statusMark) + colors.gray(title) + duration;
       else
-        text = '\u001b[2K\u001b[0G' + colors.red(`${statusMark}${++this._failure}) ` + title) + duration;
+        text = '\u001b[2K\u001b[0G' + colors.red(statusMark + title) + duration;
     }
 
     const testRow = this._testRows.get(test)!;

diff --git a/tests/playwright-test/base-reporter.spec.ts b/tests/playwright-test/base-reporter.spec.ts
@@ -16,6 +16,7 @@
 
 import { test, expect, stripAscii } from './playwright-test-fixtures';
 import * as path from 'path';
+import colors from 'colors/safe';
 
 test('handle long test names', async ({ runInlineTest }) => {
   const title = 'title'.repeat(30);
@@ -158,3 +159,25 @@ test('should not print slow tests', async ({ runInlineTest }) => {
   expect(result.passed).toBe(4);
   expect(stripAscii(result.output)).not.toContain('Slow test');
 });
+
+test('should print stdio for failures', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.test.js': `
+      const { test } = pwt;
+      test('fails', async ({}) => {
+        console.log('my log 1');
+        console.error('my error');
+        console.log('my log 2');
+        expect(1).toBe(2);
+      });
+    `,
+  }, {}, { PWTEST_SKIP_TEST_OUTPUT: '' });
+  expect(result.exitCode).toBe(1);
+  expect(result.failed).toBe(1);
+  expect(result.output).toContain('Test output');
+  expect(result.output).toContain([
+    'my log 1\n',
+    colors.red('my error\n'),
+    'my log 2\n',
+  ].join(''));
+});
diff --git a/tests/playwright-test/list-reporter.spec.ts b/tests/playwright-test/list-reporter.spec.ts
@@ -32,14 +32,18 @@ test('render each test with project name', async ({ runInlineTest }) => {
       test('passes', async ({}) => {
         expect(0).toBe(0);
       });
+      test.skip('skipped', async () => {
+      });
     `,
   }, { reporter: 'list' });
   const text = stripAscii(result.output);
   const positiveStatusMarkPrefix = process.platform === 'win32' ? 'ok' : '✓ ';
   const negativateStatusMarkPrefix = process.platform === 'win32' ? 'x ' : '✘ ';
-  expect(text).toContain(`${negativateStatusMarkPrefix} 1) [foo] › a.test.ts:6:7 › fails`);
-  expect(text).toContain(`${negativateStatusMarkPrefix} 2) [bar] › a.test.ts:6:7 › fails`);
+  expect(text).toContain(`${negativateStatusMarkPrefix} [foo] › a.test.ts:6:7 › fails`);
+  expect(text).toContain(`${negativateStatusMarkPrefix} [bar] › a.test.ts:6:7 › fails`);
   expect(text).toContain(`${positiveStatusMarkPrefix} [foo] › a.test.ts:9:7 › passes`);
   expect(text).toContain(`${positiveStatusMarkPrefix} [bar] › a.test.ts:9:7 › passes`);
+  expect(text).toContain(`-  [foo] › a.test.ts:12:12 › skipped`);
+  expect(text).toContain(`-  [bar] › a.test.ts:12:12 › skipped`);
   expect(result.exitCode).toBe(1);
 });
diff --git a/tests/playwright-test/playwright-test-fixtures.ts b/tests/playwright-test/playwright-test-fixtures.ts
@@ -145,10 +145,11 @@ async function runPlaywrightTest(baseDir: string, params: any, env: Env, options
   const testProcess = spawn('node', args, {
     env: {
       ...process.env,
-      ...env,
       PLAYWRIGHT_JSON_OUTPUT_NAME: reportFile,
       PWTEST_CACHE_DIR: cacheDir,
       PWTEST_CLI_ALLOW_TEST_COMMAND: '1',
+      PWTEST_SKIP_TEST_OUTPUT: '1',
+      ...env,
     },
     cwd: baseDir
   });