-
-
Notifications
You must be signed in to change notification settings - Fork 147
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
WIP: Update Gherkin and add support for the Rule keyword #322
Conversation
This is in preperation of upgrading to most recent version Gherkin, which only offers an async API.
This is contrary to doing it in Cypress' runtime. This is in preperation of upgrading to most recent version Gherkin, which only offers an async API.
I believe this used to run scenarios within Jest. Furthermore, I believe this to be not only completely redundant, but also wrong. Firstly, Cypress is the only interface for a user to consume this library. No other test needs to pass than those in ./cypress. Secondly, this happens to work because Jasmine aligns somewhat well with Mocha, but this isn't guaranteed. Lastly, but perhaps most importantly, this is in preperation of upgrading to most recent version Gherkin, which only offers an async API. Jasmine doesn't support deferred invocations of describe() & it() like Mocha does.
This is in preperation of upgrading to most recent version Gherkin, which only offers an async API.
This version of Gherkin outputs pickles for the JSON formatter to consume, hence we update Cucumber as well. Cucumber's own update on Gherkin is however unreleased and only resides in their master branch. Since they do not commit the build to source code, we have to be somewhat creative here. For now, one must clone the repository recursively, install Cucumber's dependencies and build it.
This fixes #143.
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. To discuss a bit the test situation. 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. 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:
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.
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 :( |
No, I was primarily referring to the global style step definitions. If one is eg. creating hooks or modifying the World object (ie.
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 |
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
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. |
I like it :-) 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. |
hey @badeball , are you not interested in working on this anymore? :( |
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 (The PR got closed when I accidentally deleted the source branch, btw) |
Thanks for the thoughtful answer. 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) 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 :) |
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. |
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 :) 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. |
ab500fd
to
e7c0804
Compare
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.Rule
in Cucumber.js is underwayThis 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.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.
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?