From fa536786f22bed5bbd5c0d0a3453fc0b2d92c7ca Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 27 Sep 2021 15:58:26 -0700 Subject: [PATCH] fix(test runner): proper serial mode with beforeAll/afterAll failures (#9183) --- src/test/dispatcher.ts | 61 ++++++++++--------- src/test/workerRunner.ts | 4 +- tests/playwright-test/test-serial.spec.ts | 72 +++++++++++++++++++++++ 3 files changed, 109 insertions(+), 28 deletions(-) diff --git a/src/test/dispatcher.ts b/src/test/dispatcher.ts index 7f5a2260a9..8a3e76ab04 100644 --- a/src/test/dispatcher.ts +++ b/src/test/dispatcher.ts @@ -126,7 +126,9 @@ export class Dispatcher { worker.stop(); worker.didFail = true; - const retryCandidates = new Set(); + const failedTestIds = new Set(); + if (params.failedTestId) + failedTestIds.add(params.failedTestId); // In case of fatal error, report first remaining test as failing with this error, // and all others as skipped. @@ -142,7 +144,7 @@ export class Dispatcher { result.error = params.fatalError; result.status = first ? 'failed' : 'skipped'; this._reportTestEnd(test, result); - retryCandidates.add(test._id); + failedTestIds.add(test._id); first = false; } if (first) { @@ -156,40 +158,45 @@ export class Dispatcher { remaining = []; } - if (params.failedTestId) { - retryCandidates.add(params.failedTestId); + const retryCandidates = new Set(); + const serialSuitesWithFailures = new Set(); + + for (const failedTestId of failedTestIds) { + retryCandidates.add(failedTestId); let outermostSerialSuite: Suite | undefined; - for (let parent = this._testById.get(params.failedTestId)!.test.parent; parent; parent = parent.parent) { + for (let parent = this._testById.get(failedTestId)!.test.parent; parent; parent = parent.parent) { if (parent._parallelMode === 'serial') outermostSerialSuite = parent; } + if (outermostSerialSuite) + serialSuitesWithFailures.add(outermostSerialSuite); + } - if (outermostSerialSuite) { - // Failed test belongs to a serial suite. We should skip all future tests - // from the same serial suite. - remaining = remaining.filter(test => { - let parent = test.parent; - while (parent && parent !== outermostSerialSuite) - parent = parent.parent; + // We have failed tests that belong to a serial suite. + // We should skip all future tests from the same serial suite. + remaining = remaining.filter(test => { + let parent = test.parent; + while (parent && !serialSuitesWithFailures.has(parent)) + parent = parent.parent; - // Does not belong to the same serial suite, keep it. - if (!parent) - return true; + // Does not belong to the failed serial suite, keep it. + if (!parent) + return true; - // Emulate a "skipped" run, and drop this test from remaining. - const { result } = this._testById.get(test._id)!; - this._reporter.onTestBegin?.(test, result); - result.status = 'skipped'; - this._reportTestEnd(test, result); - return false; - }); + // Emulate a "skipped" run, and drop this test from remaining. + const { result } = this._testById.get(test._id)!; + this._reporter.onTestBegin?.(test, result); + result.status = 'skipped'; + this._reportTestEnd(test, result); + return false; + }); - // Add all tests from the same serial suite for possible retry. - // These will only be retried together, because they have the same - // "retries" setting and the same number of previous runs. - outermostSerialSuite.allTests().forEach(test => retryCandidates.add(test._id)); - } + for (const serialSuite of serialSuitesWithFailures) { + // Add all tests from faiiled serial suites for possible retry. + // These will only be retried together, because they have the same + // "retries" setting and the same number of previous runs. + serialSuite.allTests().forEach(test => retryCandidates.add(test._id)); } for (const testId of retryCandidates) { diff --git a/src/test/workerRunner.ts b/src/test/workerRunner.ts index fe300a8101..f9e62f78fa 100644 --- a/src/test/workerRunner.ts +++ b/src/test/workerRunner.ts @@ -32,6 +32,8 @@ import { DeadlineRunner, raceAgainstDeadline } from '../utils/async'; const removeFolderAsync = util.promisify(rimraf); +type TestData = { testId: string, testInfo: TestInfoImpl, type: 'test' | 'beforeAll' | 'afterAll' }; + export class WorkerRunner extends EventEmitter { private _params: WorkerInitParams; private _loader!: Loader; @@ -47,7 +49,7 @@ export class WorkerRunner extends EventEmitter { private _isStopped = false; private _runFinished = Promise.resolve(); private _currentDeadlineRunner: DeadlineRunner | undefined; - _currentTest: { testId: string, testInfo: TestInfoImpl, type: 'test' | 'beforeAll' | 'afterAll' } | null = null; + _currentTest: TestData | null = null; constructor(params: WorkerInitParams) { super(); diff --git a/tests/playwright-test/test-serial.spec.ts b/tests/playwright-test/test-serial.spec.ts index 054b5ddcb0..40e8761b1e 100644 --- a/tests/playwright-test/test-serial.spec.ts +++ b/tests/playwright-test/test-serial.spec.ts @@ -100,6 +100,78 @@ test('test.describe.serial should work with retry', async ({ runInlineTest }) => ]); }); +test('test.describe.serial should work with retry and beforeAll failure', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.test.ts': ` + const { test } = pwt; + test.describe.serial('serial suite', () => { + test('test1', async ({}) => { + console.log('\\n%%test1'); + }); + + test.describe('inner suite', () => { + test.beforeAll(async ({}, testInfo) => { + console.log('\\n%%beforeAll'); + expect(testInfo.retry).toBe(1); + }); + test('test2', async ({}) => { + console.log('\\n%%test2'); + }); + }); + }); + `, + }, { retries: 1 }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(1); + expect(result.flaky).toBe(1); + expect(result.failed).toBe(0); + expect(result.skipped).toBe(0); + expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ + '%%test1', + '%%beforeAll', + '%%test1', + '%%beforeAll', + '%%test2', + ]); +}); + +test('test.describe.serial should work with retry and afterAll failure', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.test.ts': ` + const { test } = pwt; + test.describe.serial('serial suite', () => { + test.describe('inner suite', () => { + let firstRun = false; + test('test1', async ({}, testInfo) => { + console.log('\\n%%test1'); + firstRun = testInfo.retry === 0; + }); + test.afterAll(async ({}, testInfo) => { + console.log('\\n%%afterAll'); + expect(firstRun).toBe(false); + }); + }); + + test('test2', async ({}) => { + console.log('\\n%%test2'); + }); + }); + `, + }, { retries: 1 }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(1); + expect(result.flaky).toBe(1); + expect(result.failed).toBe(0); + expect(result.skipped).toBe(0); + expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ + '%%test1', + '%%afterAll', + '%%test1', + '%%afterAll', + '%%test2', + ]); +}); + test('test.describe.serial.only should work', async ({ runInlineTest }) => { const result = await runInlineTest({ 'a.test.ts': `