Skip to content
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

Adding Ruby 3.1 to CI #1340

Closed
petergoldstein opened this issue Jan 9, 2022 · 16 comments
Closed

Adding Ruby 3.1 to CI #1340

petergoldstein opened this issue Jan 9, 2022 · 16 comments

Comments

@petergoldstein
Copy link
Contributor

petergoldstein commented Jan 9, 2022

Ruby 3.1 is officially out, and it needs representation in CI. There are several issues that need to be addressed before this will run, and I'm working through them now. To date I've identified the following issues:

  1. Rubocop needs to be updated to a Ruby 3.1 compatible version that is also Ruby 2.4 compatible ("~> 1.0", "< 1.12")
  2. A .rubocop_todo.yml file with the new Rubocop lints needs to be added to the repo
  3. For Ruby 3.1 (and for Ruby head) we need to set the RAILS_VERSION environment variable to a version that is compatible with these Rubies. Because of the configuration here rspec-rails will use Rails "~> 6.0.0" as a default. This default is incompatible with Ruby 3.1+, so we need to explicitly set the RAILS_VERSION environment value to a compatible value (i.e. "~> 7.0.0").
  4. It looks like rspec-rails needs to be updated to remove or guard a reference to ActionMailer::DeliveryJob which was removed in Rails 7.0.0. I don't have a complete grasp on this code yet, so I'm not yet able to generate a PR here.

I've got a Ruby 3.1 build running through to item #4 above.

To get ruby-head fixed, there are additional issues I've identified:

  1. There is an undef in spec/rspec/matchers/built_in/contain_exactly_spec.rb that needs to be guarded with a respond_to?
  2. The gherkin - version 2.12.2 - library that's in use in the build via cucumber has an unguarded reference to Fixnum. As Fixnum has been removed from Ruby head, this is causing an error. It's worth noting that this appears to be a very old version of gherkin, pulled in by a very old version of cucumber, dictated by the version dependency in the rspec-expectations.gemspec. I'm going to try updating this.

Expected behavior

CI should pass with Ruby 3.1

Actual behavior

CI fails with Ruby 3.1

@pirj
Copy link
Member

pirj commented Jan 9, 2022

Rubocop needs to be updated to a Ruby 3.1 compatible version

It's a big topic, it also affects Mocks, Support, and Core as we have a shared base RuboCop config. Updating RuboCop for just Expectations would break this cohesion. Problem is, RuboCop has made a big leap, and some cops were renamed. Making this shared config compatible with RuboCop < 1.0 and > 1.0 is unlikely possible.

As a short-term solution, we can turn off RuboCop for 3.1 in the same way we skip it for Rubies < 2.4.

I'd rather handle RuboCop update after RSpec 4 is out and when we merge -core, -support, -expectations and -mocks into a single git repo.

rspec-rails will use Rails "> 6.0.0" as a default
Ruby 3.1 (and for Ruby head) we need to set the RAILS_VERSION environment variable to a version that is compatible with these Rubies
This default is incompatible with Ruby 3.1+, so we need to explicitly set the RAILS_VERSION environment value to a compatible value (i.e. "
> 7.0.0").

Why? rails 6.0.0 seems to be compatible with Ruby 3.1.
Can you send a PR that errors out CI's rspec-rails build step?

Another difficulty is that, as you mention later, we don't fully support Rails 7.0 yet, and we just can't do that.

It looks like rspec-rails needs to be updated to remove or guard a reference to ActionMailer::DeliveryJob

This should be fixed by a PR that involves fixes from several contributors.

To get ruby-head fixed

In general, I'd suggest postponing this, but still keep in mind.

The gherkin - version 2.12.2 - library that's in use in the build via cucumber has an unguarded reference to Fixnum.

We've updated cucumber in RSpec 4, and since it's only used in our test suite. Maybe it's worth backporting to the main branch.

There is an undef in spec/rspec/matchers/built_in/contain_exactly_spec.rb that needs to be guarded with a respond_to?

👍

@petergoldstein
Copy link
Contributor Author

It's a big topic, it also affects Mocks, Support, and Core as we have a shared base RuboCop config. Updating RuboCop for just Expectations would break this cohesion. Problem is, RuboCop has made a big leap, and some cops were renamed. Making this shared config compatible with RuboCop < 1.0 and > 1.0 is unlikely possible.

I saw that. It's a relatively easy upgrade (most of it is autorun) assuming each repo is willing to have its own .rubocop_todo.yml. Choosing a Rubocop version of 1.11 allows Ruby compatibility from 2.4 to 3.1. I'm happy to submit PRs that (using a common .rubocop_rspec_base.yml) upgrade all 4 repos. It's then pretty straightforward to work through the todo files.

As a short-term solution, we can turn off RuboCop for 3.1 in the same way we skip it for Rubies < 2.4.

Sure, that's an option too.

rspec-rails will use Rails "> 6.0.0" as a default
Ruby 3.1 (and for Ruby head) we need to set the RAILS_VERSION environment variable to a version that is compatible with these Rubies
This default is incompatible with Ruby 3.1+, so we need to explicitly set the RAILS_VERSION environment value to a compatible value (i.e. "
> 7.0.0").

Why? rails 6.0.0 seems to be compatible with Ruby 3.1. Can you send a PR that errors out CI's rspec-rails build step?

My understanding is that Rails 6.1.x supports Ruby 3.1, but Rails 6.0.x does not. That's what all the support matrices I'm seeing say.

We could do Rails 6.1.x, but that brings in the net-smtp, net-imap, net-pop Gemfile issue which is not fixed in Rails 6.1.x, but is fixed as of Rails 7.0.1. That would require changes to rspec-rails as well to get that working.

Another difficulty is that, as you mention later, we don't fully support Rails 7.0 yet, and we just can't do that.

Yep, I'd like to fix Rails 7 support, as I'm planning to use Ruby 3.1/Rails 7 for my next project. That's my motivation here.

I'm trying Rails 7 against rspec-rails and there are 8 failing spec and 3 acceptance tests.

It looks like rspec-rails needs to be updated to remove or guard a reference to ActionMailer::DeliveryJob

This should be fixed by a PR that involves fixes from several contributors.

Great, I didn't see that. I'll pull that in locally and replace my version. The PR seems to fix the argument handling, which I hadn't gotten to.

To get ruby-head fixed

In general, I'd suggest postponing this, but still keep in mind.

The gherkin - version 2.12.2 - library that's in use in the build via cucumber has an unguarded reference to Fixnum.

We've updated cucumber in RSpec 4, and since it's only used in our test suite. Maybe it's worth backporting to the main branch.

There is an undef in spec/rspec/matchers/built_in/contain_exactly_spec.rb that needs to be guarded with a respond_to?

👍

I've got ruby-head passing with some minor cucumber configuration changes. And with the undef fixed, ruby-head now passes.

My branch with the test changes is here - #1341
Oddly, I'm now seeing those failures on the older Rubies for Windows because it can't find ansicon on rspec/rspec-expectations again. I wasn't seeing those last night - https://github.com/petergoldstein/rspec-expectations/actions/runs/1673088527

@pirj
Copy link
Member

pirj commented Jan 9, 2022

We could do Rails 6.1.x, but that brings in the net-smtp, net-imap, net-pop Gemfile issue which is not fixed in Rails 6.1.x, but is fixed as of Rails 7.0.1.

The easiest would be to add here:

if RUBY_VERSION.to_f >= 3.1
  gem 'net-smtp'
  ...

with a note on how exactly this was fixed in 7.0.1, and that it's a mail gem issue with a link.

@petergoldstein
Copy link
Contributor Author

We could do Rails 6.1.x, but that brings in the net-smtp, net-imap, net-pop Gemfile issue which is not fixed in Rails 6.1.x, but is fixed as of Rails 7.0.1.

The easiest would be to add here:

if RUBY_VERSION.to_f >= 3.1
  gem 'net-smtp'
  ...

with a note on how exactly this was fixed in 7.0.1, and that it's a mail gem issue with a link.

I've got that queued up and am running locally now. Will put up a PR if it runs green locally on Rails 6.1.x

@pirj
Copy link
Member

pirj commented Jan 9, 2022

Ideally, on 6.0 so we don't have to set a different RAILS_VERSION.

@petergoldstein
Copy link
Contributor Author

6.1.x. To my knowledge 6.0.x isn't Ruby 3.1 compatible as it fell out of fully supported status with the release of Rails 7.0

@pirj
Copy link
Member

pirj commented Jan 9, 2022

👍
The default version of Rails used in a dependent rspec-rails build is defined in Gemfile-rails-dependencies:

-  gem "rails", "~> 6.0.0"
+  gem "rails", "~> 6.1.0"

I'm all for changing this. Appreciate if you could cite the exact failure of Rails 6.0 with Ruby 3.1 (after you deal with net-* gems).

It would be great to could send those changes as separate PRs.

@petergoldstein
Copy link
Contributor Author

@pirj It's not that there is some failure that I've detected and am aware of. There is a set of Ruby versions that the Rails team says should work with each version of Rails. The set for Rails 6.0.x doesn't include Ruby 3.1. Whether the specs pass or fail in that case is largely irrelevant - it's unsupported behavior.

It's worth noting that changing that dependency has a similar effect. Ruby 2.5 is not supported for Rails 6.1.x. So that would need to be flagged with a RAILS_VERSION environment variable.

Still working on this.

@petergoldstein
Copy link
Contributor Author

Here are 4 Rubocop update PRs that use a common .rubocop_rspec_base.yml file:

rspec/rspec-core#2927
#1342
rspec/rspec-mocks#1448
rspec/rspec-support#525

Rubocop now runs green on all four repos with these PRs, and the Rubocop version should be compatible with Ruby 2.4-3.1.

@petergoldstein
Copy link
Contributor Author

BTW, the cucumber changes to get ruby-head working are simple and I can easily extract into a dedicated PR if desired. I don't think we can upgrade to Cucumber 4 on this branch without dropping support for older Rubies, so the set of changes I've got in place can unblock this branch (and be forward compatible with the RSpec 4.0 changes)

@JonRowe
Copy link
Member

JonRowe commented Jan 11, 2022

I'm closing this because we now have Ruby 3.1 in CI, thanks for the reminder (I'd started this over the christmas break but then got distracted by christmas break things 😂 )

@JonRowe JonRowe closed this as completed Jan 11, 2022
@JonRowe
Copy link
Member

JonRowe commented Jan 11, 2022

I'm interested in the cucumber changes if it helps for ruby-head for main, but I think in 4.0.x we'll be upgrading quite significantly right @prij?

@pirj
Copy link
Member

pirj commented Jan 11, 2022

Yes, we've bumped cucumber to 7.0 in 4-0-dev

Should I backport this to main?

@petergoldstein
Copy link
Contributor Author

@pirj @JonRowe To bump up cucumber up to 7.0 you'll need to drop several legacy rubies from main. The changes I put in allow you to bump up Rubies that support it (in some cases to cucumber 9.x) while maintaining the legacy Rubies. The only real difference is syntax differences in tags.

@pirj
Copy link
Member

pirj commented Jan 11, 2022

Right, so we have it as:

s.add_development_dependency 'cucumber', '>= 3.2', '!= 4.0.0', '< 8.0.0'

cucumber 4.0.0 would run on Ruby 2.3, 2.4 and JRuby 9.1

I believe I meant 3.2, not 4.0.0

Your trick with conditionally setting wip tag is completely fine, too. I'm wondering why cucumber 4.0.0 is not getting installed and doesn't blow up.

Is skip_this_scenario supported in cucumber 1.3, too?

@petergoldstein
Copy link
Contributor Author

skip_this_scenario appears to be supported - it was doing the right thing on the specs I was examining (green, printing message, etc.). I was having trouble finding documentation for that version of cucumber, so I simply tried it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants