reporter: timeout in step and in after fixture

src/reporter/cucumber/messagesBuilder/AttachmentMapper.ts

@@ -56,7 +56,7 @@
  */
 import * as pw from '@playwright/test/reporter';
 import { AutofillMap } from '../../../utils/AutofillMap';
-import { collectStepsWithCategory, getHooksRootPwStep } from './pwUtils';
+import { collectStepsWithCategory, getHooksRootPwStep } from './pwStepUtils';
 import { PwAttachment } from '../../../playwright/types';
 import { isBddDataAttachment } from '../../../run/bddDataAttachment';
 

src/reporter/cucumber/messagesBuilder/TestCaseRun.ts

@@ -9,7 +9,7 @@ import { TestCase } from './TestCase';
 import { AutofillMap } from '../../../utils/AutofillMap';
 import { TestStepRun, TestStepRunEnvelope } from './TestStepRun';
 import { toCucumberTimestamp } from './timing';
-import { collectStepsWithCategory, getHooksRootPwStep, isTimeoutPwStep } from './pwUtils';
+import { collectStepsWithCategory, isEmptyDuration } from './pwStepUtils';
 import {
   BddDataAttachment,
   BddDataStep,
@@ -28,7 +28,8 @@ export type TestCaseRunEnvelope = TestStepRunEnvelope &
 
 export type ExecutedStepInfo = {
   bddDataStep: BddDataStep;
-  pwStep: pw.TestStep;
+  // pwStep can be missing even for executted steps when there is test timeout
+  pwStep?: pw.TestStep;
 };
 
 export class TestCaseRun {
@@ -37,6 +38,8 @@ export class TestCaseRun {
   testCase?: TestCase;
   attachmentMapper: AttachmentMapper;
   projectInfo: ProjectInfo;
+  // collect steps with error and show only these errors in report.
+  // it allows to not show the same error on parent steps
   errorSteps = new Set<pw.TestStep>();
   timeoutedStep?: pw.TestStep;
   private executedBeforeHooks: TestCaseRunHooks;
@@ -49,11 +52,6 @@ export class TestCaseRun {
     public result: pw.TestResult,
     public hooks: AutofillMap<string, Hook>,
   ) {
-    // console.error(11, this.result);
-    // console.error(
-    //   22,
-    //   this.result.steps[4].steps.find((s) => s.title.includes('timeoutedAfterFixture')),
-    // );
     this.id = this.generateTestRunId();
     this.bddData = this.getBddData();
     this.projectInfo = getProjectInfo(this.test);
@@ -91,11 +89,11 @@ export class TestCaseRun {
   }
 
   private fillExecutedSteps() {
-    const possiblePwSteps = this.getPossiblePlaywrightSteps();
+    const possiblePwSteps = this.getPossiblePlaywrightBddSteps();
     return this.bddData.steps.map((bddDataStep) => {
       const pwStep = this.findPlaywrightStep(possiblePwSteps, bddDataStep);
-      if (pwStep.error) this.errorSteps.add(pwStep);
-      if (!this.timeoutedStep && isTimeoutPwStep(pwStep)) this.timeoutedStep = pwStep;
+      if (pwStep?.error) this.errorSteps.add(pwStep);
+      this.handleTimeoutedStep(pwStep);
       return { bddDataStep, pwStep };
     });
   }
@@ -104,6 +102,14 @@ export class TestCaseRun {
     return new TestCaseRunHooks(this, hookType).fill(this.executedSteps);
   }
 
+  // eslint-disable-next-line complexity
+  private handleTimeoutedStep(pwStep?: pw.TestStep) {
+    if (!pwStep || !this.isTimeouted() || this.timeoutedStep) return;
+    if (isEmptyDuration(pwStep) || pwStep.error) {
+      this.timeoutedStep = pwStep;
+    }
+  }
+
   buildMessages() {
     return [
       this.buildTestCaseStarted(),
@@ -152,17 +158,16 @@ export class TestCaseRun {
   }
 
   private findPlaywrightStep(possiblePwSteps: pw.TestStep[], bddDataStep: BddDataStep) {
-    const pwStep = possiblePwSteps.find((pwStep) => {
+    return possiblePwSteps.find((pwStep) => {
       return pwStep.location && stringifyLocation(pwStep.location) === bddDataStep.pwStepLocation;
     });
-    if (!pwStep) throw new Error('Playwright step not found for bdd step');
-    return pwStep;
   }
 
-  private getPossiblePlaywrightSteps() {
-    const beforeHooksRoot = getHooksRootPwStep(this.result, 'before');
-    const bgSteps = collectStepsWithCategory(beforeHooksRoot, 'test.step');
-    const topLevelSteps = this.result.steps.filter((step) => step.category === 'test.step');
-    return [...bgSteps, ...topLevelSteps];
+  private getPossiblePlaywrightBddSteps() {
+    // Before we collected only top-level steps and steps from before hooks (as they are background)
+    // But it's more reliable to just collect all test.step items b/c some Playwright versions
+    // move steps to fixtures (see: https://github.com/microsoft/playwright/issues/30075)
+    // Collecting all test.step items should be ok, as later we anyway map them by location.
+    return collectStepsWithCategory(this.result, 'test.step');
   }
 }

src/reporter/cucumber/messagesBuilder/TestCaseRunHooks.ts

@@ -7,8 +7,8 @@ import {
   getHooksRootPwStep,
   collectStepsDfs,
   findDeepestStepWithError,
-  findDeepestStepWithTimeout,
-} from './pwUtils';
+  findDeepestStepWithEmptyDuration,
+} from './pwStepUtils';
 import { ExecutedStepInfo, TestCaseRun } from './TestCaseRun';
 import { TestStepRun, TestStepRunEnvelope } from './TestStepRun';
 
@@ -35,7 +35,7 @@ export class TestCaseRunHooks {
     this.addStepWithTimeout();
     this.addStepWithError();
     this.addStepsWithAttachment();
-    this.excludeBackgroundSteps(mainSteps);
+    this.excludeMainSteps(mainSteps);
     this.setExecutedHooks();
     return this;
   }
@@ -103,7 +103,9 @@ export class TestCaseRunHooks {
     if (this.testCaseRun.timeoutedStep) return;
     const timeoutedStep =
       this.hookType === 'before'
-        ? findDeepestStepWithTimeout(this.rootPwStep)
+        ? // Timeouted steps have duration = -1 in PW <= 1.39 and no error field.
+          // In PW > 1.39 timeouted tests have '.error' populated
+          findDeepestStepWithEmptyDuration(this.rootPwStep)
         : // Timeouted after hooks don't have duration = -1,
           // so there is no way to find which exactly fixture timed out.
           // We mark root 'After Hooks' step as timeouted.
@@ -114,13 +116,16 @@ export class TestCaseRunHooks {
     }
   }
 
-  private excludeBackgroundSteps(mainSteps: ExecutedStepInfo[]) {
-    // exclude background steps, b/c they are in pickle, not in hooks.
+  private excludeMainSteps(mainSteps: ExecutedStepInfo[]) {
+    // - exclude background steps, b/c they are in pickle and should not in hooks.
+    // - exclude other test.step items that are bdd steps and should not be in hooks.
     // Important to run this fn after this.fillExecutedSteps()
     // as we assume steps are already populated
-    if (this.hookType === 'before') {
-      mainSteps.forEach((stepInfo) => this.hookSteps.delete(stepInfo.pwStep));
-    }
+    mainSteps.forEach((stepInfo) => {
+      if (stepInfo.pwStep) {
+        this.hookSteps.delete(stepInfo.pwStep);
+      }
+    });
   }
 
   private setExecutedHooks() {

src/reporter/cucumber/messagesBuilder/pwUtils.ts → src/reporter/cucumber/messagesBuilder/pwStepUtils.ts

@@ -29,10 +29,10 @@ export function findDeepestStepWithError(root?: pw.TestStep) {
   });
 }
 
-export function findDeepestStepWithTimeout(root?: pw.TestStep) {
+export function findDeepestStepWithEmptyDuration(root?: pw.TestStep) {
   if (!root) return;
   return findDeepestStepWith(root, (pwStep) => {
-    return isTimeoutPwStep(pwStep) && MEANINGFUL_STEP_CATEGORIES.includes(pwStep.category);
+    return isEmptyDuration(pwStep) && MEANINGFUL_STEP_CATEGORIES.includes(pwStep.category);
   });
 }
 
@@ -66,6 +66,6 @@ export function collectStepsDfs(parent: pw.TestResult | pw.TestStep | undefined)
   );
 }
 
-export function isTimeoutPwStep(pwStep: pw.TestStep) {
+export function isEmptyDuration(pwStep: pw.TestStep) {
   return pwStep.duration === -1;
 }

src/run/StepInvoker.ts

@@ -28,22 +28,26 @@ export class StepInvoker {
    * See: https://github.com/cucumber/cucumber-js/blob/main/src/runtime/test_case_runner.ts#L299
    */
   async invoke(
-    text: string,
+    stepText: string,
     argument?: PickleStepArgument | null,
     stepFixtures?: Fixtures<TestTypeCommon>,
   ) {
     this.world.$internal.currentStepFixtures = stepFixtures || {};
-    const stepDefinition = this.getStepDefinition(text);
+    const stepDefinition = this.getStepDefinition(stepText);
 
     // Get location of step call in generated test file.
     // This call must be exactly here to have correct call stack (before async calls)
     const location = getLocationInFile(this.world.testInfo.file);
 
-    const stepTitle = this.getStepTitle(text);
+    const stepTitle = this.getStepTitle(stepText);
     const code = getStepCode(stepDefinition);
-    const parameters = await this.getStepParameters(stepDefinition, text, argument || undefined);
+    const parameters = await this.getStepParameters(
+      stepDefinition,
+      stepText,
+      argument || undefined,
+    );
 
-    this.world.$internal.bddData.registerStep(stepDefinition, text, location);
+    this.world.$internal.bddData.registerStep(stepDefinition, stepText, location);
 
     await runStepWithCustomLocation(this.world.test, stepTitle, location, () =>
       code.apply(this.world, parameters),

test/reporter-cucumber-html/check-report/failed.test.ts

@@ -98,22 +98,17 @@ test('Scenario: Failing by failingAfterFixtureNoStep', async ({ page }) => {
   // there is no automatic screenshot here in pw 1.35 - 1.41
   // see: https://github.com/microsoft/playwright/issues/29325
   const hasScreenshot = pwVersion < '1.35.0' || pwVersion >= '1.42.0';
-  await expect(scenario.getSteps()).toContainText(
-    [
-      'my attachment|before use',
-      'Givenstep that uses failingAfterFixtureNoStep',
-      'WhenAction 3',
-      hasScreenshot ? 'screenshot' : '',
-      `Hook "fixture: failingAfterFixtureNoStep" failed: ${normalize('features/fixtures.ts')}:`,
-    ].filter(Boolean),
-  );
-  await expect(scenario.getAttachments()).toHaveText(
-    [
-      'my attachment|before use', // prettier-ignore
-      hasScreenshot ? 'screenshot' : '',
-      'my attachment|after use',
-    ].filter(Boolean),
-  );
+  await expect(scenario.getSteps()).toContainText([
+    'my attachment|before use',
+    'Givenstep that uses failingAfterFixtureNoStep',
+    'WhenAction 3',
+    `Hook "fixture: failingAfterFixtureNoStep" failed: ${normalize('features/fixtures.ts')}:`,
+  ]);
+  if (hasScreenshot) await expect(scenario.getSteps()).toContainText(['screenshot']);
+  await expect(scenario.getAttachments()).toContainText([
+    'my attachment|before use', // prettier-ignore
+    'my attachment|after use',
+  ]);
   await expect(scenario.getSteps('passed')).toHaveCount(2);
   await expect(scenario.getSteps('failed')).toHaveCount(1);
   await expect(scenario.getSteps('skipped')).toHaveCount(0);
@@ -125,24 +120,19 @@ test('Scenario: Failing by failingAfterFixtureWithStep', async ({ page }) => {
   // there is no automatic screenshot here in pw 1.35 - 1.41
   // see: https://github.com/microsoft/playwright/issues/29325
   const hasScreenshot = pwVersion < '1.35.0' || pwVersion >= '1.42.0';
-  await expect(scenario.getSteps()).toContainText(
-    [
-      'my attachment|outside step (before use)',
-      'Givenstep that uses failingAfterFixtureWithStep',
-      'WhenAction 4',
-      `Hook "step in failingAfterFixtureWithStep" failed: ${normalize('features/fixtures.ts')}:`,
-      hasScreenshot ? 'screenshot' : '',
-      'my attachment|outside step (after use)',
-    ].filter(Boolean),
-  );
-  await expect(scenario.getAttachments()).toHaveText(
-    [
-      'my attachment|outside step (before use)',
-      'my attachment|in step',
-      hasScreenshot ? 'screenshot' : '',
-      'my attachment|outside step (after use)',
-    ].filter(Boolean),
-  );
+  await expect(scenario.getSteps()).toContainText([
+    'my attachment|outside step (before use)',
+    'Givenstep that uses failingAfterFixtureWithStep',
+    'WhenAction 4',
+    `Hook "step in failingAfterFixtureWithStep" failed: ${normalize('features/fixtures.ts')}:`,
+    'my attachment|outside step (after use)',
+  ]);
+  if (hasScreenshot) await expect(scenario.getSteps()).toContainText(['screenshot']);
+  await expect(scenario.getAttachments()).toContainText([
+    'my attachment|outside step (before use)',
+    'my attachment|in step',
+    'my attachment|outside step (after use)',
+  ]);
   await expect(scenario.getSteps('passed')).toHaveCount(2);
   await expect(scenario.getSteps('failed')).toHaveCount(1);
   await expect(scenario.getSteps('skipped')).toHaveCount(0);
@@ -165,54 +155,3 @@ test('Scenario: failing match snapshot', async ({ page }) => {
   await expect(scenario.getSteps('skipped')).toHaveCount(0);
   await expect(scenario.getError()).toContainText('Snapshot comparison failed');
 });
-
-test('Scenario: timeout in before fixture', async ({ page }) => {
-  const scenario = getScenario(page, 'timeout in before fixture');
-  await expect(scenario.getSteps()).toContainText([
-    // here can be different error messages
-    /Hook "fixture: (.+)" failed/,
-    'screenshot',
-    'GivenAction 0',
-    'Givenstep that uses timeouted before fixture',
-    'WhenAction 1',
-  ]);
-  await expect(scenario.getSteps('failed')).toHaveCount(1);
-  await expect(scenario.getSteps('skipped')).toHaveCount(3);
-  await expect(scenario.getError()).toContainText(
-    // here can be two different error messages
-    // eslint-disable-next-line max-len
-    /(Test timeout of \d+ms exceeded while setting up "timeoutedBeforeFixture")|(browser has been closed)|(Browser closed)/,
-  );
-});
-
-test('Scenario: timeout in step', async ({ page }) => {
-  const scenario = getScenario(page, 'timeout in step');
-  await expect(scenario.getSteps()).toContainText([
-    'GivenAction 0',
-    'Giventimeouted step',
-    'WhenAction 1',
-    'screenshot',
-  ]);
-  await expect(scenario.getSteps('passed')).toHaveCount(1);
-  await expect(scenario.getSteps('failed')).toHaveCount(1);
-  await expect(scenario.getSteps('skipped')).toHaveCount(1);
-  await expect(scenario.getError()).toContainText(/Test timeout of \d+ms exceeded/);
-  await expect(scenario.getError()).toContainText('Pending operations');
-  await expect(scenario.getError()).toContainText('page.waitForTimeout');
-});
-
-test('Scenario: timeout in after fixture', async ({ page }) => {
-  const scenario = getScenario(page, 'timeout in after fixture');
-  await expect(scenario.getSteps()).toContainText([
-    'GivenAction 0',
-    'Givenstep that uses timeouted after fixture',
-    'WhenAction 1',
-    'Hook "After Hooks" failed: Unknown location',
-  ]);
-  await expect(scenario.getSteps('passed')).toHaveCount(3);
-  await expect(scenario.getSteps('failed')).toHaveCount(1);
-  await expect(scenario.getError()).toContainText('Test finished within timeout');
-  await expect(scenario.getError()).toContainText(
-    'but tearing down "timeoutedAfterFixture" ran out of time',
-  );
-});

test/reporter-cucumber-html/check-report/header.test.ts

@@ -7,9 +7,9 @@ test.beforeEach(async ({ page }) => {
 
 test('header info', async ({ page }) => {
   // after changing this counts you should also update test/reporter-cucumber-junit
-  await expect(page.getByText('12 failed')).toBeVisible();
+  await expect(page.getByText('13 failed')).toBeVisible();
   await expect(page.getByText('7 passed')).toBeVisible();
-  await expect(page.getByText('19 executed')).toBeVisible();
+  await expect(page.getByText('20 executed')).toBeVisible();
 
   await expect(page.locator('dl').getByText('node.js')).toBeVisible();
   await expect(page.locator('dl').getByText('playwright-bdd')).toBeVisible();

test/reporter-cucumber-html/check-report/helpers.ts

@@ -57,7 +57,7 @@ export class Scenario {
     public feature: Feature,
     public title: string,
   ) {
-    this.header = this.page.getByRole('heading', { name: title });
+    this.header = this.page.getByRole('heading', { name: title, exact: true });
     this.root = feature.root.locator('section').filter({ has: this.header });
   }