fix(test.fail): expect() failure should not skip future tests (#26663)

We used to stop the worker that would skip future tests. Regressed in
https://github.com/microsoft/playwright/pull/11850.

Fixes #26435.
This commit is contained in:
Dmitry Gozman 2023-08-24 08:32:14 -07:00 committed by GitHub
parent 0406e45cf3
commit c90c943154
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 50 additions and 8 deletions

View File

@ -164,28 +164,41 @@ export class WorkerMain extends ProcessRunner {
}
unhandledError(error: Error | any) {
const failWithFatalErrorAndStop = () => {
if (!this._fatalErrors.length)
this._fatalErrors.push(serializeError(error));
void this._stop();
};
// No current test - fatal error.
if (!this._currentTest)
return failWithFatalErrorAndStop();
// Usually, we do not differentiate between errors in the control flow
// and unhandled errors - both lead to the test failing. This is good for regular tests,
// so that you can, e.g. expect() from inside an event handler. The test fails,
// and we restart the worker.
//
if (this._currentTest.expectedStatus !== 'failed') {
this._currentTest._failWithError(serializeError(error), true /* isHardError */);
void this._stop();
return;
}
// However, for tests marked with test.fail(), this is a problem. Unhandled error
// could come either from the user test code (legit failure), or from a fixture or
// a test runner. In the latter case, the worker state could be messed up,
// and continuing to run tests in the same worker is problematic. Therefore,
// we turn this into a fatal error and restart the worker anyway.
//
// The only exception is the expect() error that we still consider ok.
const isExpectError = (error instanceof Error) && !!(error as any).matcherResult;
const isCurrentTestExpectedToFail = this._currentTest?.expectedStatus === 'failed';
const shouldConsiderAsTestError = isExpectError || !isCurrentTestExpectedToFail;
if (this._currentTest && shouldConsiderAsTestError) {
if (isExpectError) {
// Note: do not stop the worker, because test marked with test.fail() that fails an assertion
// is perfectly fine.
this._currentTest._failWithError(serializeError(error), true /* isHardError */);
} else {
// No current test - fatal error.
if (!this._fatalErrors.length)
this._fatalErrors.push(serializeError(error));
failWithFatalErrorAndStop();
}
void this._stop();
}
private async _loadIfNeeded() {

View File

@ -470,6 +470,35 @@ test('should allow unhandled expects in test.fail', async ({ runInlineTest }) =>
expect(result.output).not.toContain(`Error: expect`);
});
test('should not skip tests after test.fail', async ({ runInlineTest, server }) => {
const result = await runInlineTest({
'a.spec.ts': `
import { test, expect } from '@playwright/test';
test('failing', async ({}) => {
test.fail();
expect(Promise.resolve('a')).resolves.toBe('b');
await new Promise(f => setTimeout(f, 1000));
});
`,
'b.spec.ts': `
import { test, expect } from '@playwright/test';
test('passing', async ({}) => {
console.log('b-passing');
});
`,
'c.spec.ts': `
import { test, expect } from '@playwright/test';
test('passing', async ({}) => {
console.log('c-passing');
});
`,
}, { workers: '1' });
expect(result.exitCode).toBe(0);
expect(result.passed).toBe(3);
expect(result.output).toContain('b-passing');
expect(result.output).toContain('c-passing');
});
test('should support describe.skip', async ({ runInlineTest }) => {
const result = await runInlineTest({
'nested-skip.spec.ts': `