Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested steps should query over the wire too #45

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

mattwynne
Copy link
Member

@mattwynne mattwynne commented Jul 16, 2021

This turns #1 into a failing test.

It seems like when we use a nested step, the mechanism for making that dynamic query for step defs does not query over the wire.

This is a symptom of the complexity around nested steps, and not necessarily something we want to prioritise fixing.

@aurelien-reeves
Copy link
Contributor

Copied from #1 (comment):

It seems nested steps are suggest to discussion: cucumber/cucumber-ruby#1362

Maybe not supporting this in wire is not that bad?

@aurelien-reeves
Copy link
Contributor

Actually it seems to work well as long as the nested step actually exist on the wire server

Did I miss something @mattwynne?

This reverts commit 14593cd.

This was based on a misunderstanding: this step is implemented over the
wire already, so adding a step definition for it on the ruby side just
masks the problem the scenario is supposed to highlight.
@mattwynne
Copy link
Member Author

mattwynne commented Jul 13, 2022

I've updated the feature file to hopefully clarify the problem being outlined.

Notice how the Given a step over the wire step passes when run from the feature file, but when you run the same step from Ruby step "a step over the wire" it fails with Undefined dynamic step. This is because Cucumber does not respect the "plugin" architecture for step definitions that we use here when searching for dynamic/nested steps.

@luke-hill
Copy link
Contributor

@mattwynne maybe now I'm back a bit more we want to start deprecating / removing the ability for people to create dynamic steps?

@mattwynne
Copy link
Member Author

Rather than removing nested steps altogether, I'd be in favour of re-architecting things so that they can be offered as a plugin. I know a lot of people use them, so I think it would be annoying to remove the behaviour altogether, but making it opt-in would communicate more clearly to novices what we consider to be good practice.

I also think it would probably be healthy for Cucumber's internals if we were able to decouple the code around this stuff enough that it could be offered through and extension point. Right now that's some of the most ancient code in the whole Cucumber org, and it could probably benefit from some love.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants