diff --git a/packages/playwright/src/runner/dispatcher.ts b/packages/playwright/src/runner/dispatcher.ts index a89f996fad..6fb315114e 100644 --- a/packages/playwright/src/runner/dispatcher.ts +++ b/packages/playwright/src/runner/dispatcher.ts @@ -357,27 +357,29 @@ class JobDispatcher { data.result.attachments.push(attachment); } - private _massSkipTestsFromRemaining(testIds: Set, errors: TestError[], onlyStartedTests?: boolean) { + private _failTestWithErrors(test: TestCase, errors: TestError[]) { + const runData = this._dataByTestId.get(test.id); + // There might be a single test that has started but has not finished yet. + let result: TestResult; + if (runData) { + result = runData.result; + } else { + result = test._appendTestResult(); + this._reporter.onTestBegin(test, result); + } + result.errors = [...errors]; + result.error = result.errors[0]; + result.status = errors.length ? 'failed' : 'skipped'; + this._reportTestEnd(test, result); + this._failedTests.add(test); + } + + private _massSkipTestsFromRemaining(testIds: Set, errors: TestError[]) { for (const test of this._remainingByTestId.values()) { if (!testIds.has(test.id)) continue; if (!this._failureTracker.hasReachedMaxFailures()) { - const runData = this._dataByTestId.get(test.id); - // There might be a single test that has started but has not finished yet. - let result: TestResult; - if (runData) { - result = runData.result; - } else { - if (onlyStartedTests && this._currentlyRunning) - continue; - result = test._appendTestResult(); - this._reporter.onTestBegin(test, result); - } - result.errors = [...errors]; - result.error = result.errors[0]; - result.status = errors.length ? 'failed' : 'skipped'; - this._reportTestEnd(test, result); - this._failedTests.add(test); + this._failTestWithErrors(test, errors); errors = []; // Only report errors for the first test. } this._remainingByTestId.delete(test.id); @@ -401,15 +403,14 @@ class JobDispatcher { return; } - if (params.fatalUnknownTestIds) { - const titles = params.fatalUnknownTestIds.map(testId => { - const test = this._remainingByTestId.get(testId); - return test?.titlePath().slice(1).join(' > '); - }).filter(title => !!title); - this._massSkipTestsFromRemaining(new Set(params.fatalUnknownTestIds), [{ - message: `Test(s) not found in the worker process. Make sure test titles do not change:\n${titles.join('\n')}` - }]); + for (const testId of params.fatalUnknownTestIds || []) { + const test = this._remainingByTestId.get(testId); + if (test) { + this._remainingByTestId.delete(testId); + this._failTestWithErrors(test, [{ message: `Test not found in the worker process. Make sure test title does not change.` }]); + } } + if (params.fatalErrors.length) { // In case of fatal errors, report first remaining test as failing with these errors, // and all others as skipped. @@ -417,9 +418,18 @@ class JobDispatcher { } // Handle tests that should be skipped because of the setup failure. this._massSkipTestsFromRemaining(new Set(params.skipTestsDueToSetupFailure), []); - // Handle unexpected worker exit. - if (params.unexpectedExitError) - this._massSkipTestsFromRemaining(new Set(this._remainingByTestId.keys()), [params.unexpectedExitError], true /* onlyStartedTests */); + + if (params.unexpectedExitError) { + // When worker exits during a test, we blame the test itself. + // + // The most common situation when worker exits while not running a test is: + // worker failed to require the test file (at the start) because of an exception in one of imports. + // In this case, "skip" all remaining tests, to avoid running into the same exception over and over. + if (this._currentlyRunning) + this._massSkipTestsFromRemaining(new Set([this._currentlyRunning.test.id]), [params.unexpectedExitError]); + else + this._massSkipTestsFromRemaining(new Set(this._remainingByTestId.keys()), [params.unexpectedExitError]); + } const retryCandidates = new Set(); const serialSuitesWithFailures = new Set(); diff --git a/tests/playwright-test/runner.spec.ts b/tests/playwright-test/runner.spec.ts index 4b10f678cd..ff741dc5fa 100644 --- a/tests/playwright-test/runner.spec.ts +++ b/tests/playwright-test/runner.spec.ts @@ -16,7 +16,7 @@ import fs from 'fs'; import path from 'path'; -import { test, expect, parseTestRunnerOutput } from './playwright-test-fixtures'; +import { test, expect, parseTestRunnerOutput, countTimes } from './playwright-test-fixtures'; test('it should not allow multiple tests with the same name per suite', async ({ runInlineTest }) => { const result = await runInlineTest({ @@ -377,8 +377,6 @@ test('should teardown workers that are redundant', async ({ runInlineTest }) => }); test('should not hang if test suites in worker are inconsistent with runner', async ({ runInlineTest }) => { - const oldValue = process.env.TEST_WORKER_INDEX; - delete process.env.TEST_WORKER_INDEX; const result = await runInlineTest({ 'playwright.config.ts': ` module.exports = { name: 'project-name' }; @@ -400,13 +398,13 @@ test('should not hang if test suites in worker are inconsistent with runner', as }); } `, - }, { 'workers': 1 }); - process.env.TEST_WORKER_INDEX = oldValue; + }, { 'workers': 1 }, { TEST_WORKER_INDEX: undefined }); expect(result.exitCode).toBe(1); expect(result.passed).toBe(1); - expect(result.failed).toBe(1); - expect(result.skipped).toBe(1); - expect(result.report.suites[0].specs[1].tests[0].results[0].error!.message).toBe('Test(s) not found in the worker process. Make sure test titles do not change:\nproject-name > a.spec.js > Test 1 - bar\nproject-name > a.spec.js > Test 2 - baz'); + expect(result.failed).toBe(2); + expect(result.skipped).toBe(0); + const expectedError = 'Test not found in the worker process. Make sure test title does not change.'; + expect(countTimes(result.output, expectedError)).toBe(2); // Once per each test that was missing. }); test('sigint should stop global setup', async ({ interactWithTestRunner }) => {