WIP: Update Gherkin and add support for the Rule keyword

[badeball]

Hello all, and thanks for your work so far

I've been longing for support for Rule for some time now and due to some recent changes in the Cucumber repositories, I believe this library may soon also support it. For reference, let me recap what's happened.

• The native parser has been restored and converted to typescript
• Cucumber.js' dependency on Gherkin has been updated
• Support for Rule in Cucumber.js is underway

This patch updates the dependency on Gherkin, which introduces a new interface (that is asynchronous) and modifies it to fit the new and refined pickle structure. The most controversial aspect is perhaps 4e6bf32, which removes tests I consider to redundant, and is further explained in the commit message.

This patch depends upon unrelased code in Cucumber.js, hence - in order to be able to work with it - I've added cucumber-js as a submodule. In order to do npm install you will first have to build this version of Cucumber.js, as shown below.

$ git clone https://github.com/badeball/cypress-cucumber-preprocessor
$ git checkout update-gherkin
$ git submodule init
$ git submodule update
$ cd vendor/cucumber
$ yarn
$ yarn build-local

Obviously, this isn't really viable when publishing something on npm, hence this is blocked by cucumber/cucumber-js#1273 and their release schedule.

Meanwhile, I propose that we restructure entirely how this preprocessor is tested and align it more with Cucumber.js' own tests. Currently, I see the following problem.

• Writing tests is icky, to say the least, they share state and are awkward. This is the last thing we want when striving for SelfTestingCode.
• We're not able to properly test the different configuration options (like those suggested in feat(getstepdefinitionspaths): configurable nonGlobalStepDefinitions base directory #236 and fix(getstepdefinitionspaths): custom commonPath #275), as all tests run under a single configuration.
• We're re-formulating behavior that the people behind Cucumber has already done, eg. for scenario outlines and tag behavior.

I believe that by writing tests that themselves invoke Cypress would solve all of the above-mentioned issues. For all Cucumber-behavior we're re-implementing, we could almost just copy their tests.

What do you think about this sentiment?

[lgandecki]

Hello @badeball , thank you so much for your work on cucumber and on this plugin.

I'll briefly review the code but will put a little more effort when everything is ready on the cucumber side - as I imagine there is still a chance for the API on their side to slightly change.
Nonetheless - it's great to have this work in place as a significant headstart.

To discuss a bit the test situation.
I'm not sentimental about this code and I'd definitely encourage a significant rewrite of our test-suite if it could improve the dev experience and the "safety" for automated releases.

You are right that the pseudo-jests/jasmine/mocha/cucumber tests are redundant. The reasoning behind them was to be able to iterate over the code quickly. Currently the cypress run commands takes ~120 seconds and that's on a maxed out 6 core 2019 macbook pro. Having jest with wallabyjs gave me test results in tens of miliseconds, with amount of code shifting and rewrites that we've had to do initially this was crucial.
But - I think you are right. Currently they are an unnecessary complication of the codebase. And I could test particular feature files very quickly using the cypress GUI tool, while having the .features ability decreases the amount of time to test everything from the mentioned 120 to ~16 seconds (not fast enough to run on every code change and drive the development, but definitely fast enough to run it every now and then).

I like the idea of having a parity with cucumber .feature files, while just switching the implementation to cypress-compatible! Some of our tests are real random nonsense :) I loved the creativity (and I started the bad trend myself, while it was just a PoC for one of my own projects..), but the features could really use cleaning up.

As per these two points:

Writing tests is icky, to say the least, they share state and are awkward.

Is this fixed by removal of those fake-jest tests? Or do you mean something more? Some of the .feature files invoked with cypress do share the state (if I remember correctly, the ones around tags), but I'm not sure how to make it different - definitely open for suggestions.

We're not able to properly test the different configuration options (like those suggested in #236 and #275), as all tests run under a single configuration.

Do you have ideas how to improve that? If we run things through cypress I think we still need to run things under a single configuration, and that's a problematic part. Unless we have a few different runs of cypress? But they are so slow :(

[badeball]

Is this fixed by removal of those fake-jest tests? Or do you mean something more?

No, I was primarily referring to the global style step definitions. If one is eg. creating hooks or modifying the World object (ie. this), one can't really be sure what other tests are affected. Admittedly, it's a small point. The ability to test configuration options are more important in my opinion.

Do you have ideas how to improve that? If we run things through cypress I think we still need to run things under a single configuration, and that's a problematic part. Unless we have a few different runs of cypress? But they are so slow :(

I imagine that we use eg. Cucumber.js to write tests and actually have the tests write tests, as seen eg. here. Now, I realize that this will probably come at a cost in execution time - not writing files in particular, but invoking Cypress N number of times - but I believe this is the only way to truly create acceptance tests. Without this, one will never really be confident that nothing's broken.

Edit: To reiterate on my last point - I imagine some tests will write a custom cypress.json in order to test non-default configuration values.

[badeball]

Do you have ideas how to improve that? If we run things through cypress I think we still need to run things under a single configuration, and that's a problematic part. Unless we have a few different runs of cypress? But they are so slow :(

I've been working on this in a branch called update-gherkin-2nd-attempt. I'm actually doing a whole lot in this branch, but 163d1dc is of particular interest in this regard.

In addition to that, what I've done in that branch most notably is to

• Introduce the use of generated pickles to a larger degree. This has the benefit of removing a lot of duplicated behavior related to example scenario replacement and tag behavior (an incidentally fixes [feature request] use tags for example tables for scenarios with more example tables #196 and Feature tags don't affect Hooks with tag expressions #237).
• I'm building a "browser runtime" to be loaded, which allows us to move Cucumber to dev dependencies.
• Then I'm building the browser runtime form Cucumber's source. At this point the test actually pass (ish).
• Lastly I'm updating the reference in Cucumber to latest, where I unfortunately encounter an issue with circular references in protobuf.js.

And that is where I'm currently stuck. It appears that this is on hold until Cucumber's next release, which will contain their upgrade on the Gherkin dependency.

The branch does however illustrate how I imagine we can test the library and all different configuration options, by writing tests which essentially writes a test, which is then run. I know this creates a large increase in execute time, but again - I believe this is the only way to truly create acceptance tests to obtain 100% confidence.

[lgandecki]

I like it :-)
Thanks for the work!

I agree we don't need to worry that much about the test execution time. As long as we have a reasonably good local Developer Experience (by running just a subset of the tests, but fast). It's not like we have 10 devs full-time working on this project every day to worry that much about feedback loop and integrating the code together multiple times a day.
The more we can rely on tests in this repo the easier it will be to just merge PRs and release new versions automatically.

[lgandecki]

hey @badeball , are you not interested in working on this anymore? :(

[badeball]

Hey again, @lgandecki

I know this might not be what you want to hear as a library maintainer and it pains me a great deal to say this because I don't want to contribute to unfavorable fragmentation, but I've decided to fork and use my own implementation for now.

While we have been waiting for Cucumber to release v7 (they released rc.0 some time ago now actually), another issue related to JSON reporting has arose. This issue is unfortunately a complete dealbreaker for us.

I don't have it in me to write the specification for JSON reporting and maintain it, particularly because I know that it's going to be deprecated and removed. I fear any further work on this will be in vain. I also don't see any need for the feature personally.

Moreover, I find it painful to not use TypeScript for this kind of thing. With a reduced feature-set to support, I simply found it easier (and more interesting) to write my implementation.

I don't think there's much work left here though. My 1st attempt at this will probably serve as a good base. Instead of c995d18, you can now just install the latest @cucumber/cucumber. In my 2nd attempt, I tried to do more, such as using pickles to a larger degree (e010dea), removing global state (bf9ef17) for reasons I can't remember and utilizing Cucumber itself to do integration testing (163d1dc), last which i highly recommend you do.

(The PR got closed when I accidentally deleted the source branch, btw)

[lgandecki]

Thanks for the thoughtful answer.
I understand, it does make me a bit sad to "see you go", as you were a good informal member of the team :-)

There might be a way to join the efforts though. I personally do not use this project a lot (our clients do though, so I'm not completely disconnected), so I never was able to find the time to properly reorganize it, but it seems you just did! I do like the direction you've chosen. Maybe we can merge our projects as kind of a version 3, deprecating the json reporter as well, especially if cucumber is doing that. (the current version should keep working for people that do need the reporter)
We've never used it. It was added to the project as a PR, and even though I am extremely grateful for that work, it did make the code much more complex, causing quite a few bugs going forward, and a lot of feature requests related to it. It was really hard for me to justify working on it.

One thing that I'm curious about, why did you decide not to use the "all.features"? All the other differences you mention seem completely reasonable to me. I'm also a huge fan of typescript btw and would prefer to work in a typed codebase :)

[badeball]

One thing that I'm curious about, why did you decide not to use the "all.features"?

I just think that if Cypress is too slow to load files, which I guess was the motivation behind the feature, then that is something we should bring up with Cypress*. Furthermore, it relies on implementation details (#304). Perhaps most importantly, I run my specs in parallel in CI, across five nodes, so the feature don't matter that much to me.

* Or perhaps investigate ourselves. I'm confident that it can be optimized somehow, as my impression is that we're compiling and bundling almost the same files for every feature-file, because node_modules is likely the lion's share in it.

[lgandecki]

Good points. There are definitely areas for optimizations in terms of what we compile (and I think more importantly but serve to the browser). There are some low-hanging fruits that I'm aware of. I thought that even pure cypress test files, if you do tiny tests per file, would be slow like that, but I need to retest that. I'm sure they've been improving the start times as well. The startup time was a blessing for our test times on CI, but those do not load actual pages, so I'm not sure how much of that speed up translates into real life.

As for the way forward, I'm leaning more and more towards using your approach. I think I agree it was a mistake to allow some of the extra features in. It wasn't realistic to maintain them all properly, even with all the help from volunteers! I've been dreaming for a lighter and better structure codebase to maintain, while still providing peoplpe ability to combine Cucumber and Cypress :)
We need to make sure we are still able to release fixes to the "legacy" version. I don't want to leave the people that will not want to switch (or decide to wait with the switch) alone, although I will try to make clear that the support is limited to (major) bug fixes..
I'd also like to revisit some of the defaults. I saw that you simplified the configuration but need to look deeper into what that exactly means. It looks like most people did like the idea of nonGlobalStepDeps - even though it's a bit unorthodox. But if we were to advocate that it should be set by default. (which I think you did if I understood the code correctly)
I would definitely prefer to have minimal or zero configuration for most cases, with some smart defaults.

Our company is working on a project with a hard deadline for the release of the new generation consoles, which happens mid-November. At that point, I can spend a few days reorganizing the repo, setting up the CI/CD infrastructure for two lines, and move the documentation to an actual static website.
Meanwhile I'm happy to discuss and plan but will most probably not have enough time to do any significant amount of coding.