feat(testrunner): support --retries, flaky tests (#3649)

.github/workflows/auto_roll.yml

@@ -27,7 +27,7 @@ jobs:
     # XVFB-RUN merges both STDOUT and STDERR, whereas we need only STDERR
     # Wrap `npm run` in a subshell to redirect STDERR to file.
     # Enable core dumps in the subshell.
-    - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- bash -c "ulimit -c unlimited && node test-runner/cli test/ --jobs=1 --forbid-only --timeout=30000 --reporter=dot,json"
+    - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- bash -c "ulimit -c unlimited && node test-runner/cli test/ --jobs=1 --forbid-only --retries=3 --timeout=30000 --reporter=dot,json"
       env:
         BROWSER: ${{ matrix.browser }}
         DEBUG: "pw:*,-pw:wrapped*,-pw:test*"

.github/workflows/tests.yml

@@ -37,7 +37,7 @@ jobs:
     # XVFB-RUN merges both STDOUT and STDERR, whereas we need only STDERR
     # Wrap `npm run` in a subshell to redirect STDERR to file.
     # Enable core dumps in the subshell.
-    - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- bash -c "ulimit -c unlimited && node test-runner/cli test/ --jobs=1 --forbid-only --timeout=30000 --reporter=dot,json && npm run coverage"
+    - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- bash -c "ulimit -c unlimited && node test-runner/cli test/ --jobs=1 --forbid-only --timeout=30000 --retries=3 --reporter=dot,json && npm run coverage"
       env:
         BROWSER: ${{ matrix.browser }}
         DEBUG: "pw:*,-pw:wrapped*,-pw:test*"
@@ -64,7 +64,7 @@ jobs:
     - uses: microsoft/playwright-github-action@v1
     - run: npm ci
     - run: npm run build
-    - run: node test-runner/cli test/ --jobs=1 --forbid-only --timeout=30000 --reporter=dot,json
+    - run: node test-runner/cli test/ --jobs=1 --forbid-only --timeout=30000 --retries=3 --reporter=dot,json
       env:
         BROWSER: ${{ matrix.browser }}
         DEBUG: "pw:*,-pw:wrapped*,-pw:test*"
@@ -94,7 +94,7 @@ jobs:
     - uses: microsoft/playwright-github-action@v1
     - run: npm ci
     - run: npm run build
-    - run: node test-runner/cli test/ --jobs=1 --forbid-only --timeout=30000 --reporter=dot,json
+    - run: node test-runner/cli test/ --jobs=1 --forbid-only --timeout=30000 --retries=3 --reporter=dot,json
       shell: bash
       env:
         BROWSER: ${{ matrix.browser }}
@@ -147,7 +147,7 @@ jobs:
     # XVFB-RUN merges both STDOUT and STDERR, whereas we need only STDERR
     # Wrap `npm run` in a subshell to redirect STDERR to file.
     # Enable core dumps in the subshell.
-    - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- bash -c "ulimit -c unlimited && node test-runner/cli test/ --jobs=1 --forbid-only --timeout=30000 --reporter=dot,json"
+    - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- bash -c "ulimit -c unlimited && node test-runner/cli test/ --jobs=1 --forbid-only --timeout=30000 --retries=3 --reporter=dot,json"
       if: ${{ always() }}
       env:
         BROWSER: ${{ matrix.browser }}
@@ -181,7 +181,7 @@ jobs:
     # XVFB-RUN merges both STDOUT and STDERR, whereas we need only STDERR
     # Wrap `npm run` in a subshell to redirect STDERR to file.
     # Enable core dumps in the subshell.
-    - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- bash -c "ulimit -c unlimited && node test-runner/cli test/ --jobs=1 --forbid-only --timeout=30000 --reporter=dot,json"
+    - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- bash -c "ulimit -c unlimited && node test-runner/cli test/ --jobs=1 --forbid-only --timeout=30000 --retries=3 --reporter=dot,json"
       env:
         BROWSER: ${{ matrix.browser }}
         DEBUG: "pw:*,-pw:wrapped*,-pw:test*"

test-runner/src/cli.ts

@@ -36,8 +36,9 @@ program
     .version('Version ' + /** @type {any} */ (require)('../package.json').version)
     .option('--forbid-only', 'Fail if exclusive test(s) encountered', false)
     .option('-g, --grep <grep>', 'Only run tests matching this string or regexp', '.*')
-    .option('-j, --jobs <jobs>', 'Number of concurrent jobs for --parallel; use 1 to run in serial, default: (number of CPU cores / 2)', Math.ceil(require('os').cpus().length / 2) as any)
+    .option('-j, --jobs <jobs>', 'Number of concurrent jobs for --parallel; use 1 to run in serial, default: (number of CPU cores / 2)', String(Math.ceil(require('os').cpus().length / 2)))
     .option('--reporter <reporter>', 'Specify reporter to use, comma-separated, can be "dot", "list", "json"', 'dot')
+    .option('--retries <retries>', 'Specify retry count', '0')
     .option('--trial-run', 'Only collect the matching tests and report them as passing')
     .option('--quiet', 'Suppress stdio', false)
     .option('--debug', 'Run tests in-process for debugging', false)
@@ -51,11 +52,12 @@ program
         forbidOnly: command.forbidOnly,
         quiet: command.quiet,
         grep: command.grep,
-        jobs: command.jobs,
+        jobs: parseInt(command.jobs, 10),
         outputDir: command.output,
+        retries: parseInt(command.retries, 10),
         snapshotDir: path.join(testDir, '__snapshots__'),
         testDir,
-        timeout: command.timeout,
+        timeout: parseInt(command.timeout, 10),
         trialRun: command.trialRun,
         updateSnapshots: command.updateSnapshots
       };

test-runner/src/reporters/base.ts

@@ -30,9 +30,10 @@ const stackUtils = new StackUtils();
 
 export class BaseReporter implements Reporter  {
   skipped: Test[] = [];
-  passed: { test: Test, result: TestResult }[] = [];
-  failed: { test: Test, result: TestResult }[] = [];
-  timedOut: { test: Test, result: TestResult }[] = [];
+  passed: Test[] = [];
+  flaky: Test[] = [];
+  failed: Test[] = [];
+  timedOut: Test[] = [];
   duration = 0;
   startTime: number;
   config: RunnerConfig;
@@ -66,10 +67,27 @@ export class BaseReporter implements Reporter  {
 
   onTestEnd(test: Test, result: TestResult) {
     switch (result.status) {
-      case 'skipped': this.skipped.push(test); break;
-      case 'passed': this.passed.push({ test, result }); break;
-      case 'failed': this.failed.push({ test, result }); break;
-      case 'timedOut': this.timedOut.push({ test, result }); break;
+      case 'skipped': {
+        this.skipped.push(test);
+        return;
+      }
+      case 'passed':
+        if (test.results.length === 1)
+          this.passed.push(test);
+        else
+          this.flaky.push(test);
+        return;
+      case 'failed':
+        // Fall through.
+      case 'timedOut': {
+        if (test.results.length === this.config.retries + 1) {
+          if (result.status === 'timedOut')
+            this.timedOut.push(test);
+          else
+            this.failed.push(test);
+        }
+        return;
+      }
     }
   }
 
@@ -91,6 +109,12 @@ export class BaseReporter implements Reporter  {
       this._printFailures(this.failed);
     }
 
+    if (this.flaky.length) {
+      console.log(colors.red(`  ${this.flaky.length} flaky`));
+      console.log('');
+      this._printFailures(this.flaky);
+    }
+
     if (this.timedOut.length) {
       console.log(colors.red(`  ${this.timedOut.length} timed out`));
       console.log('');
@@ -98,43 +122,48 @@ export class BaseReporter implements Reporter  {
     }
   }
 
-  private _printFailures(failures: { test: Test, result: TestResult}[]) {
-    failures.forEach(({test, result}, index) => {
-      console.log(this.formatFailure(test, result, index + 1));
+  private _printFailures(failures: Test[]) {
+    failures.forEach((test, index) => {
+      console.log(this.formatFailure(test, index + 1));
     });
   }
 
-  formatFailure(test: Test, failure: TestResult, index?: number): string {
+  formatFailure(test: Test, index?: number): string {
     const tokens: string[] = [];
     const relativePath = path.relative(process.cwd(), test.file);
     const header = `  ${index ? index + ')' : ''} ${terminalLink(relativePath, `file://${os.hostname()}${test.file}`)} › ${test.title}`;
     tokens.push(colors.bold(colors.red(header)));
-    if (failure.status === 'timedOut') {
-      tokens.push('');
-      tokens.push(indent(colors.red(`Timeout of ${test.timeout}ms exceeded.`), '    '));
-    } else {
-      const stack = failure.error.stack;
-      if (stack) {
+    for (const result of test.results) {
+      if (result.status === 'passed')
+        continue;
+      if (result.status === 'timedOut') {
         tokens.push('');
-        const messageLocation = failure.error.stack.indexOf(failure.error.message);
-        const preamble = failure.error.stack.substring(0, messageLocation + failure.error.message.length);
-        tokens.push(indent(preamble, '    '));
-        const position = positionInFile(stack, test.file);
-        if (position) {
-          const source = fs.readFileSync(test.file, 'utf8');
-          tokens.push('');
-          tokens.push(indent(codeFrameColumns(source, {
-            start: position,
-          },
-          { highlightCode: true}
-          ), '    '));
-        }
-        tokens.push('');
-        tokens.push(indent(colors.dim(stack.substring(preamble.length + 1)), '    '));
+        tokens.push(indent(colors.red(`Timeout of ${test.timeout}ms exceeded.`), '    '));
       } else {
-        tokens.push('');
-        tokens.push(indent(String(failure.error), '    '));
+        const stack = result.error.stack;
+        if (stack) {
+          tokens.push('');
+          const messageLocation = result.error.stack.indexOf(result.error.message);
+          const preamble = result.error.stack.substring(0, messageLocation + result.error.message.length);
+          tokens.push(indent(preamble, '    '));
+          const position = positionInFile(stack, test.file);
+          if (position) {
+            const source = fs.readFileSync(test.file, 'utf8');
+            tokens.push('');
+            tokens.push(indent(codeFrameColumns(source, {
+              start: position,
+            },
+            { highlightCode: true}
+            ), '    '));
+          }
+          tokens.push('');
+          tokens.push(indent(colors.dim(stack.substring(preamble.length + 1)), '    '));
+        } else {
+          tokens.push('');
+          tokens.push(indent(String(result.error), '    '));
+        }
       }
+      break;
     }
     tokens.push('');
     return tokens.join('\n');

test-runner/src/reporters/dot.ts

@@ -24,7 +24,7 @@ class DotReporter extends BaseReporter {
     switch (result.status) {
       case 'skipped': process.stdout.write(colors.yellow('∘')); break;
       case 'passed': process.stdout.write(colors.green('·')); break;
-      case 'failed': process.stdout.write(colors.red('F')); break;
+      case 'failed': process.stdout.write(colors.red(test.results.length > 1 ? '' + test.results.length : 'F')); break;
       case 'timedOut': process.stdout.write(colors.red('T')); break;
     }
   }

test-runner/src/reporters/pytest.ts

@@ -113,7 +113,7 @@ class PytestReporter extends BaseReporter {
         const row = this._append(test, title);
         row.failed = true;
         this._progress.push('F');
-        this._repaint(this.formatFailure(test, result) + '\n');
+        this._repaint(this.formatFailure(test) + '\n');
         break;
       }
     }

test-runner/src/runner.ts

@@ -119,7 +119,14 @@ export class Runner {
       }
 
       const remaining = params.remaining;
-      if (params.remaining.length)
+      if (this._config.retries) {
+        const pair = this._testById.get(params.failedTestId);
+        if (pair.test.results.length < this._config.retries + 1) {
+          pair.result = pair.test._appendResult();
+          remaining.unshift(pair.test._ordinal);
+        }
+      }
+      if (remaining.length)
         this._queue.unshift({ ...entry, ordinals: remaining });
 
       // This job is over, we just scheduled another one.

test-runner/src/runnerConfig.ts

@@ -24,6 +24,7 @@ export type RunnerConfig = {
   debug?: boolean;
   quiet?: boolean;
   grep?: string;
+  retries: number,
   trialRun?: boolean;
   updateSnapshots?: boolean;
 };

test-runner/test/assets/retry-failures.js

@@ -0,0 +1,28 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+const fs = require('fs');
+const path = require('path');
+
+it('flake', async ({}) => {
+	try {
+		fs.readFileSync(path.join(__dirname, '..', 'test-results', 'retry-failures.txt'));
+	} catch (e) {
+		// First time this fails.
+		fs.writeFileSync(path.join(__dirname, '..', 'test-results', 'retry-failures.txt'), 'TRUE');
+		expect(true).toBe(false);
+	}
+});

test-runner/test/exit-code.spec.ts

@@ -55,7 +55,7 @@ it('should access data in fixture', async () => {
 });
 
 it('should handle worker fixture timeout', async () => {
-  const result = await runTest('worker-fixture-timeout.js', 1000);
+  const result = await runTest('worker-fixture-timeout.js', { timeout: 1000 });
   expect(result.exitCode).toBe(1);
   expect(result.output).toContain('Timeout of 1000ms');
 });
@@ -81,7 +81,13 @@ it('should work with typescript', async () => {
   expect(result.exitCode).toBe(0);
 });
 
-async function runTest(filePath: string, timeout = 10000) {
+it('should retry failures', async () => {
+  const result = await runTest('retry-failures.js', { retries: 1 });
+  expect(result.exitCode).toBe(1);
+  expect(result.flaky).toBe(1);
+});
+
+async function runTest(filePath: string, params: any = {}) {
   const outputDir = path.join(__dirname, 'test-results');
   await removeFolderAsync(outputDir).catch(e => {});
 
@@ -89,8 +95,8 @@ async function runTest(filePath: string, timeout = 10000) {
     path.join(__dirname, '..', 'cli.js'),
     path.join(__dirname, 'assets', filePath),
     '--output=' + outputDir,
-    '--timeout=' + timeout,
-    '--reporter=dot,json'
+    '--reporter=dot,json',
+    ...Object.keys(params).map(key => `--${key}=${params[key]}`)
   ], {
     env: {
       ...process.env,
@@ -99,10 +105,12 @@ async function runTest(filePath: string, timeout = 10000) {
   });
   const passed = (/(\d+) passed/.exec(output.toString()) || [])[1];
   const failed = (/(\d+) failed/.exec(output.toString()) || [])[1];
+  const flaky = (/(\d+) flaky/.exec(output.toString()) || [])[1];
   return {
     exitCode: status,
     output: output.toString(),
     passed: parseInt(passed, 10),
-    failed: parseInt(failed || '0', 10)
+    failed: parseInt(failed || '0', 10),
+    flaky: parseInt(flaky || '0', 10)
   };
 }