Cucumber 4 support

[deivid-rodriguez]

I need a bug fix that hopefully will be released with cucumber 4, but since I use cucumber through parallel_tests, I won't be able to use it until parallel_tests add support. So I'm looking into adding it.

Initially it seemed pretty hard because there's pretty low level gherkin parsing stuff involved in this gem, but then thanks to this comment, I found this gem that provides exactly the kind of API parallel_tests use.

I switched parallel_tests code to use this gem in this branch and it seems to work just fine. Maybe using this gem even adds back support for cucumber 2, which is currently broken... 🤷‍♂️.

Anyways, the problem with doing this, is that another dependency needs to be added, and since this gem has no specific framework dependencies, it will need to be manually done by end users. So... I'm not sure what's best, but I lean towards extracting cucumber support to a separate parallel_tests-cucumber gem that actually depends on cucumber and cuke_modeler.

Thoughts?

[grosser]

manual gems are not that scary, add a gem 'cuke_modeler', '~> a.b'

I'm not super worried about cucumber 2, whoever wants that can hopefully use an old version of paralllel_tests

making a separate gem is a lot of overhead and will get out of sync over time, so I'd prefer not to.

would recommend to use a branch of parallel_tests until cucumber 4 and after we switch it over

[deivid-rodriguez]

Ok! I'll use my branch until cucumber 4 is out, and then we'll see 👍.

[deivid-rodriguez]

Cucumber 4 was released.

I dusted off my old branch and it seems to still work correctly. Do you want to me to prepare a PR?

[grosser]

yes, definitely make a PR!
I'm a little hesitant to switch to v4 when it's so fresh, so if we can keep both running that would be nice ... if the overhead is too big though, we might have to bite the bullet and switch (with something like gem 'cucumber', '~> 4.0' to make it obvious)

[deivid-rodriguez]

Sure, I'll prepare the PR tomorrow!

As far as I can see, old cucumbers would still be supported. As a matter of fact, support for cucumber 2.x might actually be restored as a side effect.

The downside is the need for users to manually add the cuke_modeler gem in addition to cucumber. I guess I could keep cucumber 3.x support as it is and only require cuke_modeler for cucumber 4 or higher, although it's going to get messy.

[deivid-rodriguez]

I still believe the best way to approach this would be to extract support to a parallel_tests_cucumber gem that does depend on cucumber and cuke_modeler.

You said

making a separate gem is a lot of overhead and will get out of sync over time, so I'd prefer not to.

But if we keep the gem in this repo and make releases always in sync with parallel_tests, both issues would be gone I believe?

[deivid-rodriguez]

To me it feels wrong that end users need to add dependencies they're not using directly to their Gemfile to make this work.

[grosser]

end users that don't use cucumber should not have to add cucumber to their Gemfile but if they are using cucumber they have to have one of the supported versions installed (ideally it's not just 1)

…

On Sat, Jun 6, 2020 at 8:11 AM David Rodríguez ***@***.***> wrote: To me it feels wrong that end users need to add dependencies they're not using directly to their Gemfile to make this work. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#671 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAACYZ2S37SVJVWLGBXBWR3RVJMDJANCNFSM4GJGXCMA> .

[deivid-rodriguez]

Oh, I think you might have missed my initial comments in this thread. My implementation of cucumber 4 support uses an extra gem, cuke_modeler, which end users will most likely not be using.

[grosser]

don't add it to the gemspec, but as gem 'cuke_modeler' and it should work :)

[deivid-rodriguez]

Well, it will work in the sense that it will allow the users to upgrade the gem, and then fail at runtime and tell users that they need to add the cuke_modeler gem to their Gemfile. To me that's not the best user experience.

[grosser]

yeah, it's not great, but having everyone install extra gems is also bad :( I guess the only clean solution is parallel_tests_cucumber gem :(

…

On Sat, Jun 6, 2020 at 9:39 AM David Rodríguez ***@***.***> wrote: Well, it will work in the sense that it will allow the users to upgrade them gem, and then fail at runtime and tell users that they need to add the cuke_modeler gem to their Gemfile. To me that's not the best user experience. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#671 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAACYZ23AWSJWO27EXGL2SLRVJWL5ANCNFSM4GJGXCMA> .

[deivid-rodriguez]

That's what I'm suggesting.

[grosser]

Yeah, let's do it then ... you make the new gem and then we remove cucumber support here or how do you envision this ?

[deivid-rodriguez]

That sounds good to me. I would probably leave cucumber 3 support in this gem for a while until people transition.

[grosser]

alternatively: can we leave base cucumber support in here and have the extras as a gem ? (log parsing/ splitting etc fancyness ?)

[deivid-rodriguez]

I'll have a first look at what extracting the gem involves and get back 👍.

[deivid-rodriguez]

Ok, so I understand the situation better now. Only use cases that need cuke_modeler are the --group-by step and --group-by scenario flags. Given this is the case, I believe your initial suggestion of trying to activate the extra dependency at runtime and give a proper message if not there feels easier.

[grosser]

awesome! :D

…

On Tue, Jun 9, 2020 at 3:43 AM David Rodríguez ***@***.***> wrote: Ok, so I understand the situation better now. Only use cases that need cuke_modeler are the --group-by step and --group-by scenario flags. Given this is the case, I believe your initial suggestion of trying to activate the extra dependency at runtime and give a proper message if not there feels easier. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#671 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAACYZ6PDJ77M7MKPE4Z3HTRVYG5VANCNFSM4GJGXCMA> .

[deivid-rodriguez]

Great, working on it now. One more question, are you fine with testing only against cucumber 4 from now on, or should I add some machinery to test against different major versions of cucumber?

[grosser]

should be fine, we can look into dual-testing after the initial PR is done to make it simpler

…

On Tue, Jun 9, 2020 at 8:14 AM David Rodríguez ***@***.***> wrote: Great, working on it now. One more question, are you fine with testing only against cucumber 4 from now on, or should I add some machinery to test against different major versions of cucumber? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#671 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAACYZ3OIBDAFLNLUAZ2EHDRVZGWPANCNFSM4GJGXCMA> .