timjroberts / cucumber-js-tsflow

Provides 'specflow' like bindings for Cucumber.js in TypeScript 1.7+.
MIT License
133 stars 34 forks source link

cucumber-tsflow Direction #64

Open timjroberts opened 4 years ago

timjroberts commented 4 years ago

I started this project as an experiment back in early 2016. At the time I was working full time on Node.js projects (my background was C#/.NET), and when TypeScript announced support for decorators I decided to start this experiment to see if the API surface of CucumberJS could be made more familiar. In our .NET work we used SpecFlow quite heavily and that was a major inspiration for this piece of work.

It appears that this repo has some following. npmjs.com reports weekly downloads of ~12K and it has more stars than any of my other open source work. However, I no longer work on Node.js projects after returning predominately to .NET work and haven't really contributed to it in such a long time. @mikehaas763 offered to take up maintenance a while back, and more recently, @wudong has joined to help out. This has seen some long outstanding PRs and issues being resolved and released recently which I am forever grateful for.

However, I want to gauge some feeling and opinion about the library. Where can it go from here? Is there anything we can improve upon, are there features of CucumberJS that we can expose, address, or make more familiar? Is the library due a re-write?

I'd be interested to hear people's thoughts.

wudong commented 4 years ago

@timjroberts I think it is a good project and fills some needs in the cucumber community. And we already have a fair amount of use base it would be a shame to just bin it. I'm happy to maintain / improve it whenever I can.

For the near future, I think it would be good to focus on

Feature wise, a few things i can think of on top of my head:

  1. bring it more aligned with cucumber-js, and try to support full feature set of cucumber-js. Notably missing is BeforeAll/AfterAll hooks.
  2. Code tractability. Currently the step definition are all traced back to the hook functions within cucumber-js-tsflow, rather than where it should be.

any more suggestions?

kferrone commented 4 years ago

We use this project a lot. I agree with all the above ideas. Maybe a good lil Github website for the docs to live and some advertising would do this project some good.

AlexanderSchiff commented 4 years ago

It would be nice if this were updated to use @cucumber/cucumber. Other than that, I love the package and it works really well.

timjroberts commented 3 years ago

The original experiment was to build something that was more aligned with SpecFlow (since we were .NET Developers). TypeScript decorators was the key enabler of that. SpecFlow has notions of FeatureContext, ScenarioContext, and so on, which isn't necessarily present in cucumber-js.

Arguably, these would be value adds, but on the other hand, maybe it should align more with cucumber-js. The more it does align with cucumber-js though, the more it negates the need for this library. For example, I've seen a few issues talk about accessing the cucumber-js World. cucumber-tsflow tries to follow SpecFlow's notion of Context Injection to support that though.

Perhaps, we should blend the concepts more.

Fryuni commented 1 year ago

Hey, I just joined as a contributor from another issue and wanted to pitch in here in case yall want to discuss this further.

For the near future, I think it would be good to focus on

  • improve test coverage

And add a coverage report from the CI on a badge. A library for tests should itself be properly tested 😛

  • add logging

I saw that some progress was made in this direction, but it is not clear to me yet where extra logging would be beneficial for the consumer and bug reporting.

  • update and test newer dependencies versions. cucumber.js version 5 -> 6

Since the master branch was pointing to Cucumber v7 and none of the breaking changes from v8 break this library I just change the CI tests to ensure that every PR is compatible with both v7 and v8 of Cucumber.

  • clean code & refactoring to make it more concise and pretty.

Feature wise, a few things i can think of on top of my head:

  1. bring it more aligned with cucumber-js, and try to support full feature set of cucumber-js. Notably missing is BeforeAll/AfterAll hooks.

I'll take a look at your PR (#51) to move this forward.

  1. Code tractability. Currently the step definition are all traced back to the hook functions within cucumber-js-tsflow, rather than where it should be.

I have some ideas regarding this.

Maybe a good lil Github website for the docs to live and some advertising would do this project some good.

I love this idea, but this is something that I sadly can't make by myself. I love writing docs, but my front-end skill approaches zero from the negative side. If anyone wants to make a pretty website, I can help with the text.

It would be nice if this were updated to use @cucumber/cucumber. Other than that, I love the package and it works really well.

I agree with this sentiment; this package is awesome. MUCH cleaner to use than relying solely on the core World object.

I fixed the CI to properly run the tests with the new peer dependency on a more recent node and npm versions (which have major breaking changes on how peer dependencies work).

I aim to have a stable v4 with all the features tested and validated on the CI for the new versions of Cucumber.

Speaking of versions, I'm still getting myself familiar with the release process used on this library, I want to understand it before I touch anything, and even though I managed to pack it locally, I won't try to make a release without being sure.

timjroberts commented 1 year ago

Thanks for coming onboard @Fryuni!

With regards to releases, I was:

There is already a release/4.0 branch that can be used as a frame of reference. It might also be easier to simply recreate it from master with all the work that you've now included. Also, if you're happy with the work, then I'm also happy not releasing release candidate builds first and going straight to 4.0.0.

Fryuni commented 1 year ago

Now I see what got me confused. The workflow sets a semver pre-release suffix but does not flag the version on NPM as a pre-release. This might require some changes on the CI

There is already a release/4.0 branch that can be used as a frame of reference. It might also be easier to simply recreate it from master with all the work that you've now included.

I merged master with the release branch already :)

Also, if you're happy with the work, then I'm also happy not releasing release candidate builds first and going straight to 4.0.0.

I think a release candidate will be appropriate.

timjroberts commented 1 year ago

The release workflow simply calls npm publish. So you're correct, no dist-tag information is included. It shouldn't take too much work to modify the workflow to call npm again to set the dist-tags for a pre-release build. I may take a look over the weekend.

it looks like I released 4.0.0-rc.1 2 years ago?!?! Oh dear. :-)

image

Since it uses the git height, the workflow will set the next rc release to something like 4.0.0-rc.45 (depending on the number of commits that have gone in since version.json was last modified). When 4.0 is ready, simply change the version in version.json to 4.0.{height} and it should then start producing 4.0.n builds from that branch instead.

Fryuni commented 1 year ago

So you're correct, no dist-tag information is included. It shouldn't take too much work to modify the workflow to call npm again to set the dist-tags for a pre-release build. I may take a look over the weekend.

I made it configurable on each branch on #108. I looked at the schema for version.json and it should ignore additional properties. But since I've never used the Hashbank.GitVersioning can you check it?

it looks like I released 4.0.0-rc.1 2 years ago?!?! Oh dear. :-)

I'll merge master into release/4.0 afterwards and cut an RC. Getting closer to a v4 🎉

Since it uses the git height, the workflow will set the next rc release to something like 4.0.0-rc.45 (depending on the number of commits that have gone in since version.json was last modified). When 4.0 is ready, simply change the version in version.json to 4.0.{height} and it should then start producing 4.0.n builds from that branch instead.

As mentioned, I've never used this (in fact, I've never heard of this). But after reading the reasoning on their README I think it is quite clever. I miss a GH release as an anchor to the code used on an NPM release, but that can be added to the CI later on. There are always things to improve.

ericrommel commented 1 year ago

Hi everyone,

Regarding this point:

bring it more aligned with cucumber-js, and try to support full feature set of cucumber-js. Notably missing is BeforeAll/AfterAll hooks.

I've updated my local using version 4.0.3 and now I can see/import beforeAll/afterAll. However, I'm getting a "binding" error when trying to use context injection and beforeAll decorator. If I use before decorator, everything works fine.

Here is how I'm using it:

@binding([Login, Client])
class HookSteps {
  constructor(protected login: Login, public client: Client) { }
  @beforeAll()
  public async beforeAll(): {
    await this.login.LogIn()
    await this.client.Client(this.login)
    ...
  }
}

Here is the error that I'm getting:

VError: a BeforeAll hook errored, process exiting: tests\functional\features\support\hooks.ts: Cannot read property 'LogIn' of undefined
   at Runtime.runTestRunHooks (c:\project\code\myapi\node_modules\@cucumber\cucumber\src\runtime\run_test_run_hooks.ts:32:19)
   at processTicksAndRejections (internal/process/task_queues.js:95:5)
   at async Runtime.start (c:\project\code\myapi\node_modules\@cucumber\cucumber\src\runtime\index.ts:103:5)
   at async runCucumber (c:\project\code\myapi\node_modules\@cucumber\cucumber\src\api\run_cucumber.ts:125:19)
   at async Cli.run (c:\project\code\myapi\node_modules\@cucumber\cucumber\src\cli\index.ts:78:25)
   at async Object.run [as default] (c:\project\code\myapi\node_modules\@cucumber\cucumber\src\cli\run.ts:38:14)
caused by: TypeError: Cannot read property 'LogIn' of undefined
   at Object.BeforeAll (c:\project\code\myapi\test\functional\features\support\hooks.ts:17:22)
   at globalBindFunc (c:\project\code\myapi\node_modules\cucumber-tsflow\src\binding-decorator.ts:256:48)
   at Object.run (c:\project\code\myapi\node_modules\@cucumber\cucumber\src\user_code_runner.ts:37:21)
   at Runtime.runTestRunHooks (c:\project\code\myapi\node_modules\@cucumber\cucumber\src\runtime\run_test_run_hooks.ts:21:50)
   at Runtime.start (c:\project\code\myapi\node_modules\@cucumber\cucumber\src\runtime\index.ts:103:16)
   at runCucumber (c:\project\code\myapi\node_modules\@cucumber\cucumber\src\api\run_cucumber.ts:125:33)
   at async Cli.run (c:\project\code\myapi\node_modules\@cucumber\cucumber\src\cli\index.ts:78:25)
   at async Object.run [as default] (c:\project\code\myapi\node_modules\@cucumber\cucumber\src\cli\run.ts:38:14)

Any thoughts on that?

Note: A minor issue from the code, if you check the comments added for these BeforeAll and AfterAll, they are the same for the Before and After ones.

Thank you...

Fryuni commented 1 year ago

BeforeAll and AfterAll execute on static methods, there is no context for them.

The contexts are created for each scenario, since those hooks run outside of any scenario there is nothing to inject in them.

If you need a value to be shared globally you can create a constant in a file and refer to it from everywhere. It will be the same for all scenarios. That is normally a bad idea since state of one scenario will bleed into the following scenarios.

ericrommel commented 1 year ago

How can I handle this? It's a logging in process and I don't want to log in every scenario. There are more than 2000 scenarios (and growing). A simple login for all of them could be enough

Fryuni commented 1 year ago

I opened a separate issue with your question since this issue is for the direction of the library. Answer you there.

aozolin commented 7 months ago

Hi there,

I really love the project as it makes normal cucumber stepdefinitions more object-oriented. It's great we can define them as a class and its functions - provides more flexibility to export/inherit/extend.

The context sharing is also great as allows to transfer the data between steps.

However, the current implementation misses at least one very critical for me feature.

I use the integration with allure-reporter, and we actively use allure APIs which are mapped to Cucumber's World object - hence are not available in cucumber-tsflow, such as: this.attach() this.label() this.severity() etc.

I was able to find a way how to use this.attach via CucumberAttachments, but cannot find a way to use others. It will be great you can add such feature or help me to use it if supported.

Second great feature to be added - BeforeStep/AfterStep hooks. It's not so critical as above, but still nice to have.

Thanks in advance.

Fryuni commented 7 months ago

Great ideas @aozolin, could you open them as new issues? One for each, this way we can decide on the API for them and track progress separately

cjmarkham commented 6 months ago

We've also found the need for beforeStep and afterStep hooks in our projects. To that end, I've created a PR to add both of those hooks.

There's no attached issue but I can create one if you want for tracking purposes.