diff --git a/src/reporter/cucumber/messagesBuilder/TestCaseRun.ts b/src/reporter/cucumber/messagesBuilder/TestCaseRun.ts index e98d5421..49cc2623 100644 --- a/src/reporter/cucumber/messagesBuilder/TestCaseRun.ts +++ b/src/reporter/cucumber/messagesBuilder/TestCaseRun.ts @@ -8,7 +8,12 @@ import { TestCase } from './TestCase'; import { AutofillMap } from '../../../utils/AutofillMap.js'; import { TestStepRun, TestStepRunEnvelope } from './TestStepRun'; import { toCucumberTimestamp } from './timing'; -import { areTestErrorsEqual, collectStepsWithCategory, isUnknownDuration } from './pwStepUtils'; +import { + areTestErrorsEqual, + collectStepsWithCategory, + isTopLevelStep, + isUnknownDuration, +} from './pwStepUtils'; import { AttachmentMapper } from './AttachmentMapper'; import { TestCaseRunHooks } from './TestCaseRunHooks'; import { ProjectInfo, getProjectInfo } from './Projects'; @@ -99,8 +104,12 @@ export class TestCaseRun { const possiblePwSteps = this.getPossiblePwSteps(); return this.bddTestData.steps.map((bddStep) => { const pwStep = this.findPlaywrightStep(possiblePwSteps, bddStep); - if (pwStep?.error) this.registerErrorStep(pwStep, pwStep.error); - this.registerTimeoutedStep(pwStep); + if (pwStep?.error) { + this.registerErrorStep(pwStep, pwStep.error); + } + if (this.isTimeouted() && pwStep && isUnknownDuration(pwStep)) { + this.registerTimeoutedStep(pwStep); + } return { bddStep, pwStep }; }); } @@ -113,10 +122,24 @@ export class TestCaseRun { this.errorSteps.set(pwStep, error); } - // todo: rename - private registerTimeoutedStep(pwStep?: pw.TestStep) { - if (this.isTimeouted() && !this.timeoutedStep && pwStep && isUnknownDuration(pwStep)) { - this.timeoutedStep = pwStep; + hasRegisteredError(error: pw.TestError) { + for (const registeredError of this.errorSteps.values()) { + if (registeredError === error) return true; + } + } + + registerTimeoutedStep(pwStep: pw.TestStep) { + if (this.timeoutedStep) return; + + this.timeoutedStep = pwStep; + + // Handle case when timeouted step has duration -1 and no 'error' field, + // but it's parent contains actual error. + // - timeout in bg step + // - timeout in hooks + // We register timeouted step with error from parent. + if (!pwStep.error && pwStep.parent?.error && !isTopLevelStep(pwStep)) { + this.registerErrorStep(this.timeoutedStep, pwStep.parent.error); } } diff --git a/src/reporter/cucumber/messagesBuilder/TestCaseRunHooks.ts b/src/reporter/cucumber/messagesBuilder/TestCaseRunHooks.ts index c60ee945..97daecf6 100644 --- a/src/reporter/cucumber/messagesBuilder/TestCaseRunHooks.ts +++ b/src/reporter/cucumber/messagesBuilder/TestCaseRunHooks.ts @@ -115,13 +115,8 @@ export class TestCaseRunHooks { const timeoutedStep = findDeepestStepWithUnknownDuration(this.rootPwStep); if (!timeoutedStep) return; - this.timeoutedStep = timeoutedStep; this.hookSteps.add(timeoutedStep); - this.testCaseRun.timeoutedStep = timeoutedStep; - - if (timeoutedStep.error) { - this.registerErrorStep(timeoutedStep, timeoutedStep.error); - } + this.testCaseRun.registerTimeoutedStep(timeoutedStep); } private addStepWithError() { @@ -133,20 +128,13 @@ export class TestCaseRunHooks { const stepWithError = findDeepestStepWithError(this.rootPwStep); if (!stepWithError) return; - // Handling timeout in Before hook: - // - timeout step has duration -1 and no 'error' field - // - timeout step parent has 'error' field with timeout error - // We move error from parent to timeout step and don't save parent as error step. - if ( - this.timeoutedStep && - !this.timeoutedStep.error && - this.timeoutedStep.parent === stepWithError - ) { - this.registerErrorStep(this.timeoutedStep, stepWithError.error); - return; + // If step is already added to errorSteps, don't register it as hookStep. + // This is mainly for timeout steps in hooks or bg: + // They have duration -1 and no 'error' field, but their parent has 'error' field. + // Here we find this parent again and avoid reporting the error twice. + if (!this.testCaseRun.hasRegisteredError(stepWithError.error)) { + this.registerErrorStep(stepWithError, stepWithError.error); } - - this.registerErrorStep(stepWithError, stepWithError.error); } // eslint-disable-next-line visual/complexity diff --git a/src/reporter/cucumber/messagesBuilder/pwStepUtils.ts b/src/reporter/cucumber/messagesBuilder/pwStepUtils.ts index af91b9bd..30a2a399 100644 --- a/src/reporter/cucumber/messagesBuilder/pwStepUtils.ts +++ b/src/reporter/cucumber/messagesBuilder/pwStepUtils.ts @@ -90,3 +90,7 @@ export function areTestErrorsEqual(e1: pw.TestError, e2: pw.TestError) { const keys: (keyof pw.TestError)[] = ['message', 'stack', 'value']; return keys.every((key) => e1[key] === e2[key]); } + +export function isTopLevelStep(pwStep: pw.TestStep) { + return !pwStep.parent; +} diff --git a/test/reporter-cucumber-html/features/failing-bg/specs/error.feature.spec.ts b/test/reporter-cucumber-html/features/failing-bg/specs/error.feature.spec.ts index 60e775c3..21f5c1e0 100644 --- a/test/reporter-cucumber-html/features/failing-bg/specs/error.feature.spec.ts +++ b/test/reporter-cucumber-html/features/failing-bg/specs/error.feature.spec.ts @@ -13,7 +13,7 @@ test('background is failing', async ({ feature }) => { test('scenario 1', async ({ scenario }) => { await expect(scenario.getSteps()).toContainText([ - 'GivenAction 1', // prettier-ignore + 'Action 1', // prettier-ignore 'screenshotDownload trace', ]); await expect(scenario.getSteps('skipped')).toHaveCount(1); @@ -22,7 +22,7 @@ test('scenario 1', async ({ scenario }) => { test('scenario 2', async ({ scenario }) => { await expect(scenario.getSteps()).toContainText([ - 'GivenAction 2', // prettier-ignore + 'Action 2', // prettier-ignore 'screenshotDownload trace', ]); await expect(scenario.getSteps('skipped')).toHaveCount(1); diff --git a/test/reporter-cucumber-html/features/failing-bg/specs/timeout.feature.spec.ts b/test/reporter-cucumber-html/features/failing-bg/specs/timeout.feature.spec.ts new file mode 100644 index 00000000..2fdd983f --- /dev/null +++ b/test/reporter-cucumber-html/features/failing-bg/specs/timeout.feature.spec.ts @@ -0,0 +1,32 @@ +import { expect } from '@playwright/test'; +import { test } from '../../../check-report/fixtures'; + +test('timeout in bg', async ({ feature }) => { + const background = feature.getBackground(); + await expect(background.getSteps()).toContainText([ + 'step with page', // prettier-ignore + 'timeouted step', + ]); + await expect(background.getSteps('failed')).toHaveCount(1); + await expect(background.getErrors()).toContainText([ + /Test timeout of \d+ms exceeded while running "beforeEach" hook/, + ]); +}); + +test('scenario 1', async ({ scenario }) => { + await expect(scenario.getSteps()).toContainText([ + 'Action 1', // prettier-ignore + 'screenshotDownload trace', + ]); + await expect(scenario.getSteps('skipped')).toHaveCount(1); + await expect(scenario.getErrors()).not.toBeVisible(); +}); + +test('scenario 2', async ({ scenario }) => { + await expect(scenario.getSteps()).toContainText([ + 'Action 2', // prettier-ignore + 'screenshotDownload trace', + ]); + await expect(scenario.getSteps('skipped')).toHaveCount(1); + await expect(scenario.getErrors()).not.toBeVisible(); +});