fix(testrunner): include fixture teardown into timeout, add global timeout (#3685)

This commit is contained in:
Pavel Feldman 2020-08-28 19:27:49 -07:00 committed by GitHub
parent c47af2d4f3
commit 555a8d0d10
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
23 changed files with 206 additions and 54 deletions

View file

@ -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 --retries=3 --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 --global-timeout=5400000 --reporter=dot,json"
env:
BROWSER: ${{ matrix.browser }}
DEBUG: "pw:*,-pw:wrapped*,-pw:test*"

View file

@ -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 --retries=3 --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 --global-timeout=5400000 --retries=3 --reporter=dot,json && npm run coverage"
env:
BROWSER: ${{ matrix.browser }}
PWRUNNER_JSON_REPORT: "test-results/report.json"
@ -62,7 +62,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 --retries=3 --reporter=dot,json
- run: node test-runner/cli test/ --jobs=1 --forbid-only --timeout=30000 --global-timeout=5400000 --retries=3 --reporter=dot,json
env:
BROWSER: ${{ matrix.browser }}
PWRUNNER_JSON_REPORT: "test-results/report.json"
@ -90,7 +90,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 --retries=3 --reporter=dot,json
- run: node test-runner/cli test/ --jobs=1 --forbid-only --timeout=30000 --global-timeout=5400000 --retries=3 --reporter=dot,json
shell: bash
env:
BROWSER: ${{ matrix.browser }}
@ -141,7 +141,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 --retries=3 --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 --global-timeout=5400000 --retries=3 --reporter=dot,json"
if: ${{ always() }}
env:
BROWSER: ${{ matrix.browser }}
@ -174,7 +174,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 --retries=3 --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 --global-timeout=5400000 --retries=3 --reporter=dot,json"
env:
BROWSER: ${{ matrix.browser }}
PWWIRE: true
@ -206,7 +206,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 --retries=3 --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 --global-timeout=5400000 --retries=3 --reporter=dot,json"
env:
BROWSER: ${{ matrix.browser }}
TRACING: true

View file

@ -36,6 +36,7 @@ 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('--global-timeout <timeout>', 'Specify maximum time this test suite can run (in milliseconds), default: 0 for unlimited', '0')
.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('--repeat-each <repeat-each>', 'Specify how many times to run the tests', '1')
@ -60,6 +61,7 @@ program
snapshotDir: path.join(testDir, '__snapshots__'),
testDir,
timeout: parseInt(command.timeout, 10),
globalTimeout: parseInt(command.globalTimeout, 10),
trialRun: command.trialRun,
updateSnapshots: command.updateSnapshots
};

View file

@ -17,6 +17,7 @@
import debug from 'debug';
import { RunnerConfig } from './runnerConfig';
import { serializeError, Test, TestResult } from './test';
import { raceAgainstTimeout } from './util';
type Scope = 'test' | 'worker';
@ -152,24 +153,32 @@ export class FixturePool {
return fn(params);
}
async runTestWithFixtures(fn: Function, timeout: number, info: TestInfo) {
let timer: NodeJS.Timer;
const timerPromise = new Promise(f => timer = setTimeout(f, timeout));
async runTestWithFixturesAndTimeout(fn: Function, timeout: number, info: TestInfo) {
const { timedOut } = await raceAgainstTimeout(this._runTestWithFixtures(fn, info), timeout);
// Do not overwrite test failure upon timeout in fixture.
if (timedOut && info.result.status === 'passed')
info.result.status = 'timedOut';
}
async _runTestWithFixtures(fn: Function, info: TestInfo) {
try {
await this.resolveParametersAndRun(fn, info.config, info);
info.result.status = 'passed';
} catch (error) {
// Prefer original error to the fixture teardown error or timeout.
if (info.result.status === 'passed') {
info.result.status = 'failed';
info.result.error = serializeError(error);
}
}
try {
await Promise.race([
this.resolveParametersAndRun(fn, info.config, info).then(() => {
info.result.status = 'passed';
clearTimeout(timer);
}).catch(e => {
info.result.status = 'failed';
info.result.error = serializeError(e);
}),
timerPromise.then(() => {
info.result.status = 'timedOut';
})
]);
} finally {
await this.teardownScope('test');
} catch (error) {
// Prefer original error to the fixture teardown error or timeout.
if (info.result.status === 'passed') {
info.result.status = 'failed';
info.result.error = serializeError(error);
}
}
}
}

View file

@ -25,8 +25,10 @@ import { registerFixture as registerFixtureT, registerWorkerFixture as registerW
import { Reporter } from './reporter';
import { Runner } from './runner';
import { RunnerConfig } from './runnerConfig';
import { Suite } from './test';
import { Matrix, TestCollector } from './testCollector';
import { installTransform } from './transform';
import { raceAgainstTimeout } from './util';
export { parameters, registerParameter } from './fixtures';
export { Reporter } from './reporter';
export { RunnerConfig } from './runnerConfig';
@ -93,7 +95,15 @@ export async function run(config: RunnerConfig, files: string[], reporter: Repor
const total = suite.total();
if (!total)
return 'no-tests';
const { result, timedOut } = await raceAgainstTimeout(runTests(config, suite, reporter), config.globalTimeout);
if (timedOut) {
reporter.onTimeout(config.globalTimeout);
process.exit(1);
}
return result;
}
async function runTests(config: RunnerConfig, suite: Suite, reporter: Reporter) {
// Trial run does not need many workers, use one.
const jobs = (config.trialRun || config.debug) ? 1 : config.jobs;
const runner = new Runner(suite, { ...config, jobs }, reporter);

View file

@ -20,8 +20,9 @@ import { Suite, Test, TestResult } from './test';
export interface Reporter {
onBegin(config: RunnerConfig, suite: Suite): void;
onTestBegin(test: Test): void;
onTestStdOut(test: Test, chunk: string | Buffer);
onTestStdErr(test: Test, chunk: string | Buffer);
onTestEnd(test: Test, result: TestResult);
onTestStdOut(test: Test, chunk: string | Buffer): void;
onTestStdErr(test: Test, chunk: string | Buffer): void;
onTestEnd(test: Test, result: TestResult): void;
onTimeout(timeout: number): void;
onEnd(): void;
}

View file

@ -32,11 +32,13 @@ export class BaseReporter implements Reporter {
skipped: Test[] = [];
asExpected: Test[] = [];
unexpected = new Set<Test>();
flaky: Test[] = [];
expectedFlaky: Test[] = [];
unexpectedFlaky: Test[] = [];
duration = 0;
startTime: number;
config: RunnerConfig;
suite: Suite;
timeout: number;
constructor() {
process.on('SIGINT', async () => {
@ -76,7 +78,10 @@ export class BaseReporter implements Reporter {
this.asExpected.push(test);
} else {
// as expected after unexpected -> flaky.
this.flaky.push(test);
if (test.isFlaky())
this.expectedFlaky.push(test);
else
this.unexpectedFlaky.push(test);
}
return;
}
@ -86,6 +91,10 @@ export class BaseReporter implements Reporter {
}
}
onTimeout(timeout: number) {
this.timeout = timeout;
}
onEnd() {
this.duration = Date.now() - this.startTime;
}
@ -105,10 +114,13 @@ export class BaseReporter implements Reporter {
this._printFailures(filteredUnexpected);
}
if (this.flaky.length) {
console.log(colors.red(` ${this.flaky.length} flaky`));
console.log('');
this._printFailures(this.flaky);
const allFlaky = this.expectedFlaky.length + this.unexpectedFlaky.length;
if (allFlaky) {
console.log(colors.red(` ${allFlaky} flaky`));
if (this.unexpectedFlaky.length) {
console.log('');
this._printFailures(this.unexpectedFlaky);
}
}
const timedOut = [...this.unexpected].filter(t => t._hasResultWithStatus('timedOut'));
@ -118,6 +130,10 @@ export class BaseReporter implements Reporter {
this._printFailures(timedOut);
}
console.log('');
if (this.timeout) {
console.log(colors.red(` Timed out waiting ${this.timeout / 1000}s for the entire test run`));
console.log('');
}
}
private _printFailures(failures: Test[]) {

View file

@ -29,6 +29,11 @@ class DotReporter extends BaseReporter {
}
}
onTimeout(timeout) {
super.onTimeout(timeout);
this.onEnd();
}
onEnd() {
super.onEnd();
process.stdout.write('\n');

View file

@ -19,6 +19,11 @@ import { Suite, Test, TestResult } from '../test';
import * as fs from 'fs';
class JSONReporter extends BaseReporter {
onTimeout(timeout) {
super.onTimeout(timeout);
this.onEnd();
}
onEnd() {
super.onEnd();
const result = {

View file

@ -50,6 +50,11 @@ export class Multiplexer implements Reporter {
reporter.onTestEnd(test, result);
}
onTimeout(timeout: number) {
for (const reporter of this._reporters)
reporter.onTimeout(timeout);
}
onEnd() {
for (const reporter of this._reporters)
reporter.onEnd();

View file

@ -15,17 +15,18 @@
*/
export type RunnerConfig = {
debug?: boolean;
forbidOnly?: boolean;
globalTimeout: number;
grep?: string;
jobs: number;
outputDir: string;
quiet?: boolean;
repeatEach: number;
retries: number,
snapshotDir: string;
testDir: string;
timeout: number;
debug?: boolean;
quiet?: boolean;
grep?: string;
repeatEach: number;
retries: number,
trialRun?: boolean;
updateSnapshots?: boolean;
};

View file

@ -103,8 +103,8 @@ export class Runnable {
return this._slow || (this.parent && this.parent._isSlow());
}
_isFlaky(): boolean {
return this._flaky || (this.parent && this.parent._isFlaky());
isFlaky(): boolean {
return this._flaky || (this.parent && this.parent.isFlaky());
}
titlePath(): string[] {
@ -162,7 +162,7 @@ export class Test extends Runnable {
const hasFailedResults = !!this.results.find(r => r.status !== r.expectedStatus);
if (!hasFailedResults)
return true;
if (!this._isFlaky())
if (!this.isFlaky())
return false;
const hasPassedResults = !!this.results.find(r => r.status === r.expectedStatus);
return hasPassedResults;

View file

@ -158,7 +158,7 @@ export class TestRunner extends EventEmitter {
// We only know resolved skipped/flaky value in the worker,
// send it to the runner.
test._skipped = test._isSkipped();
test._flaky = test._isFlaky();
test._flaky = test.isFlaky();
test._slow = test._isSlow();
this.emit('testBegin', {
id,
@ -189,7 +189,7 @@ export class TestRunner extends EventEmitter {
if (!this._trialRun) {
await this._runHooks(test.parent, 'beforeEach', 'before', testInfo);
const timeout = test._isSlow() ? this._timeout * 3 : this._timeout;
await fixturePool.runTestWithFixtures(test.fn, timeout, testInfo);
await fixturePool.runTestWithFixturesAndTimeout(test.fn, timeout, testInfo);
await this._runHooks(test.parent, 'afterEach', 'after', testInfo);
} else {
result.status = result.expectedStatus;

45
test-runner/src/util.ts Normal file
View file

@ -0,0 +1,45 @@
/**
* 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.
*/
export async function raceAgainstTimeout<T>(promise: Promise<T>, timeout: number): Promise<{ result?: T, timedOut?: boolean }> {
if (!timeout)
return { result: await promise };
let timer: NodeJS.Timer;
let done = false;
let fulfill: (t: { result?: T, timedOut?: boolean }) => void;
let reject: (e: Error) => void;
const result = new Promise((f, r) => {
fulfill = f;
reject = r;
});
setTimeout(() => {
done = true;
fulfill({ timedOut: true });
}, timeout);
promise.then(result => {
clearTimeout(timer);
if (!done) {
done = true;
fulfill({ result });
}
}).catch(e => {
clearTimeout(timer);
if (!done)
reject(e);
});
return result;
}

View file

@ -0,0 +1,30 @@
/**
* 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 { registerFixture } = require('../../');
registerFixture('timeout', async ({}, runTest) => {
await runTest();
await new Promise(f => setTimeout(f, 100000));
});
it('fixture timeout', async({timeout}) => {
expect(1).toBe(1);
});
it('failing fixture timeout', async({timeout}) => {
expect(1).toBe(2);
});

View file

@ -62,10 +62,18 @@ it('should access data in fixture', async () => {
expect(testResult.stderr).toEqual([{ text: 'console.error\n' }]);
});
it('should handle fixture timeout', async () => {
const { exitCode, output, failed, timedOut } = await runTest('fixture-timeout.js', { timeout: 500 });
expect(exitCode).toBe(1);
expect(output).toContain('Timeout of 500ms');
expect(failed).toBe(1);
expect(timedOut).toBe(1);
});
it('should handle worker fixture timeout', async () => {
const result = await runTest('worker-fixture-timeout.js', { timeout: 1000 });
const result = await runTest('worker-fixture-timeout.js', { timeout: 500 });
expect(result.exitCode).toBe(1);
expect(result.output).toContain('Timeout of 1000ms');
expect(result.output).toContain('Timeout of 500ms');
});
it('should handle worker fixture error', async () => {
@ -171,6 +179,12 @@ it('should retry unhandled rejection', async () => {
expect(result.output).toContain('Unhandled rejection');
});
it('should respect global timeout', async () => {
const { exitCode, output } = await runTest('one-timeout.js', { 'timeout': 100000, 'global-timeout': 500 });
expect(exitCode).toBe(1);
expect(output).toContain('Timed out waiting 0.5s for the entire test run');
});
async function runTest(filePath: string, params: any = {}) {
const outputDir = path.join(__dirname, 'test-results');
const reportFile = path.join(outputDir, 'results.json');

View file

@ -68,7 +68,9 @@ it('should support ignoreHTTPSErrors option', async ({httpsServer, launchPersist
expect(response.ok()).toBe(true);
});
it('should support extraHTTPHeaders option', async ({server, launchPersistent}) => {
it('should support extraHTTPHeaders option', test => {
test.flaky(options.FIREFOX && !options.HEADLESS && LINUX, 'Intermittent timeout on bots');
}, async ({server, launchPersistent}) => {
const {page} = await launchPersistent({extraHTTPHeaders: { foo: 'bar' }});
const [request] = await Promise.all([
server.waitForRequest('/empty.html'),
@ -119,7 +121,7 @@ it('should restore state from userDataDir', test => {
it('should restore cookies from userDataDir', test => {
test.slow();
test.flaky(options.CHROMIUM && WIN);
test.flaky(options.CHROMIUM);
}, async ({browserType, defaultBrowserOptions, server, launchPersistent}) => {
const userDataDir = await makeUserDataDir();
const browserContext = await browserType.launchPersistentContext(userDataDir, defaultBrowserOptions);

View file

@ -131,7 +131,8 @@ it('should be isolated between frames', async ({page, server}) => {
});
it('should work in iframes that failed initial navigation', test => {
test.fail(options.CHROMIUM || options.FIREFOX);
test.fail(options.CHROMIUM);
test.fixme(options.FIREFOX);
}, async ({page}) => {
// - Firefox does not report domcontentloaded for the iframe.
// - Chromium and Firefox report empty url.

View file

@ -435,7 +435,9 @@ it('should not throw an error when evaluation does a synchronous navigation and
expect(result).toBe(undefined);
});
it('should transfer 100Mb of data from page to node.js', async ({ page }) => {
it('should transfer 100Mb of data from page to node.js', test => {
test.skip(options.WIRE);
}, async ({ page }) => {
// This is too slow with wire.
const a = await page.evaluate(() => Array(100 * 1024 * 1024 + 1).join('a'));
expect(a.length).toBe(100 * 1024 * 1024);

View file

@ -58,7 +58,7 @@ it('should handle odd values', async ({page}) => {
});
it('should handle object', test => {
test.fail(options.FIREFOX);
test.fixme(options.FIREFOX);
}, async ({page}) => {
// Firefox just does not report this error.
const [error] = await Promise.all([
@ -69,7 +69,7 @@ it('should handle object', test => {
});
it('should handle window', test => {
test.fail(options.FIREFOX);
test.fixme(options.FIREFOX);
}, async ({page}) => {
// Firefox just does not report this error.
const [error] = await Promise.all([

View file

@ -149,7 +149,7 @@ describe('permissions', suite => {
it('should support clipboard read', test => {
test.fail(options.WEBKIT);
test.fail(options.FIREFOX, 'No such permissions (requires flag) in Firefox');
test.fail(options.CHROMIUM && !options.HEADLESS);
test.fixme(options.CHROMIUM && !options.HEADLESS);
}, async ({page, server, context}) => {
await page.goto(server.EMPTY_PAGE);
expect(await getPermission(page, 'clipboard-read')).toBe('prompt');

View file

@ -78,7 +78,7 @@ it('should authenticate', async ({browserType, defaultBrowserOptions, server}) =
});
it('should exclude patterns', test => {
test.fail(options.CHROMIUM && !options.HEADLESS, 'Chromium headful crashes with CHECK(!in_frame_tree_) in RenderFrameImpl::OnDeleteFrame.');
test.flaky(options.CHROMIUM && !options.HEADLESS, 'Chromium headful crashes with CHECK(!in_frame_tree_) in RenderFrameImpl::OnDeleteFrame.');
}, async ({browserType, defaultBrowserOptions, server}) => {
server.setRoute('/target.html', async (req, res) => {
res.end('<html><title>Served by the proxy</title></html>');
@ -119,7 +119,9 @@ it('should exclude patterns', test => {
await browser.close();
});
it('should use socks proxy', async ({ browserType, defaultBrowserOptions }) => {
it('should use socks proxy', test => {
test.flaky(MAC && options.WEBKIT, 'Intermittent page.goto: The network connection was lost error on bots');
}, async ({ browserType, defaultBrowserOptions }) => {
const server = socks.createServer((info, accept, deny) => {
let socket;
if ((socket = accept(true))) {

View file

@ -204,6 +204,7 @@ describe('screencast', suite => {
it('should capture navigation', test => {
test.flaky(options.WEBKIT);
test.flaky(options.FIREFOX);
}, async ({page, tmpDir, server, videoPlayer, toImpl}) => {
const videoFile = path.join(tmpDir, 'v.webm');
await page.goto(server.PREFIX + '/background-color.html#rgb(0,0,0)');
@ -263,7 +264,8 @@ describe('screencast', suite => {
it('should fire start/stop events when page created/closed', test => {
test.slow();
}, async ({browser, tmpDir, server, toImpl}) => {
test.flaky(options.FIREFOX, 'Even slow is not slow enough');
}, async ({browser, tmpDir, toImpl}) => {
// Use server side of the context. All the code below also uses server side APIs.
const context = toImpl(await browser.newContext());
await context._enableScreencast({width: 640, height: 480, dir: tmpDir});