Fix duplicate step registration

timjroberts / cucumber-js-tsflow

Provides 'specflow' like bindings for Cucumber.js in TypeScript 1.7+.

MIT License

133 stars 34 forks source link

Fix duplicate step registration #53

Closed justinbhopper closed 4 years ago

justinbhopper commented 4 years ago

This fixes #52. The root causes were:

The steps were being registered multiple times.
The "Ambiguous step definitions" error was never being displayed, due to the Error being returned instead of thrown. This was making it appear as though steps were being ignored.

This PR fixes the "Ambiguous step definitions" error so it is now thrown.

This PR also adds a duplicate check using the Callsite's filename and lineNumber. If the filename and lineNumber is the same, then we should assume the step file is being compiled again and we should not re-register the binding.

justinbhopper commented 4 years ago

@timjroberts @mikehaas763

Is there anything I can do to help give insight or better context to this fix?

justinbhopper commented 4 years ago

@wudong What are the chances of this PR getting merged? I ask because it seems like nothing has been committed since Dec 2018, so I'm wondering if the repo has been abandoned.

timjroberts commented 4 years ago

I started this as an experiment in applying TypeScript decorators as bindings to CucumberJS and didn't really expect it to be as widely used as it is. I no longer work in the Node.js space so I haven't contributed to it in a long time.

@justinbhopper, @wudong - would either of you like to come on board as contributors and keep the project going?

wudong commented 4 years ago

@timjroberts I am happy to be a contributor of the project. My company does use it quite a lot and would like to see it to grow!

timjroberts commented 4 years ago

@wudong - An invite has been sent!

mikehaas763 commented 4 years ago

That's awesome to see more people interested in contributing back! Welcome!

mikehaas763 commented 4 years ago

@justinbhopper thanks for the contribution! I'm assuming you ran the test suite ran successfully, as well as the husky lint and formatting commands? We could really use a pipeline that hooks into the github status checks 🙂

wudong commented 4 years ago

Yeah, it is a good idea. I see if i can add github action into the project.

justinbhopper commented 4 years ago

I've removed the changes that were unrelated to the PR, such as the as any castings. VSCode's intellisense was mistakenly using the latest version of TS instead of the workspace's version.

wudong commented 4 years ago

@mikehaas763 @timjroberts I have merged this PR and bump the path version. But I cannot publish to the npm. can either one of you publish it or grant me permission? my npm login is 'wudongliu'.

justinbhopper commented 4 years ago

@mikehaas763 @timjroberts

Can either of you please respond? Maintenance on this repo is dead in the water without someone available to grant publish permissions.

timjroberts commented 4 years ago

@wudong - I've just added you as a maintainer to https://www.npmjs.com/package/cucumber-tsflow

wudong commented 4 years ago

Thanks @timjroberts. new release go out fine now. However, i was meant to add the CI/CD pipeline with github actions, so we could make release more easily. But I don't seem to have permission to add the npm token into the repo (missing the 'setting' tab), anyone know is contributor supposed to be able to do that ?

timjroberts commented 4 years ago

Unfortunately @wudong, I have no experience using GitHub Actions, and this repo pre-dates them. I did find this article which seems to document how you might do it though:

https://sergiodxa.com/articles/github-actions-npm-publish/

It appears to use repository level secrets to store the NPM token.

justinbhopper commented 4 years ago

@wudong The new npm version just published does not appear to have a dist folder