diff --git a/packages/playwright-test/src/worker/workerMain.ts b/packages/playwright-test/src/worker/workerMain.ts index f53a9641c0..5d5b7efd1c 100644 --- a/packages/playwright-test/src/worker/workerMain.ts +++ b/packages/playwright-test/src/worker/workerMain.ts @@ -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() { diff --git a/tests/playwright-test/basic.spec.ts b/tests/playwright-test/basic.spec.ts index 36663f02fd..18afbe2fd4 100644 --- a/tests/playwright-test/basic.spec.ts +++ b/tests/playwright-test/basic.spec.ts @@ -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': `