diff --git a/packages/playwright-test/src/worker/testInfo.ts b/packages/playwright-test/src/worker/testInfo.ts index 786055cfbb..770660c324 100644 --- a/packages/playwright-test/src/worker/testInfo.ts +++ b/packages/playwright-test/src/worker/testInfo.ts @@ -33,6 +33,8 @@ interface TestStepInternal { category: string; wallTime: number; location?: Location; + steps: TestStepInternal[]; + error?: TestInfoError; } export class TestInfoImpl implements TestInfo { @@ -49,6 +51,7 @@ export class TestInfoImpl implements TestInfo { _lastStepId = 0; readonly _projectInternal: FullProjectInternal; readonly _configInternal: FullConfigInternal; + readonly _steps: TestStepInternal[] = []; // ------------ TestInfo fields ------------ readonly testId: string; @@ -205,14 +208,14 @@ export class TestInfoImpl implements TestInfo { } } - _addStep(data: Omit): TestStepInternal { + _addStep(data: Omit): TestStepInternal { const stepId = `${data.category}@${data.title}@${++this._lastStepId}`; const parentStep = zones.zoneData('stepZone', captureRawStack()); let callbackHandled = false; - const firstErrorIndex = this.errors.length; const step: TestStepInternal = { stepId, ...data, + steps: [], complete: result => { if (callbackHandled) return; @@ -225,10 +228,11 @@ export class TestInfoImpl implements TestInfo { // Internal API step reported an error. error = result.error; } else { - // There was some other error (porbably soft expect) during step execution. - // Report step as failed to make it easier to spot. - error = this.errors[firstErrorIndex]; + // One of the child steps failed (probably soft expect). + // Report this step as failed to make it easier to spot. + error = step.steps.map(s => s.error).find(e => !!e); } + step.error = error; const payload: StepEndPayload = { testId: this._test.id, stepId, @@ -238,6 +242,8 @@ export class TestInfoImpl implements TestInfo { this._onStepEnd(payload); } }; + const parentStepList = parentStep ? parentStep.steps : this._steps; + parentStepList.push(step); const hasLocation = data.location && !data.location.file.includes('@playwright'); // Sanitize location that comes from user land, it might have extra properties. const location = data.location && hasLocation ? { file: data.location.file, line: data.location.line, column: data.location.column } : undefined; @@ -275,7 +281,7 @@ export class TestInfoImpl implements TestInfo { this.errors.push(error); } - async _runAsStep(stepInfo: Omit, cb: (step: TestStepInternal) => Promise): Promise { + async _runAsStep(stepInfo: Omit, cb: (step: TestStepInternal) => Promise): Promise { const step = this._addStep({ ...stepInfo, wallTime: Date.now() }); return await zones.run('stepZone', step, async () => { try { diff --git a/tests/playwright-test/test-step.spec.ts b/tests/playwright-test/test-step.spec.ts index ae3a6c3b71..7b2c73b4a5 100644 --- a/tests/playwright-test/test-step.spec.ts +++ b/tests/playwright-test/test-step.spec.ts @@ -720,3 +720,117 @@ test('should nest steps based on zones', async ({ runInlineTest }) => { } ]); }); + +test('should not mark page.close as failed when page.click fails', 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'; + let page: Page; + + test.beforeAll(async ({ browser }) => { + page = await browser.newPage(); + }); + + test.afterAll(async () => { + await page.close(); + }); + + test('fails', async () => { + test.setTimeout(2000); + await page.setContent('hello'); + await page.click('div'); + }); + ` + }, { 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)); + expect(objects).toEqual([ + { + category: 'hook', + title: 'Before Hooks', + steps: [ + { + category: 'hook', + title: 'beforeAll hook', + location: { + column: 'number', + file: 'a.test.ts', + line: 'number', + }, + steps: [ + { + category: 'pw:api', + title: 'browserType.launch', + }, + { + category: 'pw:api', + title: 'browser.newPage', + location: { + column: 'number', + file: 'a.test.ts', + line: 'number', + }, + }, + ], + }, + ], + }, + { + category: 'pw:api', + title: 'page.setContent', + location: { + column: 'number', + file: 'a.test.ts', + line: 'number', + }, + }, + { + category: 'pw:api', + title: 'page.click(div)', + location: { + column: 'number', + file: 'a.test.ts', + line: 'number', + }, + error: '', + }, + + { + category: 'hook', + title: 'After Hooks', + steps: [ + { + category: 'hook', + title: 'afterAll hook', + location: { + column: 'number', + file: 'a.test.ts', + line: 'number', + }, + steps: [ + { + category: 'pw:api', + title: 'page.close', + location: { + column: 'number', + file: 'a.test.ts', + line: 'number', + }, + }, + ], + }, + { + category: 'pw:api', + title: 'browser.close', + }, + ], + }, + ]); +});