fix duplicate steps error message, [#74]

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## dev
 * fix empty step locations for esm
+* fix duplicate steps error message, [#74](https://github.com/vitalets/playwright-bdd/issues/74)
 
 ## 5.5.0
 * add support for hooks, [#15](https://github.com/vitalets/playwright-bdd/issues/15)

src/cucumber/loadSteps.ts

@@ -35,11 +35,12 @@ export function findStepDefinition(
   if (matchedSteps.length > 1)
     exit(
       [
-        `Several step definitions found for text: ${stepText} (${file})`,
-        ...matchedSteps.map((s) => `- ${s.pattern}`),
+        `Multiple step definitions matched for text: "${stepText}" (${file})`,
+        // todo: print location of every step definition (as in cucumber)
+        ...matchedSteps.map((s) => `  ${s.pattern}`),
       ].join('\n'),
     );
-  // todo: check stepDefinition.keyword with PickleStepType
+
   return matchedSteps[0];
 }
 

src/utils/exit.ts

@@ -1,12 +1,13 @@
 /**
  * Exit utils.
  *
- * When calling process.exit() in worker thread used for file generation,
+ * When calling process.exit() in worker thread used for test-file generation,
  * logs are not flushed (https://github.com/vitalets/playwright-bdd/issues/59).
  * That's why instead of process.exit we throw ExitError
  * that just sets process.exitCode = 1 and allow program to exit normally.
+ * This esnured by wrapping code with withExitHandler().
  *
- * On the other hand, when running in main thread, especially inside Playwright,
+ * On the other hand, when running in the main thread, especially inside Playwright,
  * thrown error is captured by Playwright and show with additional messages (e.g. no tests found).
  * That's why in main thread we to call process.exit() to show only needed error.
  *
@@ -16,7 +17,6 @@
  * - https://github.com/cucumber/cucumber-js/pull/123
  */
 
-import { logger } from './logger';
 import { isMainThread } from 'node:worker_threads';
 
 class ExitError extends Error {
@@ -30,6 +30,8 @@ export async function withExitHandler(fn: () => unknown) {
     return await fn();
   } catch (e) {
     if (e instanceof ExitError) {
+      // eslint-disable-next-line no-console, max-depth
+      if (e.message) console.error(e.message);
       process.exitCode = 1;
     } else {
       throw e;
@@ -40,7 +42,10 @@ export async function withExitHandler(fn: () => unknown) {
 export function exit(...messages: string[]) {
   messages = messages.filter(Boolean);
   if (isMainThread) {
-    if (messages.length) logger.error('Error:', ...messages);
+    // use console.error() here instead of logger.error() to have less stack
+    // for flushing messages to stderr.
+    // eslint-disable-next-line no-console, max-depth
+    if (messages.length) console.error('Error:', ...messages);
     process.exit(1);
   } else {
     throw new ExitError(messages.join(' '));

test/duplicate-steps/features/one.feature

@@ -0,0 +1,5 @@
+Feature: unique steps
+
+    Scenario: scenario 1
+        Given unique step
+

test/duplicate-steps/features/two.feature

@@ -0,0 +1,5 @@
+Feature: duplicate steps
+
+    Scenario: scenario 1
+        Given duplicate step
+

test/duplicate-steps/playwright.config.ts

@@ -0,0 +1,26 @@
+import { defineConfig } from '@playwright/test';
+import { defineBddConfig } from '../../dist';
+
+export default defineConfig({
+  projects: [
+    {
+      name: 'no duplicate steps',
+      testDir: defineBddConfig({
+        outputDir: `.features-gen/one`,
+        paths: ['features/one.feature'],
+        importTestFrom: 'steps/fixtures.ts',
+      }),
+    },
+    {
+      // important to have duplicate steps in the second project
+      // that runs in a worker process
+      name: 'duplicates',
+      testDir: defineBddConfig({
+        outputDir: `.features-gen/two`,
+        paths: ['features/two.feature'],
+        importTestFrom: 'steps/fixtures.ts',
+      }),
+    },
+  ],
+  forbidOnly: Boolean(process.env.FORBID_ONLY),
+});

test/duplicate-steps/steps/TodoPage.ts

@@ -0,0 +1,15 @@
+import { Fixture, Given } from '../../../dist/decorators';
+import { test } from './fixtures';
+
+export
+@Fixture<typeof test>('todoPage')
+class TodoPage {
+  @Given('unique step')
+  async step1() {}
+
+  @Given('duplicate step')
+  async step2() {}
+
+  @Given('duplicate step')
+  async step3() {}
+}

test/duplicate-steps/steps/fixtures.ts

@@ -0,0 +1,10 @@
+import { test as base } from '../../../dist';
+import { TodoPage } from './TodoPage';
+
+type Fixtures = {
+  todoPage: TodoPage;
+};
+
+export const test = base.extend<Fixtures>({
+  todoPage: ({}, use) => use(new TodoPage()),
+});

test/duplicate-steps/test.mjs

@@ -0,0 +1,21 @@
+import { test, TestDir, execPlaywrightTestWithError, DEFAULT_CMD } from '../helpers.mjs';
+
+const testDir = new TestDir(import.meta);
+
+test(`${testDir.name} (main thread)`, () => {
+  execPlaywrightTestWithError(
+    testDir.name,
+    DUPLICATE_STEPS_ERROR,
+    `${DEFAULT_CMD} -- project duplicates`,
+  );
+});
+
+test(`${testDir.name} (worker)`, () => {
+  execPlaywrightTestWithError(testDir.name, DUPLICATE_STEPS_ERROR);
+});
+
+const DUPLICATE_STEPS_ERROR = [
+  'Multiple step definitions matched for text: "duplicate step" (features/two.feature)',
+  '  duplicate step',
+  '  duplicate step',
+].join('\n');