feat(test runner): separate interrupted status from skipped (#16124)

This commit is contained in:
Dmitry Gozman 2022-08-02 12:55:43 -07:00 committed by GitHub
parent de147fafba
commit 445fe032f5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 51 additions and 32 deletions

View File

@ -162,7 +162,7 @@ Errors thrown during test execution, if any.
## property: TestInfo.expectedStatus ## property: TestInfo.expectedStatus
* since: v1.10 * since: v1.10
- type: <[TestStatus]<"passed"|"failed"|"timedOut"|"skipped">> - type: <[TestStatus]<"passed"|"failed"|"timedOut"|"skipped"|"interrupted">>
Expected status for the currently running test. This is usually `'passed'`, except for a few cases: Expected status for the currently running test. This is usually `'passed'`, except for a few cases:
* `'skipped'` for skipped tests, e.g. with [`method: Test.skip#2`]; * `'skipped'` for skipped tests, e.g. with [`method: Test.skip#2`];
@ -461,7 +461,7 @@ Suffix used to differentiate snapshots between multiple test configurations. For
## property: TestInfo.status ## property: TestInfo.status
* since: v1.10 * since: v1.10
- type: ?<[TestStatus]<"passed"|"failed"|"timedOut"|"skipped">> - type: ?<[TestStatus]<"passed"|"failed"|"timedOut"|"skipped"|"interrupted">>
Actual status for the currently running test. Available after the test has finished in [`method: Test.afterEach`] hook and fixtures. Actual status for the currently running test. Available after the test has finished in [`method: Test.afterEach`] hook and fixtures.

View File

@ -18,7 +18,7 @@ Learn more about [test annotations](../test-annotations.md).
## property: TestCase.expectedStatus ## property: TestCase.expectedStatus
* since: v1.10 * since: v1.10
- type: <[TestStatus]<"passed"|"failed"|"timedOut"|"skipped">> - type: <[TestStatus]<"passed"|"failed"|"timedOut"|"skipped"|"interrupted">>
Expected test status. Expected test status.
* Tests marked as [`method: Test.skip#1`] or [`method: Test.fixme#1`] are expected to be `'skipped'`. * Tests marked as [`method: Test.skip#1`] or [`method: Test.fixme#1`] are expected to be `'skipped'`.

View File

@ -49,7 +49,7 @@ Start time of this particular test run.
## property: TestResult.status ## property: TestResult.status
* since: v1.10 * since: v1.10
- type: <[TestStatus]<"passed"|"failed"|"timedOut"|"skipped">> - type: <[TestStatus]<"passed"|"failed"|"timedOut"|"skipped"|"interrupted">>
The status of this test result. See also [`property: TestCase.expectedStatus`]. The status of this test result. See also [`property: TestCase.expectedStatus`].

View File

@ -18,7 +18,7 @@ import * as icons from './icons';
import './colors.css'; import './colors.css';
import './common.css'; import './common.css';
export function statusIcon(status: 'failed' | 'timedOut' | 'skipped' | 'passed' | 'expected' | 'unexpected' | 'flaky'): JSX.Element { export function statusIcon(status: 'failed' | 'timedOut' | 'skipped' | 'passed' | 'expected' | 'unexpected' | 'flaky' | 'interrupted'): JSX.Element {
switch (status) { switch (status) {
case 'failed': case 'failed':
case 'unexpected': case 'unexpected':
@ -31,6 +31,7 @@ export function statusIcon(status: 'failed' | 'timedOut' | 'skipped' | 'passed'
case 'flaky': case 'flaky':
return icons.warning(); return icons.warning();
case 'skipped': case 'skipped':
case 'interrupted':
return icons.blank(); return icons.blank();
} }
} }

View File

@ -38,7 +38,7 @@ type ErrorDetails = {
type TestSummary = { type TestSummary = {
skipped: number; skipped: number;
expected: number; expected: number;
skippedWithError: TestCase[]; interrupted: TestCase[];
unexpected: TestCase[]; unexpected: TestCase[];
flaky: TestCase[]; flaky: TestCase[];
failuresToPrint: TestCase[]; failuresToPrint: TestCase[];
@ -131,7 +131,7 @@ export class BaseReporter implements Reporter {
return fileDurations.filter(([,duration]) => duration > threshold).slice(0, count); return fileDurations.filter(([,duration]) => duration > threshold).slice(0, count);
} }
protected generateSummaryMessage({ skipped, expected, unexpected, flaky, fatalErrors }: TestSummary) { protected generateSummaryMessage({ skipped, expected, interrupted, unexpected, flaky, fatalErrors }: TestSummary) {
const tokens: string[] = []; const tokens: string[] = [];
if (fatalErrors.length) if (fatalErrors.length)
tokens.push(colors.red(` ${fatalErrors.length} fatal ${fatalErrors.length === 1 ? 'error' : 'errors'}`)); tokens.push(colors.red(` ${fatalErrors.length} fatal ${fatalErrors.length === 1 ? 'error' : 'errors'}`));
@ -140,6 +140,11 @@ export class BaseReporter implements Reporter {
for (const test of unexpected) for (const test of unexpected)
tokens.push(colors.red(formatTestHeader(this.config, test, ' '))); tokens.push(colors.red(formatTestHeader(this.config, test, ' ')));
} }
if (interrupted.length) {
tokens.push(colors.red(` ${interrupted.length} interrupted`));
for (const test of interrupted)
tokens.push(colors.red(formatTestHeader(this.config, test, ' ')));
}
if (flaky.length) { if (flaky.length) {
tokens.push(colors.yellow(` ${flaky.length} flaky`)); tokens.push(colors.yellow(` ${flaky.length} flaky`));
for (const test of flaky) for (const test of flaky)
@ -158,16 +163,21 @@ export class BaseReporter implements Reporter {
protected generateSummary(): TestSummary { protected generateSummary(): TestSummary {
let skipped = 0; let skipped = 0;
let expected = 0; let expected = 0;
const skippedWithError: TestCase[] = []; const interrupted: TestCase[] = [];
const interruptedToPrint: TestCase[] = [];
const unexpected: TestCase[] = []; const unexpected: TestCase[] = [];
const flaky: TestCase[] = []; const flaky: TestCase[] = [];
this.suite.allTests().forEach(test => { this.suite.allTests().forEach(test => {
switch (test.outcome()) { switch (test.outcome()) {
case 'skipped': { case 'skipped': {
++skipped; if (test.results.some(result => result.status === 'interrupted')) {
if (test.results.some(result => !!result.error)) if (test.results.some(result => !!result.error))
skippedWithError.push(test); interruptedToPrint.push(test);
interrupted.push(test);
} else {
++skipped;
}
break; break;
} }
case 'expected': ++expected; break; case 'expected': ++expected; break;
@ -176,11 +186,11 @@ export class BaseReporter implements Reporter {
} }
}); });
const failuresToPrint = [...unexpected, ...flaky, ...skippedWithError]; const failuresToPrint = [...unexpected, ...flaky, ...interruptedToPrint];
return { return {
skipped, skipped,
expected, expected,
skippedWithError, interrupted,
unexpected, unexpected,
flaky, flaky,
failuresToPrint, failuresToPrint,
@ -313,6 +323,11 @@ export function formatResultFailure(config: FullConfig, test: TestCase, result:
message: indent(colors.red(`Expected to fail, but passed.`), initialIndent), message: indent(colors.red(`Expected to fail, but passed.`), initialIndent),
}); });
} }
if (result.status === 'interrupted') {
errorDetails.push({
message: indent(colors.red(`Test was interrupted.`), initialIndent),
});
}
for (const error of result.errors) { for (const error of result.errors) {
const formattedError = formatError(config, error, highlightCode, test.location.file); const formattedError = formatError(config, error, highlightCode, test.location.file);

View File

@ -104,7 +104,7 @@ export type TestResult = {
steps: TestStep[]; steps: TestStep[];
errors: string[]; errors: string[];
attachments: TestAttachment[]; attachments: TestAttachment[];
status: 'passed' | 'failed' | 'timedOut' | 'skipped'; status: 'passed' | 'failed' | 'timedOut' | 'skipped' | 'interrupted';
}; };
export type TestStep = { export type TestStep = {

View File

@ -79,7 +79,7 @@ class LineReporter extends BaseReporter {
override onTestEnd(test: TestCase, result: TestResult) { override onTestEnd(test: TestCase, result: TestResult) {
super.onTestEnd(test, result); super.onTestEnd(test, result);
if (!this.willRetry(test) && (test.outcome() === 'flaky' || test.outcome() === 'unexpected')) { if (!this.willRetry(test) && (test.outcome() === 'flaky' || test.outcome() === 'unexpected' || result.status === 'interrupted')) {
if (!process.env.PW_TEST_DEBUG_REPORTERS) if (!process.env.PW_TEST_DEBUG_REPORTERS)
process.stdout.write(`\u001B[1A\u001B[2K`); process.stdout.write(`\u001B[1A\u001B[2K`);
console.log(formatFailure(this.config, test, { console.log(formatFailure(this.config, test, {

View File

@ -159,7 +159,7 @@ export class TestCase extends Base implements reporterTypes.TestCase {
} }
outcome(): 'skipped' | 'expected' | 'unexpected' | 'flaky' { outcome(): 'skipped' | 'expected' | 'unexpected' | 'flaky' {
const nonSkipped = this.results.filter(result => result.status !== 'skipped'); const nonSkipped = this.results.filter(result => result.status !== 'skipped' && result.status !== 'interrupted');
if (!nonSkipped.length) if (!nonSkipped.length)
return 'skipped'; return 'skipped';
if (nonSkipped.every(result => result.status === this.expectedStatus)) if (nonSkipped.every(result => result.status === this.expectedStatus))

View File

@ -74,9 +74,8 @@ export class WorkerRunner extends EventEmitter {
// Interrupt current action. // Interrupt current action.
this._currentTest?._timeoutManager.interrupt(); this._currentTest?._timeoutManager.interrupt();
// TODO: mark test as 'interrupted' instead.
if (this._currentTest && this._currentTest.status === 'passed') if (this._currentTest && this._currentTest.status === 'passed')
this._currentTest.status = 'skipped'; this._currentTest.status = 'interrupted';
} }
return this._runFinished; return this._runFinished;
} }

View File

@ -1294,7 +1294,7 @@ export interface FullConfig<TestArgs = {}, WorkerArgs = {}> {
webServer: TestConfigWebServer | null; webServer: TestConfigWebServer | null;
} }
export type TestStatus = 'passed' | 'failed' | 'timedOut' | 'skipped'; export type TestStatus = 'passed' | 'failed' | 'timedOut' | 'skipped' | 'interrupted';
/** /**
* `WorkerInfo` contains information about the worker that is running tests. It is available to * `WorkerInfo` contains information about the worker that is running tests. It is available to
@ -1506,7 +1506,7 @@ export interface TestInfo {
* ``` * ```
* *
*/ */
expectedStatus: "passed"|"failed"|"timedOut"|"skipped"; expectedStatus: "passed"|"failed"|"timedOut"|"skipped"|"interrupted";
/** /**
* Marks the currently running test as "should fail". Playwright Test runs this test and ensures that it is actually * Marks the currently running test as "should fail". Playwright Test runs this test and ensures that it is actually
@ -1714,7 +1714,7 @@ export interface TestInfo {
* ``` * ```
* *
*/ */
status?: "passed"|"failed"|"timedOut"|"skipped"; status?: "passed"|"failed"|"timedOut"|"skipped"|"interrupted";
/** /**
* Output written to `process.stderr` or `console.error` during the test execution. * Output written to `process.stderr` or `console.error` during the test execution.

View File

@ -424,7 +424,7 @@ test('should not reuse worker after unhandled rejection in test.fail', async ({
}, { workers: 1 }); }, { workers: 1 });
expect(result.exitCode).toBe(1); expect(result.exitCode).toBe(1);
expect(result.failed).toBe(1); expect(result.failed).toBe(1);
expect(result.skipped).toBe(1); expect(result.interrupted).toBe(1);
expect(result.output).toContain(`Error: Oh my!`); expect(result.output).toContain(`Error: Oh my!`);
expect(result.output).not.toContain(`Did not teardown test scope`); expect(result.output).not.toContain(`Did not teardown test scope`);
}); });

View File

@ -37,6 +37,7 @@ export type RunResult = {
failed: number, failed: number,
flaky: number, flaky: number,
skipped: number, skipped: number,
interrupted: number,
report: JSONReport, report: JSONReport,
results: any[], results: any[],
}; };
@ -156,7 +157,7 @@ async function runPlaywrightTest(childProcess: CommonFixtures['childProcess'], b
testProcess.onOutput = () => { testProcess.onOutput = () => {
if (options.sendSIGINTAfter && !didSendSigint && countTimes(testProcess.output, '%%SEND-SIGINT%%') >= options.sendSIGINTAfter) { if (options.sendSIGINTAfter && !didSendSigint && countTimes(testProcess.output, '%%SEND-SIGINT%%') >= options.sendSIGINTAfter) {
didSendSigint = true; didSendSigint = true;
process.kill(testProcess.process.pid, 'SIGINT'); process.kill(testProcess.process.pid!, 'SIGINT');
} }
}; };
const { exitCode } = await testProcess.exited; const { exitCode } = await testProcess.exited;
@ -176,6 +177,7 @@ async function runPlaywrightTest(childProcess: CommonFixtures['childProcess'], b
const failed = summary(/(\d+) failed/g); const failed = summary(/(\d+) failed/g);
const flaky = summary(/(\d+) flaky/g); const flaky = summary(/(\d+) flaky/g);
const skipped = summary(/(\d+) skipped/g); const skipped = summary(/(\d+) skipped/g);
const interrupted = summary(/(\d+) interrupted/g);
let report; let report;
try { try {
report = JSON.parse(fs.readFileSync(reportFile).toString()); report = JSON.parse(fs.readFileSync(reportFile).toString());
@ -205,6 +207,7 @@ async function runPlaywrightTest(childProcess: CommonFixtures['childProcess'], b
failed, failed,
flaky, flaky,
skipped, skipped,
interrupted,
report, report,
results, results,
}; };

View File

@ -657,7 +657,7 @@ test('should print expected/received on Ctrl+C', async ({ runInlineTest }) => {
}, { workers: 1 }, {}, { sendSIGINTAfter: 1 }); }, { workers: 1 }, {}, { sendSIGINTAfter: 1 });
expect(result.exitCode).toBe(130); expect(result.exitCode).toBe(130);
expect(result.passed).toBe(0); expect(result.passed).toBe(0);
expect(result.skipped).toBe(1); expect(result.interrupted).toBe(1);
expect(stripAnsi(result.output)).toContain('Expected string: "Text 2"'); expect(stripAnsi(result.output)).toContain('Expected string: "Text 2"');
expect(stripAnsi(result.output)).toContain('Received string: "Text content"'); expect(stripAnsi(result.output)).toContain('Received string: "Text content"');
}); });

View File

@ -450,7 +450,7 @@ test('should report click error on sigint', async ({ runInlineTest }) => {
expect(result.exitCode).toBe(130); expect(result.exitCode).toBe(130);
expect(result.passed).toBe(0); expect(result.passed).toBe(0);
expect(result.failed).toBe(0); expect(result.failed).toBe(0);
expect(result.skipped).toBe(1); expect(result.interrupted).toBe(1);
expect(stripAnsi(result.output)).toContain(`8 | const promise = page.click('text=Missing');`); expect(stripAnsi(result.output)).toContain(`8 | const promise = page.click('text=Missing');`);
}); });

View File

@ -29,7 +29,7 @@ test('should lead in uncaughtException when page.route raises', async ({ runInli
}); });
`, `,
}, { workers: 1 }); }, { workers: 1 });
expect(result.skipped).toBe(1); expect(result.interrupted).toBe(1);
expect(result.output).toContain('foobar'); expect(result.output).toContain('foobar');
}); });
@ -46,7 +46,7 @@ test('should lead in unhandledRejection when page.route raises', async ({ runInl
}); });
`, `,
}, { workers: 1 }); }, { workers: 1 });
expect(result.skipped).toBe(1); expect(result.interrupted).toBe(1);
expect(result.output).toContain('foobar'); expect(result.output).toContain('foobar');
}); });
@ -63,7 +63,7 @@ test('should lead in uncaughtException when context.route raises', async ({ runI
}); });
`, `,
}, { workers: 1 }); }, { workers: 1 });
expect(result.skipped).toBe(1); expect(result.interrupted).toBe(1);
expect(result.output).toContain('foobar'); expect(result.output).toContain('foobar');
}); });
@ -80,6 +80,6 @@ test('should lead in unhandledRejection when context.route raises', async ({ run
}); });
`, `,
}, { workers: 1 }); }, { workers: 1 });
expect(result.skipped).toBe(1); expect(result.interrupted).toBe(1);
expect(result.output).toContain('foobar'); expect(result.output).toContain('foobar');
}); });

View File

@ -112,7 +112,8 @@ test('sigint should stop workers', async ({ runInlineTest }) => {
expect(result.exitCode).toBe(130); expect(result.exitCode).toBe(130);
expect(result.passed).toBe(0); expect(result.passed).toBe(0);
expect(result.failed).toBe(0); expect(result.failed).toBe(0);
expect(result.skipped).toBe(4); expect(result.skipped).toBe(2);
expect(result.interrupted).toBe(2);
expect(result.output).toContain('%%SEND-SIGINT%%1'); expect(result.output).toContain('%%SEND-SIGINT%%1');
expect(result.output).toContain('%%SEND-SIGINT%%2'); expect(result.output).toContain('%%SEND-SIGINT%%2');
expect(result.output).not.toContain('%%skipped1'); expect(result.output).not.toContain('%%skipped1');
@ -177,7 +178,7 @@ test('worker interrupt should report errors', async ({ runInlineTest }) => {
expect(result.exitCode).toBe(130); expect(result.exitCode).toBe(130);
expect(result.passed).toBe(0); expect(result.passed).toBe(0);
expect(result.failed).toBe(0); expect(result.failed).toBe(0);
expect(result.skipped).toBe(1); expect(result.interrupted).toBe(1);
expect(result.output).toContain('%%SEND-SIGINT%%'); expect(result.output).toContain('%%SEND-SIGINT%%');
expect(result.output).toContain('Error: INTERRUPT'); expect(result.output).toContain('Error: INTERRUPT');
}); });

View File

@ -95,7 +95,7 @@ export interface FullConfig<TestArgs = {}, WorkerArgs = {}> {
// [internal] !!! DO NOT ADD TO THIS !!! See prior note. // [internal] !!! DO NOT ADD TO THIS !!! See prior note.
} }
export type TestStatus = 'passed' | 'failed' | 'timedOut' | 'skipped'; export type TestStatus = 'passed' | 'failed' | 'timedOut' | 'skipped' | 'interrupted';
export interface WorkerInfo { export interface WorkerInfo {
config: FullConfig; config: FullConfig;