diff --git a/.github/workflows/auto_roll.yml b/.github/workflows/auto_roll.yml index e6781dd055..fdce9a1031 100644 --- a/.github/workflows/auto_roll.yml +++ b/.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*" diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 483565c187..572898bc6f 100644 --- a/.github/workflows/tests.yml +++ b/.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*" diff --git a/test-runner/src/cli.ts b/test-runner/src/cli.ts index 33740d9dc6..606b8d74c2 100644 --- a/test-runner/src/cli.ts +++ b/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 ', 'Only run tests matching this string or regexp', '.*') - .option('-j, --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 ', '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 ', 'Specify reporter to use, comma-separated, can be "dot", "list", "json"', 'dot') + .option('--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 }; diff --git a/test-runner/src/reporters/base.ts b/test-runner/src/reporters/base.ts index 12af020e6b..01e7095d8d 100644 --- a/test-runner/src/reporters/base.ts +++ b/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'); diff --git a/test-runner/src/reporters/dot.ts b/test-runner/src/reporters/dot.ts index 1e4fe32069..dc98f00904 100644 --- a/test-runner/src/reporters/dot.ts +++ b/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; } } diff --git a/test-runner/src/reporters/pytest.ts b/test-runner/src/reporters/pytest.ts index e9de390f0b..9e4debb979 100644 --- a/test-runner/src/reporters/pytest.ts +++ b/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; } } diff --git a/test-runner/src/runner.ts b/test-runner/src/runner.ts index 1fd938fe06..dae49bfed7 100644 --- a/test-runner/src/runner.ts +++ b/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. diff --git a/test-runner/src/runnerConfig.ts b/test-runner/src/runnerConfig.ts index 91aa18d39e..1be76f2bf0 100644 --- a/test-runner/src/runnerConfig.ts +++ b/test-runner/src/runnerConfig.ts @@ -24,6 +24,7 @@ export type RunnerConfig = { debug?: boolean; quiet?: boolean; grep?: string; + retries: number, trialRun?: boolean; updateSnapshots?: boolean; }; diff --git a/test-runner/test/assets/retry-failures.js b/test-runner/test/assets/retry-failures.js new file mode 100644 index 0000000000..f29af76049 --- /dev/null +++ b/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); + } +}); diff --git a/test-runner/test/exit-code.spec.ts b/test-runner/test/exit-code.spec.ts index 4ab44c6bca..50f182892a 100644 --- a/test-runner/test/exit-code.spec.ts +++ b/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) }; }