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

Keep setDefinitionFunctionWrapper around #1772

Closed
christian-bromann opened this issue Aug 20, 2021 · 8 comments
Closed

Keep setDefinitionFunctionWrapper around #1772

christian-bromann opened this issue Aug 20, 2021 · 8 comments
Labels
✅ accepted The core team has agreed that it is a good idea to fix this ⚡ enhancement Request for new functionality

Comments

@christian-bromann
Copy link

Is your feature request related to a problem? Please describe.
I am currently trying to update Cucumber within WebdriverIO to the latest version and saw that setDefinitionFunctionWrapper is marked as deprecated. I would vote for keeping it around (doesn't need to be publicly documented) so WebdriverIO can make use of it.

The reason we need to wrap the step definition is because we have our own custom hooks around steps and hooks. We use that for our retry mechanisms and other things.

Describe the solution you'd like
It would be amazing to keep it around as we can't just insert a Cucumber hook to achieve this. It doesn't need to be publicly documented.

Describe alternatives you've considered
I am not sure if there is any other way to wrap individual steps and hook in Cucumber. If there is one, please let me know.

Additional context
Here is the part where we wrap things: https://github.com/webdriverio/webdriverio/blob/main/packages/wdio-cucumber-framework/src/index.ts#L296-L322

@aurelien-reeves aurelien-reeves added the ❓ question Consider using support forums: https://cucumber.io/tools/cucumber-open/support label Aug 23, 2021
@davidjgoss davidjgoss added ✅ accepted The core team has agreed that it is a good idea to fix this ⚡ enhancement Request for new functionality and removed ❓ question Consider using support forums: https://cucumber.io/tools/cucumber-open/support labels Sep 3, 2021
@charlierudolph
Copy link
Member

Taking a look at your code reference, there appears to be a few different things.

  1. Enabling retry. Cucumber itself has retry now but its done at the scenario level. Do you need retry at the step level? If so, could retry be handled another way (ie user has helpers for retry logic)

  2. Before / after step hooks - cucumber-js now has these as well. Could these be used instead?

Are there other things you are accomplishing with the step definition wrapper or are there other items?

Also, at a high level, how is your library using cucumber?

Instead of locking ourselves to the features that existed before (for a much different reason), I think it'd be nicer to start fresh to see what problem you are trying to solve and see if we can agree on a feature set that makes sense for cucumber-js and your library

@christian-bromann
Copy link
Author

Thanks @charlierudolph for responding!

  1. Enabling retry. Cucumber itself has retry now but its done at the scenario level. Do you need retry at the step level?

Not really. Happy to have users use the standard way of retrying. Can you reference some docs around this?

2. Before / after step hooks - cucumber-js now has these as well. Could these be used instead?

Can I register these hooks when initialising the framework? WebdriverIO has their own before/after-Step/Freature/Scenario hooks and need to be able to plug them into the Cucumber framework.

Also, at a high level, how is your library using cucumber?

We use Cucumber programatically to allow running it within the WebdriverIO framework. The challenge we have is to make sure our reporting framework works with the one Cucumber has (which is why we have our own eventListener as well as ensure that WebdriverIO services work (which is why we register things like Cucumber.Before in advance).

Actually after revisiting this it appears that the reason we need setDefinitionFunctionWrapper is that we have to wrap every step definition function into Fibers to allow synchronous execution. Now, we already announce deprecating it as it won't be supported with Node.js v16 and upwards. I wonder if we can keep it deprecated until Node.js v14 is out of LTS?

@davidjgoss
Copy link
Contributor

@christian-bromann

Retry docs are here https://github.com/cucumber/cucumber-js/blob/main/docs/retry.md - let me know if anything unclear

With BeforeStep and AfterStep, they work in more or less the same way as Before and After, so if you're able to integrate with those you should be fine.

@christian-bromann
Copy link
Author

@davidjgoss beforeStep and afterStep don't really help me as I need to wrap the function passed into a step definition or hook.

@charlierudolph
Copy link
Member

Node 14 LTS ends in April 2023.

Could you have the version of your code that wraps the definitions just rely on the old version of cucumber or is there something that you need from the latest version of cucumber?

Also if you need to wrap user provided functions that are certainly other ways to do it, although they certainly wouldn't be as nice as the previous API (could create wrapped step definitions functions or provide a helper to users to wrap their own functions).

Does your library require the functions to be wrapped or is that just the suggested? (ie you could use async/await instead of synchronous execution.

@christian-bromann
Copy link
Author

Could you have the version of your code that wraps the definitions just rely on the old version of cucumber or is there something that you need from the latest version of cucumber?

I haven't had anyone asking to update Cucumber to get access to a specific feature. We however regularly update versions so it is hard to answer the question.

although they certainly wouldn't be as nice as the previous API

And it would be a breaking change that would cause many people to not update.

Does your library require the functions to be wrapped or is that just the suggested?

WebdriverIO provides sync and async execution. Many people coming from other languages like Java or Python preferred synchronous execution and have written their step definitions this way. As we are not able to continue support for sync execution we are working on helping the user base to transition to async/await. However it will take a while until we moved the majority of the user base.

What would be the harm of Cucumber to keep the method around? I am fine if it is undocumented and even marked as deprecated.

@davidjgoss
Copy link
Contributor

Closing as this was added back in #1795 - thanks for the feedback!

@christian-bromann
Copy link
Author

Thanks for considering to keep it, much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✅ accepted The core team has agreed that it is a good idea to fix this ⚡ enhancement Request for new functionality
Projects
None yet
Development

No branches or pull requests

4 participants