From 9a61a55ea11e32bae12405f112bb503791864648 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 3 May 2023 16:04:20 -0700 Subject: [PATCH] chore: nest api steps by time (#22801) Fixes https://github.com/microsoft/playwright/issues/22786 --- packages/playwright-test/src/index.ts | 1 + .../playwright-test/src/worker/testInfo.ts | 27 ++++++-- tests/playwright-test/reporter.spec.ts | 2 +- tests/playwright-test/test-step.spec.ts | 66 +++++++++++++++++-- 4 files changed, 83 insertions(+), 13 deletions(-) diff --git a/packages/playwright-test/src/index.ts b/packages/playwright-test/src/index.ts index 95891edcad..22caeb71ce 100644 --- a/packages/playwright-test/src/index.ts +++ b/packages/playwright-test/src/index.ts @@ -278,6 +278,7 @@ const playwrightFixtures: Fixtures = ({ category: 'pw:api', title: apiCall, wallTime, + laxParent: true, }); userData.userObject = step; }, diff --git a/packages/playwright-test/src/worker/testInfo.ts b/packages/playwright-test/src/worker/testInfo.ts index 770660c324..cfe538e582 100644 --- a/packages/playwright-test/src/worker/testInfo.ts +++ b/packages/playwright-test/src/worker/testInfo.ts @@ -34,6 +34,8 @@ interface TestStepInternal { wallTime: number; location?: Location; steps: TestStepInternal[]; + laxParent?: boolean; + endWallTime?: number; error?: TestInfoError; } @@ -210,16 +212,31 @@ export class TestInfoImpl implements TestInfo { _addStep(data: Omit): TestStepInternal { const stepId = `${data.category}@${data.title}@${++this._lastStepId}`; - const parentStep = zones.zoneData('stepZone', captureRawStack()); - let callbackHandled = false; + let parentStep = zones.zoneData('stepZone', captureRawStack()); + + // For out-of-stack calls, locate the enclosing step. + let isLaxParent = false; + if (!parentStep && data.laxParent) { + const visit = (step: TestStepInternal) => { + // Never nest into under another lax element, it could be a series + // of no-reply actions, ala page.continue(). + if (!step.endWallTime && step.category === data.category && !step.laxParent) + parentStep = step; + step.steps.forEach(visit); + }; + this._steps.forEach(visit); + isLaxParent = !!parentStep; + } + const step: TestStepInternal = { stepId, ...data, + laxParent: isLaxParent, steps: [], complete: result => { - if (callbackHandled) + if (step.endWallTime) return; - callbackHandled = true; + step.endWallTime = Date.now(); let error: TestInfoError | undefined; if (result.error instanceof Error) { // Step function threw an error. @@ -236,7 +253,7 @@ export class TestInfoImpl implements TestInfo { const payload: StepEndPayload = { testId: this._test.id, stepId, - wallTime: Date.now(), + wallTime: step.endWallTime, error, }; this._onStepEnd(payload); diff --git a/tests/playwright-test/reporter.spec.ts b/tests/playwright-test/reporter.spec.ts index e7260d3c0e..4a83dcc484 100644 --- a/tests/playwright-test/reporter.spec.ts +++ b/tests/playwright-test/reporter.spec.ts @@ -348,7 +348,7 @@ test('should report api steps', async ({ runInlineTest }) => { `end {\"title\":\"Before Hooks\",\"category\":\"hook\",\"steps\":[{\"title\":\"browserType.launch\",\"category\":\"pw:api\"},{\"title\":\"browser.newContext\",\"category\":\"pw:api\"},{\"title\":\"browserContext.newPage\",\"category\":\"pw:api\"},{\"title\":\"apiRequest.newContext\",\"category\":\"pw:api\"}]}`, `begin {\"title\":\"page.waitForNavigation\",\"category\":\"pw:api\"}`, `begin {\"title\":\"page.goto(data:text/html,)\",\"category\":\"pw:api\"}`, - `end {\"title\":\"page.waitForNavigation\",\"category\":\"pw:api\"}`, + `end {\"title\":\"page.waitForNavigation\",\"category\":\"pw:api\",\"steps\":[{\"title\":\"page.goto(data:text/html,)\",\"category\":\"pw:api\"}]}`, `end {\"title\":\"page.goto(data:text/html,)\",\"category\":\"pw:api\"}`, `begin {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, `end {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, diff --git a/tests/playwright-test/test-step.spec.ts b/tests/playwright-test/test-step.spec.ts index 7b2c73b4a5..4dc0caf635 100644 --- a/tests/playwright-test/test-step.spec.ts +++ b/tests/playwright-test/test-step.spec.ts @@ -89,7 +89,7 @@ test('should report api step hierarchy', async ({ runInlineTest }) => { }, { reporter: '', workers: 1 }); expect(result.exitCode).toBe(0); - const objects = result.output.split('\n').filter(line => line.startsWith('%% ')).map(line => line.substring(3).trim()).filter(Boolean).map(line => JSON.parse(line)); + const objects = result.outputLines.map(line => JSON.parse(line)); expect(objects).toEqual([ { category: 'hook', @@ -199,7 +199,7 @@ test('should report before hooks step error', async ({ runInlineTest }) => { }, { reporter: '', workers: 1 }); 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)); + const objects = result.outputLines.map(line => JSON.parse(line)); expect(objects).toEqual([ { category: 'hook', @@ -244,7 +244,7 @@ test('should not report nested after hooks', async ({ runInlineTest }) => { }, { 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)); + const objects = result.outputLines.map(line => JSON.parse(line)); expect(objects).toEqual([ { category: 'hook', @@ -363,7 +363,7 @@ test('should report expect step locations', async ({ runInlineTest }) => { }, { reporter: '', workers: 1 }); expect(result.exitCode).toBe(0); - const objects = result.output.split('\n').filter(line => line.startsWith('%% ')).map(line => line.substring(3).trim()).filter(Boolean).map(line => JSON.parse(line)); + const objects = result.outputLines.map(line => JSON.parse(line)); expect(objects).toEqual([ { category: 'hook', @@ -441,7 +441,7 @@ test('should report custom expect steps', async ({ runInlineTest }) => { }, { reporter: '', workers: 1 }); expect(result.exitCode).toBe(0); - const objects = result.output.split('\n').filter(line => line.startsWith('%% ')).map(line => line.substring(3).trim()).filter(Boolean).map(line => JSON.parse(line)); + const objects = result.outputLines.map(line => JSON.parse(line)); expect(objects).toEqual([ { category: 'hook', @@ -509,7 +509,7 @@ test('should mark step as failed when soft expect fails', async ({ runInlineTest }, { reporter: '', workers: 1 }); 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)); + const objects = result.outputLines.map(line => JSON.parse(line)); expect(objects).toEqual([ { title: 'Before Hooks', category: 'hook' }, { @@ -750,7 +750,7 @@ test('should not mark page.close as failed when page.click fails', async ({ runI }, { reporter: '' }); 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)); + const objects = result.outputLines.map(line => JSON.parse(line)); expect(objects).toEqual([ { category: 'hook', @@ -834,3 +834,55 @@ test('should not mark page.close as failed when page.click fails', async ({ runI }, ]); }); + +test('should nest page.continue insize page.goto steps', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': stepHierarchyReporter, + 'playwright.config.ts': `module.exports = { reporter: './reporter', };`, + 'a.test.ts': ` + import { test, expect } from '@playwright/test'; + test('pass', async ({ page }) => { + await page.route('**/*', route => route.fulfill('')); + await page.goto('http://localhost:1234'); + }); + ` + }, { reporter: '' }); + + expect(result.exitCode).toBe(0); + const objects = result.outputLines.map(line => JSON.parse(line)); + expect(objects).toEqual([ + { + title: 'Before Hooks', + category: 'hook', + steps: [ + { title: 'browserType.launch', category: 'pw:api' }, + { title: 'browser.newContext', category: 'pw:api' }, + { title: 'browserContext.newPage', category: 'pw:api' }, + ], + }, + { + title: 'page.route', + category: 'pw:api', + location: { file: 'a.test.ts', line: 'number', column: 'number' }, + }, + { + title: 'page.goto(http://localhost:1234)', + category: 'pw:api', + location: { file: 'a.test.ts', line: 'number', column: 'number' }, + steps: [ + { + title: 'route.fulfill', + category: 'pw:api', + location: { file: 'a.test.ts', line: 'number', column: 'number' }, + }, + ] + }, + { + title: 'After Hooks', + category: 'hook', + steps: [ + { title: 'browserContext.close', category: 'pw:api' }, + ], + }, + ]); +});