diff --git a/src/test/dispatcher.ts b/src/test/dispatcher.ts index 59c897df13..648e74b8bb 100644 --- a/src/test/dispatcher.ts +++ b/src/test/dispatcher.ts @@ -297,7 +297,7 @@ export class Dispatcher { return; } const { test, result, steps, stepStack } = this._testById.get(params.testId)!; - const parentStep = [...stepStack].pop(); + const parentStep = params.forceNoParent ? undefined : [...stepStack].pop(); const step: TestStep = { title: params.title, titlePath: () => { diff --git a/src/test/expect.ts b/src/test/expect.ts index 6740c7a656..7fa94025a8 100644 --- a/src/test/expect.ts +++ b/src/test/expect.ts @@ -75,7 +75,12 @@ function wrap(matcherName: string, matcher: any) { const INTERNAL_STACK_LENGTH = 3; const stackLines = new Error().stack!.split('\n').slice(INTERNAL_STACK_LENGTH + 1); - const step = testInfo._addStep('expect', `expect${this.isNot ? '.not' : ''}.${matcherName}`, true); + const step = testInfo._addStep({ + category: 'expect', + title: `expect${this.isNot ? '.not' : ''}.${matcherName}`, + canHaveChildren: true, + forceNoParent: false + }); const reportStepEnd = (result: any) => { const success = result.pass !== this.isNot; diff --git a/src/test/index.ts b/src/test/index.ts index 0a5da6c313..6294a7ca01 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -212,8 +212,12 @@ export const test = _baseTest.extend({ } (context as any)._csi = { onApiCallBegin: (apiCall: string) => { - const testInfoImpl = testInfo as any; - const step = testInfoImpl._addStep('pw:api', apiCall, false); + const step = (testInfo as any)._addStep({ + category: 'pw:api', + title: apiCall, + canHaveChildren: false, + forceNoParent: false, + }); return { userObject: step }; }, onApiCallEnd: (data: { userObject: any }, error?: Error) => { diff --git a/src/test/ipc.ts b/src/test/ipc.ts index e42a295deb..da64a0a35c 100644 --- a/src/test/ipc.ts +++ b/src/test/ipc.ts @@ -52,6 +52,7 @@ export type StepBeginPayload = { title: string; category: string; canHaveChildren: boolean; + forceNoParent: boolean; wallTime: number; // milliseconds since unix epoch }; diff --git a/src/test/testType.ts b/src/test/testType.ts index 98a6220ff8..1fcc1164c7 100644 --- a/src/test/testType.ts +++ b/src/test/testType.ts @@ -186,7 +186,12 @@ export class TestTypeImpl { const testInfo = currentTestInfo(); if (!testInfo) throw errorWithLocation(location, `test.step() can only be called from a test`); - const step = testInfo._addStep('test.step', title, true); + const step = testInfo._addStep({ + category: 'test.step', + title, + canHaveChildren: true, + forceNoParent: false + }); try { await body(); step.complete(); diff --git a/src/test/types.ts b/src/test/types.ts index 113e5674e0..480a0c957a 100644 --- a/src/test/types.ts +++ b/src/test/types.ts @@ -27,11 +27,13 @@ export type Annotations = { type: string, description?: string }[]; export interface TestStepInternal { complete(error?: Error | TestError): void; + title: string; category: string; canHaveChildren: boolean; + forceNoParent: boolean; } export interface TestInfoImpl extends TestInfo { _testFinished: Promise; - _addStep: (category: string, title: string, canHaveChildren: boolean) => TestStepInternal; + _addStep: (data: Omit) => TestStepInternal; } diff --git a/src/test/workerRunner.ts b/src/test/workerRunner.ts index c8f5a35ca4..fe300a8101 100644 --- a/src/test/workerRunner.ts +++ b/src/test/workerRunner.ts @@ -267,12 +267,11 @@ export class WorkerRunner extends EventEmitter { deadlineRunner.updateDeadline(deadline()); }, _testFinished: new Promise(f => testFinishedCallback = f), - _addStep: (category: string, title: string, canHaveChildren: boolean) => { - const stepId = `${category}@${title}@${++lastStepId}`; + _addStep: data => { + const stepId = `${data.category}@${data.title}@${++lastStepId}`; let callbackHandled = false; const step: TestStepInternal = { - category, - canHaveChildren, + ...data, complete: (error?: Error | TestError) => { if (callbackHandled) return; @@ -292,9 +291,7 @@ export class WorkerRunner extends EventEmitter { const payload: StepBeginPayload = { testId, stepId, - category, - canHaveChildren, - title, + ...data, wallTime: Date.now(), }; if (reportEvents) @@ -423,7 +420,12 @@ export class WorkerRunner extends EventEmitter { } private async _runTestWithBeforeHooks(test: TestCase, testInfo: TestInfoImpl) { - const step = testInfo._addStep('hook', 'Before Hooks', true); + const step = testInfo._addStep({ + category: 'hook', + title: 'Before Hooks', + canHaveChildren: true, + forceNoParent: true + }); if (test._type === 'test') await this._runBeforeHooks(test, testInfo); @@ -457,7 +459,12 @@ export class WorkerRunner extends EventEmitter { let step: TestStepInternal | undefined; let teardownError: TestError | undefined; try { - step = testInfo._addStep('hook', 'After Hooks', true); + step = testInfo._addStep({ + category: 'hook', + title: 'After Hooks', + canHaveChildren: true, + forceNoParent: true + }); if (test._type === 'test') await this._runHooks(test.parent!, 'afterEach', testInfo); } catch (error) { diff --git a/tests/config/browserTest.ts b/tests/config/browserTest.ts index 2cf8701350..d0972c313f 100644 --- a/tests/config/browserTest.ts +++ b/tests/config/browserTest.ts @@ -143,7 +143,12 @@ export const playwrightFixtures: Fixtures { const testInfoImpl = testInfo as any; - const step = testInfoImpl._addStep('pw:api', apiCall, false); + const step = testInfoImpl._addStep({ + category: 'pw:api', + title: apiCall, + canHaveChildren: false, + forceNoParent: false + }); return { userObject: step }; }, onApiCallEnd: (data: { userObject: any }, error?: Error) => { diff --git a/tests/playwright-test/test-step.spec.ts b/tests/playwright-test/test-step.spec.ts index 156c9b13ef..eab8e1a0d5 100644 --- a/tests/playwright-test/test-step.spec.ts +++ b/tests/playwright-test/test-step.spec.ts @@ -16,44 +16,44 @@ import { test, expect } from './playwright-test-fixtures'; -test('should report api step hierarchy', async ({ runInlineTest }) => { - const expectReporterJS = ` - class Reporter { - onBegin(config: FullConfig, suite: Suite) { - this.suite = suite; - } +const stepHierarchyReporter = ` +class Reporter { + onBegin(config: FullConfig, suite: Suite) { + this.suite = suite; + } - distillStep(step) { - return { - ...step, - startTime: undefined, - duration: undefined, - parent: undefined, - data: undefined, - steps: step.steps.length ? step.steps.map(s => this.distillStep(s)) : undefined, - }; - } + distillStep(step) { + return { + ...step, + startTime: undefined, + duration: undefined, + parent: undefined, + data: undefined, + steps: step.steps.length ? step.steps.map(s => this.distillStep(s)) : undefined, + }; + } - async onEnd() { - const processSuite = (suite: Suite) => { - for (const child of suite.suites) - processSuite(child); - for (const test of suite.tests) { - for (const result of test.results) { - for (const step of result.steps) { - console.log('%% ' + JSON.stringify(this.distillStep(step))); - } - } + async onEnd() { + const processSuite = (suite: Suite) => { + for (const child of suite.suites) + processSuite(child); + for (const test of suite.tests) { + for (const result of test.results) { + for (const step of result.steps) { + console.log('%% ' + JSON.stringify(this.distillStep(step))); } - }; - processSuite(this.suite); + } } - } - module.exports = Reporter; - `; + }; + processSuite(this.suite); + } +} +module.exports = Reporter; +`; +test('should report api step hierarchy', async ({ runInlineTest }) => { const result = await runInlineTest({ - 'reporter.ts': expectReporterJS, + 'reporter.ts': stepHierarchyReporter, 'playwright.config.ts': ` module.exports = { reporter: './reporter', @@ -128,6 +128,54 @@ test('should report api step hierarchy', async ({ runInlineTest }) => { ]); }); +test('should not report nested after hooks', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': stepHierarchyReporter, + 'playwright.config.ts': ` + module.exports = { + reporter: './reporter', + }; + `, + 'a.test.ts': ` + const { test } = pwt; + test('timeout', async ({ page }) => { + await test.step('my step', async () => { + await new Promise(() => {}); + }); + }); + ` + }, { reporter: '', workers: 1, timeout: 2000 }); + + expect(result.exitCode).toBe(1); + const objects = result.output.split('\n').filter(line => line.startsWith('%% ')).map(line => line.substring(3).trim()).filter(Boolean).map(line => JSON.parse(line)); + expect(objects).toEqual([ + { + category: 'hook', + title: 'Before Hooks', + steps: [ + { + category: 'pw:api', + title: 'browserContext.newPage', + }, + ], + }, + { + category: 'test.step', + title: 'my step', + }, + { + category: 'hook', + title: 'After Hooks', + steps: [ + { + category: 'pw:api', + title: 'browserContext.close', + }, + ], + }, + ]); +}); + test('should report test.step from fixtures', async ({ runInlineTest }) => { const expectReporterJS = ` class Reporter {