[Discussion] Scoped Step Definitions

vitalets commented 1 month ago

The problem

Cucumber forbids duplicate step definitions. It is considered anti-pattern to create feature-specific steps.

Meanwhile, there are many requests from community to allow coupling steps with particular feature scope.

A typical use-case Imagine two features game.feature and video-player.feature, both having own PLAY button. game.feature:

Given I have not started a game yet
When I click the PLAY button # <- duplicated step
Then the game begins

video-player.feature:

Given I am watching a youtube video
When I click the PLAY button # <- duplicated step
Then the video plays

Step implementation for I click the PLAY button is different for each feature. But if you define it in two different files, you will get an error:

Multiple step definitions matched for text: "I click the PLAY button"

It forces you to modify step pattern to avoid the ambiguity. E.g.

When('I click the PLAY button in game', ...);
When('I click the PLAY button in video player', ...);

That's annoying.

Existing solutions

Non of official Cucumber implementations supports duplicate steps. They report ambiguous step error once your step matches more than one step definition.
Cucumber plugin for Cypress introduced interesting feature called Paring. It allows to have duplicate step definitions paired to particular features via special configuration pattern (see more in their docs).

Proposal

I think, scoping steps to particular domain is reasonable, especially for large applications. I see several options to achieve that.

Option 1

Re-implement pairing via [filepath] / [fileparts] in steps configuration:

defineBddConfig({
  features: 'features',
  steps: [
    'features/steps/common.ts',
    'features/steps/[filepath].ts', // <- pair steps to particular feature
  ],
});

It solves the use-case above with the following file structure:

└── features/
    ├── steps/
    │   ├── common.ts
    │   ├── game.ts
    │   └── video-player.ts
    ├── game.feature
    └── video-player.feature

Or with [filepart] for feature-focused structure:

defineBddConfig({
  features: 'features',
  steps: [
    'features/steps/common.ts',
    'features/[filepart]/steps.ts', // <- pair steps to particular feature
  ],
});

Files structure:

└── features/
    ├── steps/
    │   └── common.ts
    ├── game/
    │   ├── game.feature   
    │   └── steps.ts
    └── video-player/
        ├── video-player.feature    
        └── steps.ts

Option 2

While thinking about steps pairing in Cypress plugin, I've got another idea how it can be implemented. First, I've noted two drawbacks of pairing approach:

You can't just define steps as a single string pattern, see a common mistake. You should make it more complex, splitting on common steps + pairing pattern steps.
Pairing can't be resolved without reading the configuration file. That is mostly for tools like IDE extensions, for navigating to step definition by cmd + click. Currently, the most popular one does not support it, but hopefully will.

The solution is inspired by Next.js route groups. We can introduce steps scope - a file or directory with name in parenthesis, e.g. (game) or (video-player).

Step definitions inside scoped directory are applicable only to features inside that directory.

This is the only rule one should know to understand the approach.

Now we can define the file structure:

└── features/
    ├── steps/
    │   └── common.ts
    ├── (game)/
    │   ├── game.feature   
    │   └── steps.ts
    └── (video-player)/
        ├── video-player.feature    
        └── steps.ts

(game)/steps.ts are applied only to game.feature
(video-player)/steps.ts are applied only to video-player.feature
steps/common.ts are applied to both

The main advantage is that any tool or human can understand paring without reading configuration. The BDD configuration itself simply defines steps without any patterns:

defineBddConfig({
  features: 'features',
  steps: 'features/**/*.ts',
});

If project has separate directories for features and steps, the rule can be slightly enhanced:

Step definitions inside scoped directory are applicable only to features having that scope in the path.

Now the following structure is available:

└── features/
    ├── steps/
    │   ├── common.ts
    │   ├── (game).ts
    │   └── (video-player).ts
    ├── (game).feature   
    └── (video-player).feature

steps from features/steps/(game).ts will be applied only to features/(game).feature, because feature path contains (game)
steps from features/steps/(video-player).ts will be applied only to features/(video-player).feature, because feature path contains (video-player)
steps from features/steps/common.ts will be applied to both features, because there are no scoped directories in steps path

Such file structure explicitly shows how features are connected to steps.

Your feedback is welcome!

All of you have different projects, with unique setup and structure. It would be very helpful, if your apply (virtually) these approaches to your project and share your feedback / concerns in the comments. Or just add your 👍 Thanks in advance!

rschulz-scisys commented 2 weeks ago

Since working on an enterprise level product I tend to bundle the things that belong together so they can easily be found and related to. Thus the tests for an angular component are on the same level in the project structure. Any of the approaches would work for us but I like the explicit definition of the 2nd approach. I just think we have to define it in a resilient way. What would happen if in the path there are multiple scopes, like this

.../modules/login/tests/(login)/(auth).feature

or what happens if a user decides to do something like this:

(auth)-(login).feature

when working with a constantly changing team you never know what a user is going to do ;-)

Thinking a bit about it, at least for my use cases it would be sufficient to treat everything inside the same directory as scoped together and I would prefer to explicitly mark the steps that are generic somehow, since our application gets quite big and it is hard to define extra scopes everywhere.

vitalets commented 2 weeks ago

@rschulz-scisys good points!

For multi-scoped paths like tests/(login)/(auth).feature I think we should treat whole sequence as a scope. So, scope here is (login),(auth). And it matches exactly the same scope in the same order, e.g. tests/steps/(login)/(auth)/steps.ts.
For (auth)-(login).feature well.. my suggestion is to handle it in the same way, it's just a sequence of (login),(auth) and it will match (login)/(auth)/steps.ts. I totally agree, it should be documented.
Thinking a bit about it, at least for my use cases it would be sufficient to treat everything inside the same directory as scoped together and I would prefer to explicitly mark the steps that are generic somehow, since our application gets quite big and it is hard to define extra scopes everywhere.

I'd like it to be possible - instead of marking every feature directory with parenthesis, mark common steps. Taking previous example, here (steps)/common.ts are shared between all features:

└── features/
    ├── (steps)/
    │   └── common.ts
    ├── game/
    │   ├── game.feature   
    │   └── steps.ts
    └── video-player/
        ├── video-player.feature    
        └── steps.ts

But in that case we force users to put steps close to features. I personally like such structure more, but projects are very different. Somewhere it can be a strict rule to keep feature files separate from steps. How would we solve that?

And another case - if someone decides to add nested directories inside feature, e.g.:

└── features/
    ├── (steps)/
    │   └── common.ts
    ├── game/
    │   ├── levels/
    │   |   ├── beginner.feature   
    │   |   └── professional.feature 
    │   └── steps.ts

level/beginner.feature can't access ../steps.ts because scoped by levels/. Making game steps common as game/(steps).ts is also not a solution as I want these steps be shared inside game dir, not entire project. What are your thoughts on that?

viktor-silakov commented 1 week ago

Instead of using pairing or strict scoping rules for step definitions, I propose adding the ability to override step definitions for specific directories or subdirectories containing feature files. In these directories, an override.ts file can be created, containing references to imported step definitions that are specific to that directory and its subdirectories.

└── features/
    ├── steps/
    │   ├── common.ts
    ├── game/
    │   ├── override.ts  # Overrides for game
    │   │   └── steps: [
    │   │       'features/steps/game-specific.ts',
    │   │     ],
    │   ├── levels/
    │   │   ├── beginner.feature   
    │   │   └── professional.feature 
    ├── video-player/
        ├── override.ts  # Overrides for video-player
        │   └── steps: [
        │       'features/steps/video-player-specific.ts',
        │     ],
        └── video-player.feature

Advantages:

•   Simplifies project configuration by eliminating the need for complex pairing rules.
•   Provides flexibility and control over step definitions within specific directories, allowing tests to be adapted for different modules or application features.
•   Enables the use of common step definitions while easily overriding them where necessary by importing the relevant files.

Concern:

There may be challenges with how IDE extensions handle these overrides, potentially affecting auto-completion and navigation to step definition implementations.

vitalets commented 1 week ago

@viktor-silakov interesting idea. Could you show how exactly override.ts would look like? Is it file with steps or only with references to feature path / steps?

viktor-silakov commented 1 week ago

@viktor-silakov interesting idea. Could you show how exactly override.ts would look like? Is it file with steps or only with references to feature path / steps?

Reference to step definition files that are not imported in the main config.

For example, we have definitions in common.ts:

sd1
sd2
sd3

In game.override.ts:

sd2

After merge we have:

sd1 (main.ts)
sd2 (game.override.ts)
sd3 (main.ts)

for the game folder and their subfolders. I think this is a more flexible approach and quite explicit.

vitalets commented 1 week ago

For me it's still not clear, what is inside game.override.ts. You said it contains reference to step definition file, can you show what exactly the reference is? Lets take an example:

// features/steps/common.ts
When('I click the PLAY button', () => { 
  console.log('[common] click the PLAY button');
});

Now we want to override this step for game:

// features/game/overides.ts

// reference ???

When('I click the PLAY button', () => { 
  console.log('[overriden for game] click the PLAY button');
});

viktor-silakov commented 1 week ago

This is not really important, I just wouldn't want to store the implementation of steps in the feature folder. Besides, it gives more flexibility, and allows merging several files with implementation, which I don’t need right now, but won’t be superfluous. Therefore, I would suggest simply storing links in some form, for example, you can write something like:

// features/steps/common.ts
When('I click the PLAY button', () => { 
  console.log('[common] click the PLAY button');
});

// features/steps/games.ts
When('I click the PLAY button', () => { 
  console.log('[overriden for game] click the PLAY button');
});

// features/game/overides.ts
export overrides = [
    'features/steps/games.ts'
]

rschulz-scisys commented 1 week ago

For multi-scoped paths like tests/(login)/(auth).feature I think we should treat whole sequence as a scope. So, scope here is (login),(auth). And it matches exactly the same scope in the same order, e.g. tests/steps/(login)/(auth)/steps.ts.

For (auth)-(login).feature well.. my suggestion is to handle it in the same way, it's just a sequence of (login),(auth) and it will match (login)/(auth)/steps.ts. I totally agree, it should be documented.

Interesting, would get a while to get used to but could actually be quite practical ;-)

I'd like it to be possible - instead of marking every feature directory with parenthesis, mark common steps. Taking previous example, here (steps)/common.ts are shared between all features:
└── features/
    ├── (steps)/
    │   └── common.ts
    ├── game/
    │   ├── game.feature   
    │   └── steps.ts
    └── video-player/
        ├── video-player.feature    
        └── steps.ts
But in that case we force users to put steps close to features. I personally like such structure more, but projects are very different. Somewhere it can be a strict rule to keep feature files separate from steps. How would we solve that?

I think we're alike here, that would also be my personal favorite. With such a structure I would as good as never run into a redefinition of a step, except when it is coming from a common step. This would make life so much easier ;-).

The larger an application grows the harder it gets to maintain when it is not structured. I always have the feeling that flat structures are nice for quick and easy but they do not scale. For us it would even be hard to find filenames that do not clash ;-)

But I also see your point as know of quite some projects that would go the flat way.

So, I was thinking about it a bit and maybe it is a way to go with annotations in the files, since both, js and feature files allow comments, we could do something like

# bdd-context: authentication

and in the step definitions also

// bdd-context: authentication

we could even allow multiple contexts

I may need to think about it a while longer...

vitalets commented 1 week ago

// bdd-context: authentication

This is very close to the suggestion by @viktor-silakov with special export in steps file:

export overrides = [ 'features/steps/games.ts' ];

Are you guys sure, we should add this new level of managing stuff? Context names / feature file names can't be type-checked, how to keep them in sync? If I make a typo in bdd-context: authentciation, it could be difficult to find out why tests started to fail..

rschulz-scisys commented 1 week ago

// bdd-context: authentication

This is very close to the suggestion by @viktor-silakov with special export in steps file:
export overrides = [ 'features/steps/games.ts' ];

yes and no, it does not require you to give the full/relative path of features and it does not require an additional export, it is fully relying on a preprocessor probably, you know the implementation details better then I do.

Are you guys sure, we should add this new level of managing stuff? Context names / feature file names can't be type-checked, how to keep them in sync? If I make a typo in bdd-context: authentciation, it could be difficult to find out why tests started to fail..

true, and no I am not sure yet that this is the way to go, but we're discussing and it seemed to be one of the few approaches that works without a hierarchy/ structure requirement also in flat projects.

The issue is mainly keeping on top of the complexity. Thus I kinda like the idea of everything being scoped per default as everything else ramps up complexity quite fast. This is one way of doing it, but I could also live with an appraoch based on filenames. If there is a pair like component1.feature and component1.steps.ts it is scoped (thats probably being back to pairing) and step files that cannot be matched to a feature file are automatically common. That would also assume that paired step definitions are unavailable to any other feature file, which would be fine for me. Interesting things may happen if feature files have the same name in different directories (not in flat projects as those are safe in that case).

vitalets commented 1 week ago

Thanks for all the input @rschulz-scisys :)

One thing I thought during reading the last message. Actually, in Cucumber already there is a way to bind some code to the particular feature. When I define hook, I can bind it to feature by tags. For example:

Before({ tags: "@game" }, function () {
  // This hook will be executed before scenarios tagged with @game
});

So, one more idea - just extend that existing approach to steps:

When('I click the PLAY button', { tags: '@game' }, () => { 
  console.log('[overriden for game] click the PLAY button');
});

Such step will be applied only to features tagged with @game:

@game
Feature: Game
...

No rules for directory structure, can be applied to any project. The downside - if there are many feature bound steps in a file, it requires you to mark them all with tag. What do you think?

rschulz-scisys commented 1 week ago

Thanks for all the input @rschulz-scisys :)

you're welcome ;-)

So, one more idea - just extend that existing approach to steps:
When('I click the PLAY button', { tags: '@game' }, () => { 
  console.log('[overriden for game] click the PLAY button');
});
Such step will be applied only to features tagged with @game:
@game
Feature: Game
...
No rules for directory structure, can be applied to any project. The downside - if there are many feature bound steps in a file, it requires you to mark them all with tag. What do you think?

That sounds nice and totally in line with the cucumber way (well, kinda). I think I like that approach as this will give all the needed flexibility. I could tag the features of a specific component/ area with (0..n) tags and then just need to make sure that the tags are consistent (e.g. @component1).

Will that solve the duplicate step definition? It might. The syntax is definitely good for it. A bit of work for the preprocessor probably and IDE tools.

The approach would work even right now for us since we still have the overall problem with the dupe steps and were forced to create unique ones. So we could adapt one by one with this approach.

thumbs up from me

alescinskis commented 1 week ago

I personally prefer the original approach with all of their cons, so would be great if duplicate step definition feature will be possible to enable only via config flag + providing paths (or whatever extra settings you come up with).

rschulz-scisys commented 1 week ago

I personally prefer the original approach with all of their cons, so would be great if duplicate step definition feature will be possible to enable only via config flag + providing paths (or whatever extra settings you come up with).

with the last approach you just would not use the tags in the steps and everything should be as it was before, if I got it right. No need for a config in that case. Or did I misread your comment wrt original approach? Did you mean as it is right now or did you mean the pairing options described as first option?

vitalets commented 1 week ago

@alescinskis could you share more on which exactly approach you'd like?

alescinskis commented 4 days ago

@vitalets sorry, should have made it more clear, I'm talking about default approach where cucumber forbids duplicate step definitions. I want to be able to keep that strict approach.

vitalets commented 4 days ago

@vitalets sorry, should have made it more clear, I'm talking about default approach where cucumber forbids duplicate step definitions. I want to be able to keep that strict approach.

Yes, that would be a default. Until tags manually assigned to step definitions, everything will work as now and duplicate steps will be not allowed and reported.

vitalets commented 2 days ago

I've discovered that SpecFlow has exactly the same approach we ended up here - use tags to bind step definitions to features. Even the name is the same - Scoped Step Definitions! Everything is already invented, we just need to search :)

rschulz-scisys commented 1 day ago

I've discovered that SpecFlow has exactly the same approach we ended up here - use tags to bind step definitions to features. Even the name is the same - Scoped Step Definitions! Everything is already invented, we just need to search :)

and even with a little more options (by Feature and Scenario name). Seems the approach would work and is acceptable. Now you have to tell us, if it would be working in this implementation and if you would be willing to implement it ;-)

vitalets commented 1 day ago

Yes, I will add it to playwright-bdd.

I'm not sure about binding by Feature and Scenario name. These entities are free text. When I change them, I don't expect that some code is bound to the text. I assume it can often lead to failing tests, and a user will have no clue why.

I'd suggest to implement only tags for the start, live with it and consider other binding options if we get a strong request for them.

vitalets / playwright-bdd