test: timeout in bg

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);
     }
   }
 

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

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;
+}

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);

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();
+});