Drop CI support for Ruby < 2.3 #259

pirj · 2020-11-29T10:41:59Z

Extracted from:

Repo PRs:

NOT DONE (mostly JRuby-related, saved for a subsequent iteration):

Remove continue-on-error: ${{ matrix.ruby == 'jruby-9.2.13.0' || endsWith(matrix.ruby, 'head') }}
add jruby-9.1, 9.2 in ci.yml (bumps 9.1.7.0 to 9.1.17.0)
remove jruby from travis
always use bundler 2 bundler: ${{ (matrix.ruby == 'jruby-9.1.17.0' && 1) || 2 }}
# For some reason JRuby doesn't like our improved bundler setup
yes | gem update --system --no-document - JRuby fails to install docs sometimes
yes | gem install bundler --no-document
turn on for all rubies? function documentation_enforced {/if is_mri; then

pirj · 2020-11-29T12:50:18Z

@JonRowe @benoittgt Please take a look.
I've also removed appveyor.yml along with all its traces in pull requests.

ci/script/clone_all_rspec_repos

JonRowe · 2020-12-02T08:31:12Z

ci/.travis.yml

 matrix:
  include:
-    - rvm: jruby-1.7
-      env: JRUBY_OPTS='--dev --1.8'
    - rvm: 2.7.1


You can remove the diff lcs version builds too, and set the minimum diff lcs version.

Should I use ">= 1.4.3", "< 2.0"?
I'll make those changes separately.

Can you just add it as a commit to the prs.

Sure, but which one? 😄 I have those four generated from this PR, also those ones from which I've extracted these changes.

I plan to send a few more series:

update rake to 13, and cucumber to 2.4 (broke my teeth trying to run 1.3 locally) <--- will add diff-lcs update here

rethink define_optimized_require_for_rspec

fix kwargs delegation

update JRuby to 9.1.17.0/ add JRuby 9.2

get rid of most of the <<-/chomp/unindent/undent/gsub in favour of <<~

Do you have some suggestions for this plan?

update rake to 13, and cucumber to 2.4 (broke my teeth trying to run 1.3 locally) <--- will add diff-lcs update here

Please just add diff-lcs to these PRs, any gem which mentions it in the gemspec just set the minimum version and remove the diff lcs builds.

*rethink define_optimized_require_for_rspec

I want to keep this, it is a lot of churn just to remove a generated method.

fix kwargs delegation

We may find this is impossible due to the hash behaviour, I suspect we will need to keep ruby 2 keywords for quite some time.

get rid of most of the <<-/chomp/unindent/undent/gsub in favour of <<~

That can wait until later prs, its subjective please don't combine it into your mega PRs they are already too hard to review.

update JRuby to 9.1.17.0/ add JRuby 9.2

Do this seperatly as I'm also working on this for main

I plan to send a few more series

I'd like to emphasize and elaborate on the above statement ☝️ to remove all the ambiguity.
I plan to send series (dev+support+core+mocks+expectations) of PRs for each of those changes separately.
After this current one is merged.
All against 4-0-dev.

define_optimized_require_for_rspec

I want to keep this

👍

Please just add diff-lcs to these PRs, any gem which mentions it in the gemspec just set the minimum version and remove the diff lcs builds.

Will do in a follow-up 👍

fix kwargs delegation

this is impossible due to the hash behaviour, I suspect we will need to keep ruby 2 keywords for quite some time.

That's is the plan, to use ruby2_keywords, there's no other way around. I intend to use this comment as the guideline.

ci/.travis.yml

JonRowe · 2020-12-02T08:33:00Z

ci/script/functions.sh

@@ -9,14 +9,20 @@ SPECS_HAVE_RUN_FILE=specs.out
 MAINTENANCE_BRANCH=`cat maintenance-branch`

 # Don't allow rubygems to pollute what's loaded. Also, things boot faster


Can we revert all these changes for now, it makes diffing harder and its easy to clean up post release.

With an exception of BRANCH_TO_CLONE trick, we can. But we equally can just set s.required_ruby_version = '>= 2.3.0' and clean everything else after the release 😆

Jokes aside, rspec-mock has undertaken the smallest change, just +360 −880 of all repos. Not much compared to +45 -145 induced by this update.

All of the changes to ci/script/functions.sh and ci/script/predicate_functions.sh consist of removals and simple changes a-la is_mri_2plus -> is_mri. That can be skimmed through really quickly during the review (of the total 4-0-dev -> main merge I suppose).

Except for the BRANCH_TO_CLONE trick that is in rspec-* repos' 4-0-dev, but not on main, and not here, in rspec-dev anywhere. I've added it to this PR to avoid it being overwritten with rake ci:update_files next time we update 4-0-dev.

JonRowe · 2020-12-02T08:33:05Z

ci/script/predicate_functions.sh

@@ -1,67 +1,21 @@
 function is_mri {
-  if ruby -e "exit(!defined?(RUBY_ENGINE) || RUBY_ENGINE == 'ruby')"; then


Can we revert all these changes for now, it makes diffing harder and its easy to clean up post release.

JonRowe · 2020-12-02T08:33:32Z

ci/script/update_rubygems_and_install_bundler

-  gem install bundler -v '1.17.3'
-fi
+yes | gem update --system
+yes | gem install bundler


Can we revert all these changes for now, it makes diffing harder and its easy to clean up post release.

JonRowe

Hi @pirj good work here, just some changes to drop more builds and a request to revert some changes for easy of diffing until after release

pirj · 2020-12-02T19:05:21Z

I would leave this PR hanging until we release 4.0.
However, in those four generated PRs I've also cleaned up Appveyor, and I don't feel too amused when thinking that I'll have to re-do this.
It would be wrong to merge those four PRs without merging this one that generated them.
Also, this work has been done. I understand that it's hard work to review the changes, sometimes even more time-consuming than "delete function" hasty removals.
There are also four other pull requests from which I've extracted these changes, waiting. And I'll have to rearrange the numerous commits there to revert the changes to those to sh files. It's not like I wouldn't have to rearrange commits there otherwise, but it feels like some additional work to do.

To sum it up, it feels like a lot of extra work upfront, and also re-doing of the same removals I've done here at a later point in time. I don't feel too enthusiastic about doing that that compared to the overhead of reviewing this.

See rspec/rspec-dev#259 (comment)

pirj

Some notes to the future self.

pirj · 2020-12-02T20:13:57Z

ci/.github/workflows/ci.yml

@@ -8,6 +8,8 @@ on:
  pull_request:
    branches:
      - '*'
+env:
+  RSPEC_CI: true


Forces rspec-rails to depend on 4.0.0.pre. See https://github.com/rspec/rspec-rails/blob/934276b0e12b21ad44bac54494c42f5a93e08a33/rspec-rails.gemspec#L48

pirj · 2020-12-02T20:14:23Z

Rakefile

@@ -383,7 +383,8 @@ namespace :ci do

  desc "Update build files"
  task :update_files do
-    update_ci_files_in_repos
+    opts = { except: %w[ rspec-rails ] }
+    update_ci_files_in_repos(opts)


Do not send CI file updates to rspec-rails.

pirj · 2020-12-02T20:15:50Z

ci/script/clone_all_rspec_repos

@@ -9,11 +9,8 @@ if is_mri; then
  clone_repo "rspec-core"
  clone_repo "rspec-expectations"
  clone_repo "rspec-mocks"
-  clone_repo "rspec-rails"
-
-  if rspec_support_compatible; then


All builds are rspec-support compatible.

pirj · 2020-12-02T20:16:57Z

ci/script/clone_all_rspec_repos

-  if rspec_support_compatible; then
-    clone_repo "rspec-support"
-  fi
+  clone_repo "rspec-rails" "main"


Not all builds (Ruby 2.3, Ruby 2.4) builds are rspec-rails compatible.
It's an unnecessary step to check it out for those builds, but I'll leave this improvement for later.

pirj · 2020-12-02T20:18:36Z

ci/script/functions.sh

  export RUBYOPT="--disable=gem"
 fi

 function clone_repo {
  if [ ! -d $1 ]; then # don't clone if the dir is already there
-    travis_retry eval "git clone https://github.com/rspec/$1 --depth 1 --branch $MAINTENANCE_BRANCH"
+    if [ -z "$2" ]; then
+      BRANCH_TO_CLONE="$MAINTENANCE_BRANCH"


Because rspec-rails is alienated from other repos, there's no 4-0-dev branch there, and we clone main.

JonRowe · 2020-12-03T19:37:59Z

ci/.travis.yml

 env:
  - JRUBY_OPTS='--dev'
+  - RSPEC_CI='true'


You don't need this, its only used by Github to recognise windows CI.

As far as I remember, rspec-rails sub-build breaks with #259 (comment)

I'd rather keep this line since probably we'll get rid of this file altogether soon anyway.

JonRowe · 2020-12-03T19:38:51Z

ci/.travis.yml

@@ -12,29 +12,14 @@ before_install:
 bundler_args: "--binstubs --standalone --without documentation --path ../bundle"


To be honest you can delete this file, we won't be using it after the release of RSpec 4, and Jruby is being worked on elsewhere.

I could, but

rspec-dev/ci/script/functions.sh

Line 82 in d27313d

bundle_install_flags=`cat .travis.yml | grep bundler_args | tr -d '"' | grep -o " .*"`

bundle_install_flags=`cat .travis.yml | grep bundler_args | tr -d '"' | grep -o " .*"`

I tried to keep this PR focused to its title, "Drop CI support for Ruby < 2.3", not to upset code reviewers 😄

Ok to adjust ci/script/functions.sh and inline those bundler flags? They don't differ across repos "--binstubs --standalone --without documentation --path ../bundle".

I'd rather keep .travis.yml for now if you don't mind. Because some specs pass on all MRIs, but fail on JRuby on CI. And if my memory doesn't let me down, I couldn't reproduce them locally.
Again, .travis.yml will be removed soon anyway?

pirj · 2020-12-03T20:24:12Z

I'll provide a short summary of what we've been discussing above in the comments.

follow-up: update rake to 13, cucumber to 2.4, will add diff-lcs to >= 1.4.4
[-] ~~rethink define_optimized_require_for_rspec~~
follow-up: fix kwargs delegation (with ruby2_keywords)
drop .travis.ym: can't do, I need JRuby builds for now in Drop Ruby < 2.3 support rspec-core#2787 and sibling PRs
revert most of the changes to script/functions.sh/script/predicate_functions.sh/script/update_rubygems_and_install_bundler - I'm personally not sold on the tradeoff of redoing/merging overhead vs code review overhead
remove RSPEC_CI from .travis.yml - rspec-rails sub-builds on Travis will break (JRuby are the only ones left)
inline bundle_install_flags to remove dependency of script/functions.sh on the presence of .travis.yml
follow-up (or probably wait for main and merge it in 4-0-dev): update JRuby to 9.1.17.0/ add JRuby 9.2
follow-up (the last, since least important): get rid of most of the <<-/chomp/unindent/undent/gsub in favour of <<~
follow-up (post-4.0?): clean up travis naming in build functions

@JonRowe Do you want me to squeeze in some of the above changes in this PR, or ok with addressing in follow-ups?
Is this PR (and related generated PRs) good to merge as is?

1. Clone rspec-rails' main branch for sub-builds 2. Trick rspec-rails' sub-builds into using 4.0.0.pre ============= Starting rspec-rails specs =============== Running specs for rspec-rails ~/work/rspec-mocks/rspec-rails ~/work/rspec-mocks/rspec-mocks Fetching gem metadata from https://rubygems.org/......... Fetching gem metadata from https://rubygems.org/. Resolving dependencies... Bundler could not find compatible versions for gem "rspec-core": In Gemfile: rspec-core rspec-rails was resolved to 4.1.0.pre, which depends on rspec-core (= 3.11.0.pre) Could not find gem 'rspec-core (= 3.11.0.pre)', which is required by gem 'rspec-rails', in any of the relevant sources: source at `/home/runner/work/rspec-mocks/rspec-core`

pirj · 2020-12-06T20:01:42Z

Rebased 4-0-dev on main, and this PR onto 4-0-dev.
Incoming update from main made it possible to remove .travis.yml (no Travis builds left!).

I'll send generated pulls when rspec/rspec-core#2785 is green - rspec-core's 4-0-dev is not yet in line with other repos in terms of CI matrices.

Rails do use Gem in rails/railties/lib/rails/ruby_version_check.rb if Gem::Version.new(RUBY_VERSION) < Gem::Version.new("2.5.0") and obviously since RubyGems are loaded by default by recent rubies, it doesn't care to explicitly require it. That results in: /home/runner/work/rspec-expectations/rspec-rails/bin/rspec An error occurred while loading spec_helper. Failure/Error: require 'rails/all' NameError: uninitialized constant Gem # /home/runner/work/rspec-expectations/bundle/ruby/2.5.0/gems/railties-6.0.3.4/lib/rails/ruby_version_check.rb:3:in `<top (required)>' # /home/runner/work/rspec-expectations/bundle/ruby/2.5.0/gems/railties-6.0.3.4/lib/rails.rb:3:in `require'

pirj self-assigned this Nov 29, 2020

pirj requested review from JonRowe and benoittgt November 29, 2020 11:37

This comment has been minimized.

Sign in to view

benoittgt approved these changes Nov 29, 2020

View reviewed changes

pirj commented Nov 29, 2020

View reviewed changes

ci/script/clone_all_rspec_repos Outdated Show resolved Hide resolved

JonRowe reviewed Dec 2, 2020

View reviewed changes

ci/.travis.yml Outdated Show resolved Hide resolved

JonRowe reviewed Dec 2, 2020

View reviewed changes

JonRowe requested changes Dec 2, 2020

View reviewed changes

pirj added a commit to rspec/rspec-core that referenced this pull request Dec 2, 2020

TEMPORARY! Demonstrate rspec-rails build failure

cc55f91

See rspec/rspec-dev#259 (comment)

pirj force-pushed the drop-old-rubies branch from d853fb9 to d59ba9a Compare December 2, 2020 19:46

pirj commented Dec 2, 2020

View reviewed changes

JonRowe reviewed Dec 3, 2020

View reviewed changes

pirj added 2 commits December 6, 2020 22:46

Drop older rubies from CI

b983f41

Drop CI support code for Ruby < 2.3

5f6b8e0

pirj added 2 commits December 6, 2020 22:46

Skip CI updates for rspec-rails

34f7367

pirj force-pushed the drop-old-rubies branch from 80ad803 to 4b36694 Compare December 6, 2020 19:47

pirj force-pushed the drop-old-rubies branch from a8ec697 to 5f1ff8e Compare December 6, 2020 22:46

pirj closed this Dec 8, 2020

pirj deleted the drop-old-rubies branch December 8, 2020 10:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop CI support for Ruby < 2.3 #259

Drop CI support for Ruby < 2.3 #259

pirj commented Nov 29, 2020 •

edited

Loading

pirj commented Nov 29, 2020

This comment has been minimized.

JonRowe Dec 2, 2020

pirj Dec 2, 2020

JonRowe Dec 2, 2020

JonRowe Dec 2, 2020

pirj Dec 2, 2020

JonRowe Dec 3, 2020

pirj Dec 3, 2020

JonRowe Dec 2, 2020

pirj Dec 2, 2020

JonRowe Dec 2, 2020

JonRowe Dec 2, 2020

JonRowe left a comment

pirj commented Dec 2, 2020

pirj left a comment

pirj Dec 2, 2020

pirj Dec 2, 2020

pirj Dec 2, 2020

pirj Dec 2, 2020

pirj Dec 2, 2020

JonRowe Dec 3, 2020

pirj Dec 3, 2020

JonRowe Dec 3, 2020

pirj Dec 3, 2020

pirj commented Dec 3, 2020 •

edited

Loading

pirj commented Dec 6, 2020

		@@ -9,14 +9,20 @@ SPECS_HAVE_RUN_FILE=specs.out
		MAINTENANCE_BRANCH=`cat maintenance-branch`

		# Don't allow rubygems to pollute what's loaded. Also, things boot faster

		@@ -1,67 +1,21 @@
		function is_mri {
		if ruby -e "exit(!defined?(RUBY_ENGINE) \|\| RUBY_ENGINE == 'ruby')"; then

		@@ -12,29 +12,14 @@ before_install:
		bundler_args: "--binstubs --standalone --without documentation --path ../bundle"

Drop CI support for Ruby < 2.3 #259

Drop CI support for Ruby < 2.3 #259

Conversation

pirj commented Nov 29, 2020 • edited Loading

pirj commented Nov 29, 2020

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JonRowe left a comment

Choose a reason for hiding this comment

pirj commented Dec 2, 2020

pirj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pirj commented Dec 3, 2020 • edited Loading

pirj commented Dec 6, 2020

pirj commented Nov 29, 2020 •

edited

Loading

pirj commented Dec 3, 2020 •

edited

Loading