cucumber / cucumber-rails

Rails Generators for Cucumber with special support for Capybara and DatabaseCleaner
https://github.com/cucumber/cucumber-rails
MIT License
1.02k stars 327 forks source link

Upgrade cucumber dependency to work with cucumber 6 #515

Closed aurelien-reeves closed 3 years ago

luke-hill commented 3 years ago

This needs 2 fixes.

aurelien-reeves commented 3 years ago

This needs 2 fixes.

* Amend one of the steps to mitigate the error

* Edit the gemfiles (6.0 gemfile should require `cucumber < 6`)

Thanks a lot @luke-hill !

I was pretty confused here 😅

I still don't get "Amend one of the steps to mitigate the error" for now. I wait for the CI to pass before taking a deeper look.

luke-hill commented 3 years ago

Somewhere in one of the tests we write a cuke "the client requests GET /{word}"

this now needs amending a tiny amount to

"the client requests GET \/{word}"

the client requests GET /{word}
                        ^
Alternative may not be empty.
If you did not mean to use an alternative you can use '\/' to escape the the '/' (Cucumber::CucumberExpressions::AlternativeMayNotBeEmpty)
aurelien-reeves commented 3 years ago

Somewhere in one of the tests we write a cuke "the client requests GET /{word}"

this now needs amending a tiny amount to

"the client requests GET \/{word}"

the client requests GET /{word}
                        ^
Alternative may not be empty.
If you did not mean to use an alternative you can use '\/' to escape the the '/' (Cucumber::CucumberExpressions::AlternativeMayNotBeEmpty)

Yeap, did it right before your comment, thanks a lot :)

Regarding Rails 6.0, you would not to support cucumber 6 with rails 6.0 on purpose? Or did you notice something that would prevent us to do so?

luke-hill commented 3 years ago

Semi on purpose.

A while back we started to try trim down our support matrix. And we came up with a semi-ok set of rules.

https://github.com/cucumber/cucumber-rails/blob/main/.github/workflows/build.yml#L13-L18

Obviously over time this has then extended to other gems/versions. but basically if it's not rails 5.2 or the latest 6.x we don't support it with the latest gems. To try and kick people to one of those versions. So we then started staggering capybara and cucumber versions accordingly.

aslakhellesoy commented 3 years ago

@luke-hill @aurelien-reeves I'd suggest removing the parts of that comment that are inconsistent with the versions specified below.

aurelien-reeves commented 3 years ago

Ok, thanks.

I would suggest to let rails 5.2 with cucumber < 6, but support rails 6.0 with cucumber < 7 like rails 6.1 if it works well

I noticed also that it seems we are not yet compatible with ruby 3. I will open a PR for that soon I guess ;)

aurelien-reeves commented 3 years ago

@luke-hill @aurelien-reeves I'd suggest removing the parts of that comment that are inconsistent with the versions specified below.

Did you spot something inconsistent in it?

aslakhellesoy commented 3 years ago

This: 2.4 -> Only use legacy rails versions (5.0/5.1)

Versus this: - { ruby: '2.4', gemfile: 'rails_5_2' }

Same for 2.5 and 2.6

Comments always get out of date. Better to remove them.

aurelien-reeves commented 3 years ago

That looks good to me.

With ruby 2.4 we want to check only rails 5.0 and rails 5.1, so we exclude rails 5.2, rails 6.0 and rails 6.1 from ruby 2.4 in the matrix.

The same for other versions of ruby

aslakhellesoy commented 3 years ago

Oh I see now. I missed the exclude above 👍🏻