vitalets / playwright-bdd

BDD testing with Playwright runner
https://vitalets.github.io/playwright-bdd/
MIT License
318 stars 40 forks source link

Feature: Strict Step Registration #240

Open Aaron-Pool opened 3 weeks ago

Aaron-Pool commented 3 weeks ago

The problem When it comes to code dependencies, I love traceability, and the fact that a step registered by a Given returned from a different invocation createBdd(test) can be used in any generated test can often be a blocker to setting up traceable step sharing. Some people may prefer this ease of sharing, as it mimics the step sharing in cucumber, but personally, I prefer to group my steps by domain and require any test suite that wants to use those steps to explicitly call a registration type function.

For example, a step.ts file I write for a high level feature might look like this:

// entityEditor defines the base "test" instance that a "test" instance must extend
// to use these steps
export const registerInventoryManager = shareSteps(entityEditor, ({ Given }) => {
  Given(
    /I am on the "(.*)" page of "(.*)"/,
    async ({ page, tab, getIdByName, id }, route: string, name: string) => {
      const _id = getIdByName(name, entries);
      id.set(_id);
      await page.goto(`/admin/inventory/${id.get()}${route}`);
      return tab('current').waitFor();
    }
  );
  Given(/I am on the "(.*)" page of a new inventory entry/, async ({ page, tab, id }, route: string) => {
    id.set(unsavedIdentifier);
    await page.goto(`/admin/inventory/new${route}`);
    return tab('current').waitFor();
  });
});

const { Given, When, Then } = registerInventoryManager(createBdd(test));

Given('the entry {string} exists', async ({}, Name) => {
  const newEntry = entry.factory({ Name });
  entries.tap((store) => store.set(newEntry.Id, [newEntry]));
});

Given('{string} has the item {string}', async ({}, entryName: string, itemName: string) => {
  entries.tap((store) => {
    const [entryId, list] = [...store.entries()].find(([, list]) =>
      list.find((q) => q.Name === entryName)
    ) ?? [null, null];
    if (!entryId || !list) throw new Error('target entry not found!');
    const updated = list.map((q) => {
      if (q.Name !== entryName) return q;
      const newItem = item({ name: itemName });
      return { ...q, items: q.items.concat(newItem) };
    });
    store.set(entryId, updated);
  });
});

Certain of these steps are usable by other, more granular tests that are in different suites. I do want to expose those steps to be sharable (though only by explicit registration). Other steps in this file are more specific and I want them to not be usable elsewhere. This makes it very straight forward to find the specific step definition a given tests is using by tracing the import path in an IDE. It also prevents step discovery that is semi-"magical".

The above shareSteps utility is a small helper I wrote to create a registration function, and it looks like:

import { createBdd, TestType } from '@resolution/playwright';

type BDD<T extends object, W extends object> = ReturnType<typeof createBdd<T, W>>;
const registered = new Map<unknown, Set<unknown>>();

export function shareSteps<BT extends object, BW extends object>(
  _: TestType<BT, BW>,
  registrationFn: (bdd: BDD<BT, BW>) => void
) {
  return <DerivedBDD extends BDD<BT, BW>>(bdd: DerivedBDD) => {
    if (registered.has(bdd) && registered.get(bdd)?.has(registrationFn)) return bdd;
    if (!registered.has(bdd)) registered.set(bdd, new Set());
    registrationFn(bdd);
    registered.get(bdd)?.add(registrationFn);
    return bdd;
  };
}

This is fine, but it only creates the illusion of traceability, because the other, non-shared steps are still visible and available to any steps file that imports the first file, even if they only imported the file to use the register* function.

A solution

The solution I had in mind would be to add a strictRegistration option to the input configuration, which scopes the StepRegistry for each created instance returned by createBdd?

vitalets commented 3 weeks ago

Would not step tagging per https://github.com/vitalets/playwright-bdd/issues/205 be a solution for that?

To have steps exclusively bound to entityEditor, you can define them with tag:

Given(/I am on the "(.*)" page of "(.*)"/, { tags: '@entityEditor' }, async () => { ... });
Given(/I am on the "(.*)" page of a new inventory entry/, { tags: '@entityEditor' }, async () => { ... });

And in the feature file:

@entityEditor
Feature: Entity Editor

  ...

Other steps without tag are shareable across all features.

I personally like even more the idea in https://github.com/vitalets/playwright-bdd/issues/217, so we can put all entityEditor stuff into tagged folder and steps will be automatically bound to it:

features
└── @entityEditor
    ├── entityEditor.feature
    └── steps.ts

All steps from @entityEditor/steps.ts can be used only in entityEditor. If other suites need them, they should explicitly have @entityEditor tag.

Some other questions, to better understand the context:

  1. Certain of these steps are usable by other... Other steps in this file are more specific

    Would not it better to split shareable and non-shareable steps into two different files?

  2. I do want to expose those steps to be sharable (though only by explicit registration)

    Can tags serve as explicit registration of using steps? Otherwise, as a feature-writer how can I know which steps can I use for the particular feature?

  3. This makes it very straight forward to find the specific step definition a given tests is using by tracing the import path in an IDE.

    Do you start tracing from feature file by clicking on the step?

Aaron-Pool commented 3 weeks ago

@vitalets Thank you for your speedy response and detailed thoughts. I'll try address these thoughts individually. Ultimately, I completely respect the decisions you've made and your overall approach with playwright-bdd, so I'm just going to try to outline the outcome I see as desirable, and if it seems reasonable to you, then that's great, and if not, I get it.

Would not it better to split shareable and non-shareable steps into two different files?

I did consider this, but ultimately I feel that traceability is such a "solved problem" on the javascript/typescript side, so to me it seems the most logical to use code, rather than file structure or tagging conventions, to drive traceability. Because of this, I organize my tests in pairs of *.feature and step.ts files. And the mental model I try to communicate to the junior devs that I manage is that the step.ts file that lives beside a *.feature file is the source of truth for what Given/Then/When statements are available in that feature file. If the Given in a step.ts file is not provided a definition of a specific step by some means, it won't be able to run that step in the feature file. Which, wise or unwise, is my attempt to make the "magic gap" between the .feature and .ts worlds as small as possible (e.g. - just between two co-located files). So even though in this specific example splitting the files would prevent the incidental step registration, it doesn't resolve the overarching issue of incidental step registration happening due to import side effects 🤷‍♂️

Can tags serve as explicit registration of using steps? Otherwise, as a feature-writer how can I know which steps can I use for the particular feature?

My team specifically doesn't have any dedicated "feature writers". The same devs who automate tests write feature files, and our QA specialists/Analysts simply review those feature files for gaps/unconsidered use cases.

Do you start tracing from feature file by clicking on the step?

Personally, I do not, no. Because most IDE tools that enable this feature, to my knowledge, rely on globs and implicit attachment of step definitions to feature files. And perhaps I'm overly overparticular about this, but I really value explicit dependencies over implicit dependencies. It's probably a result of trauma from my experiences in the early 2010s pre-modular javascript world.