Add Ruby 3.2 / Rails 7 to the CI matrix

[petergoldstein]

This PR adds Ruby 3.2 and Rails 7 to the CI matrix. As part of these changes we also:

1. Added fail-fast: false to the CI configuration to enable easier debugging
2. Removed the lockfiles so that dependencies can be rebuilt using current gems
3. Updated the rspec less than 3.10 requirement to be ~> 3.9.0 as opposed to < 3.10.0 so that the rspec-rails requirement doesn't pull in a very old rspec
4. Removed the cache action and gem installation in the GitHub action configuration and instead used the bundler-cache functionality built into ruby/setup-ruby
5. Added a Rails 7 gemfile (only for RSpec >= 3.10)
6. Only load pry-byebug / pry-nav for Ruby versions < 3.2

I didn't update the Appraisals file, because appraisal is basically unsupported at this point, and nothing in the code base depends on appraisal other than Gemfile generation.

I'm also not sure why there are JAVA_OPTS in the GitHub Actions configuration given that specs aren't running jruby. So these may be able to be removed.

This runs green on my fork.

[mcmire]

Hey @petergoldstein, thanks for the PR. I had a few thoughts:

I didn't update the Appraisals file, because appraisal is basically unsupported at this point, and nothing in the code base depends on appraisal other than Gemfile generation.

I admit that there haven't been any recent changes to Appraisal, but it does work just fine, and I've been relying on it to run tests in various scenarios and thus to generate the gemfiles that represent those scenarios. So I'd rather not write these gemfiles by hand but rather rely upon the configuration in Appraisals to do that. In addition:

Removed the lockfiles so that dependencies can be rebuilt using current gems

Hmm, I'm not sure this is a good idea. Since the gemfiles generated by Appraisals are used in CI, if we remove the lockfiles, there is a chance that CI will use a separate lockfile than a developer would use locally. Therefore if CI fails and you wish to reproduce the failure locally, you may not be able to do this. Using lockfiles removes the indeterminism and makes builds reproducible.

I'm also not sure why there are JAVA_OPTS in the GitHub Actions configuration given that specs aren't running jruby. So these may be able to be removed.

When I first made this gem I initially wanted to run the tests on JRuby, but they took a very long time to run (tracked in #40), but I guess I didn't remove JAVA_OPTS, so you're right that that isn't necessary. Maybe it's best to remove that in a separate commit, however, so that it can be re-added easily in the future.

[petergoldstein]

@mcmire No problem. My focus was as a user - to determine whether super_diff could be safely used with Ruby 3.2 and Rails 7. Given that CI didn't have anything more recent than Ruby 3.0 and Rails 6, I needed to get it green before I would incorporate super_diff in my project.

Let me make clear that you're the maintainer here and thus these kinds of decisions are yours to make. That said, my thoughts:

I admit that there haven't been any recent changes to Appraisal, but it does work just fine, and I've been relying on it to run tests in various scenarios and thus to generate the gemfiles that represent those scenarios. So I'd rather not write these gemfiles by hand but rather rely upon the configuration in Appraisals to do that.

I've been seeing widespread failures with appraisal when attempting to get CI running with Ruby 3.2. Given that appraisal hasn't had functional CI for over a year and a half, green CI for over 2 years, and hasn't had a released version in over a year and a half I personally consider it abandonware.

That said, if you want me to update the Appraisals file to reflect these changes, I can do that.

Hmm, I'm not sure this is a good idea. Since the gemfiles generated by Appraisals are used in CI, if we remove the lockfiles, there is a chance that CI will use a separate lockfile than a developer would use locally. Therefore if CI fails and you wish to reproduce the failure locally, you may not be able to do this. Using lockfiles removes the indeterminism and makes builds reproducible.

For a stand-alone application I agree with this logic.

For a gem that is intended to be used in a large variety of applications, this simply isn't true. The purpose of CI isn't to ensure that the experience of a developer working on a gem is consistent and correct, it's to ensure that the experience of an end-user consumer of that gem is consistent and correct. And since the experience of an end user is going to be dependent on an ever evolving ecosystem of gems, you want to make sure that your library is working with that ecosystem pretty much at all times.

Failure to rebuild lockfiles puts you in a situation where your dependencies in a "real-life" situation may resolve in a surprising fashion. I found just that sort of situation when the lt rspec 3.1.0 gemfiles rebuilt - they pulled in an archaic version of rspec because of some dependency graph resolution.

But if you want to get the rebuilt and fully updated versions of these files as of 2023-01-26 and check them in, that should be straightforward. And shouldn't make any different to getting CI to green with modern Rubies/Rails

When I first made this gem I initially wanted to run the tests on JRuby, but they took a very long time to run (tracked in #40), but I guess I didn't remove JAVA_OPTS, so you're right that that isn't necessary. Maybe it's best to remove that in a separate commit, however, so that it can be re-added easily in the future.

Sure.

[petergoldstein]

One other item that may require attention - the Rails 6.0 gemfiles are actually loading Rails 6.1, because the Gemfile specification allows that. It's probably desirable to have a different set of Rails 6.1 gemfiles if that testing is required, because Rails 6 and 6.1 are pretty different.

See from this image:

[petergoldstein]

@mcmire Any feedback here? appraisal is actually getting some activity now, and as noted above I'm happy to fill out the Appraisals file.

Do you have a set of steps you'd like me to take to get this to mergeable? Also, it's probably desirable to get this merged - #177 - as well, so users of the gem get the proper signal about the CI status.

[mcmire]

@petergoldstein Sorry for the delay. I'm glad to hear that Appraisal will be getting some love! I also agree with you about removing the lockfiles out of the repo. I think I might have had some issues with dependencies in the past that led me to include them, but your argument makes sense. In any case yes please fill out the Appraisals file if you would :)

[mcmire]

One other item that may require attention - the Rails 6.0 gemfiles are actually loading Rails 6.1, because the Gemfile specification allows that. It's probably desirable to have a different set of Rails 6.1 gemfiles if that testing is required, because Rails 6 and 6.1 are pretty different.

It looks like someone else encountered this — there's #134. I realize I'm a bit behind on these PRs. I'll take a look at that now.

[petergoldstein]

@mcmire This branch is rebased, and has the following additional changes:

1. The Rails 6.0 Appraisals entry and the corresponding gemfiles was corrected to use ~> 6.0.0
2. The Rails 6.1 Appraisals entry and Gemfile from the referenced Test with Rails 6.1 #134 was pulled in
3. The Rails 7.0 entry was added to Appraisals

[mcmire]

@petergoldstein Thank you so much! This will help a lot.