From 02ca68d0159451cfd19ecf87bb211f7f63c9a909 Mon Sep 17 00:00:00 2001 From: Peter Goldstein Date: Sat, 8 Jan 2022 15:51:01 -0800 Subject: [PATCH 1/6] Add Ruby 3.1 and Rails 7 to CI The bulk of the changes in this PR are test changes. These include: 1. Updating the example app configuration for Rails 7, including disabling eager class loading on CI 2. Updating specs to filter out additional lines that may be generated when running commands 3. Removing ".html.erb" and ".xml.erb" suffixes in render calls 4. Updating specs to accomodate differences in Rails view scaffolding before and after Rails 7 Material code changes include: 1. Adding additional logic to the ActionMailer argument parsing to accomodate for differences under Ruby 3.1/Rails 6.1 2. Symbolizing the handler argument parsed from the spec description for view specs with an empty render --- .github/workflows/ci.yml | 10 +++++++--- example_app_generator/generate_stuff.rb | 6 ++++++ .../spec/support/default_preview_path | 6 ++++++ .../spec/verify_mailer_preview_path_spec.rb | 3 +++ lib/generators/rspec/scaffold/templates/index_spec.rb | 3 ++- lib/rspec/rails/matchers/have_enqueued_mail.rb | 10 +++++++++- .../rspec/scaffold/scaffold_generator_spec.rb | 8 ++++---- 7 files changed, 37 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cdd1d56d72..3809b9fe57 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,6 +30,10 @@ jobs: matrix: include: # Edge Rails (7.1) builds >= 2.7 + - ruby: 3.1 + allow_failure: true + env: + RAILS_VERSION: 'main' - ruby: '3.0' allow_failure: true env: @@ -41,19 +45,19 @@ jobs: # Rails 7.0 builds >= 2.7 - ruby: 3.1 - allow_failure: true env: RAILS_VERSION: '~> 7.0.0' - ruby: '3.0' - allow_failure: true env: RAILS_VERSION: '~> 7.0.0' - ruby: 2.7 - allow_failure: true env: RAILS_VERSION: '~> 7.0.0' # Rails 6.1 builds >= 2.5 + - ruby: 3.1 + env: + RAILS_VERSION: '~> 6.1.0' - ruby: '3.0' env: RAILS_VERSION: '~> 6.1.0' diff --git a/example_app_generator/generate_stuff.rb b/example_app_generator/generate_stuff.rb index ddb2c1a96f..dc1f20fea1 100644 --- a/example_app_generator/generate_stuff.rb +++ b/example_app_generator/generate_stuff.rb @@ -167,4 +167,10 @@ def using_source_path(path) gsub_file 'spec/controllers/uploads_controller_spec.rb', 'skip("Add a hash of attributes valid for your model")', '{}' + +if Rails.version >= '7.0.0' + # Some gems (ActionMailBox, ActionCable, etc.) are not used when running `example_app_generator/spec/verify_mailer_preview_path_spec.rb`, so `eager_load` must be false. + gsub_file "config/environments/test.rb", 'ENV["CI"].present?', "false" +end + final_tasks diff --git a/example_app_generator/spec/support/default_preview_path b/example_app_generator/spec/support/default_preview_path index bb765c510b..5ce38ec73c 100755 --- a/example_app_generator/spec/support/default_preview_path +++ b/example_app_generator/spec/support/default_preview_path @@ -26,6 +26,10 @@ require_file_stub 'config/environment' do require "action_controller/railtie" require "action_mailer/railtie" unless ENV['NO_ACTION_MAILER'] require "action_view/railtie" + if Rails::VERSION::STRING >= '6' + require "action_cable/engine" + require "action_mailbox/engine" + end # Require the gems listed in Gemfile, including any gems # you've limited to :test, :development, or :production. @@ -44,6 +48,8 @@ require_file_stub 'config/environment' do if ENV['SHOW_PREVIEWS'] config.action_mailer.show_previews = (ENV['SHOW_PREVIEWS'] == 'true') end + + config.active_record.legacy_connection_handling = false if Rails::VERSION::STRING >= '7' end end diff --git a/example_app_generator/spec/verify_mailer_preview_path_spec.rb b/example_app_generator/spec/verify_mailer_preview_path_spec.rb index 8dfff2f718..869cecb72c 100644 --- a/example_app_generator/spec/verify_mailer_preview_path_spec.rb +++ b/example_app_generator/spec/verify_mailer_preview_path_spec.rb @@ -22,6 +22,9 @@ def capture_exec(*ops) out = io.readlines .reject { |line| line =~ /warning: circular argument reference/ } .reject { |line| line =~ /Gem::Specification#rubyforge_project=/ } + .reject { |line| line =~ /DEPRECATION WARNING/ } + .reject { |line| line =~ /warning: previous/ } + .reject { |line| line =~ /warning: already/ } .join .chomp CaptureExec.new(out, $?.exitstatus) diff --git a/lib/generators/rspec/scaffold/templates/index_spec.rb b/lib/generators/rspec/scaffold/templates/index_spec.rb index fc5a0f285b..4abbc0da09 100644 --- a/lib/generators/rspec/scaffold/templates/index_spec.rb +++ b/lib/generators/rspec/scaffold/templates/index_spec.rb @@ -18,8 +18,9 @@ it "renders a list of <%= ns_table_name %>" do render + cell_selector = Rails::VERSION::STRING >= '7' ? 'div>p' : 'tr>td' <% for attribute in output_attributes -%> - assert_select "tr>td", text: <%= value_for(attribute) %>.to_s, count: 2 + assert_select cell_selector, text: Regexp.new(<%= value_for(attribute) %>.to_s), count: 2 <% end -%> end end diff --git a/lib/rspec/rails/matchers/have_enqueued_mail.rb b/lib/rspec/rails/matchers/have_enqueued_mail.rb index 86aae6c5cc..3489c0c1ba 100644 --- a/lib/rspec/rails/matchers/have_enqueued_mail.rb +++ b/lib/rspec/rails/matchers/have_enqueued_mail.rb @@ -102,11 +102,19 @@ def process_arguments(job, given_mail_args) def use_given_mail_args?(job) return true if FeatureCheck.has_action_mailer_parameterized? && job[:job] <= ActionMailer::Parameterized::DeliveryJob - return false if FeatureCheck.ruby_3_1? + return false if rails_6_1_and_ruby_3_1? !(FeatureCheck.has_action_mailer_unified_delivery? && job[:job] <= ActionMailer::MailDeliveryJob) end + # TODO: move to FeatureCheck + def rails_6_1_and_ruby_3_1? + return false unless RUBY_VERSION >= "3.1" + return false unless ::Rails::VERSION::STRING >= '6.1' + + ::Rails::VERSION::STRING < '7' + end + def base_mailer_args [mailer_class_name, @method_name.to_s, MAILER_JOB_METHOD] end diff --git a/spec/generators/rspec/scaffold/scaffold_generator_spec.rb b/spec/generators/rspec/scaffold/scaffold_generator_spec.rb index 4638052b44..121d4d614b 100644 --- a/spec/generators/rspec/scaffold/scaffold_generator_spec.rb +++ b/spec/generators/rspec/scaffold/scaffold_generator_spec.rb @@ -208,16 +208,16 @@ before { run_generator %w[posts upvotes:integer downvotes:integer] } subject { file("spec/views/posts/index.html.erb_spec.rb") } it { is_expected.to exist } - it { is_expected.to contain('assert_select "tr>td", text: 2.to_s, count: 2') } - it { is_expected.to contain('assert_select "tr>td", text: 3.to_s, count: 2') } + it { is_expected.to contain('assert_select cell_selector, text: Regexp.new(2.to_s), count: 2') } + it { is_expected.to contain('assert_select cell_selector, text: Regexp.new(3.to_s), count: 2') } end describe 'with multiple float attributes index' do before { run_generator %w[posts upvotes:float downvotes:float] } subject { file("spec/views/posts/index.html.erb_spec.rb") } it { is_expected.to exist } - it { is_expected.to contain('assert_select "tr>td", text: 2.5.to_s, count: 2') } - it { is_expected.to contain('assert_select "tr>td", text: 3.5.to_s, count: 2') } + it { is_expected.to contain('assert_select cell_selector, text: Regexp.new(2.5.to_s), count: 2') } + it { is_expected.to contain('assert_select cell_selector, text: Regexp.new(3.5.to_s), count: 2') } end describe 'with reference attribute' do From e82089462046996448d5cbecd54849985d79c8f3 Mon Sep 17 00:00:00 2001 From: Peter Goldstein Date: Tue, 18 Jan 2022 10:45:02 -0800 Subject: [PATCH 2/6] Update requires to avoid pulling in ActiveRecord, fix other require errors --- Rakefile | 2 +- .../no_active_record/app/models/in_memory/model.rb | 2 ++ example_app_generator/spec/support/default_preview_path | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Rakefile b/Rakefile index 9ed330aaec..945753c0f0 100644 --- a/Rakefile +++ b/Rakefile @@ -160,7 +160,7 @@ namespace :no_active_record do sh "rm -f #{bindir}/rails" sh "bundle exec rails new #{example_app_dir} --no-rc --skip-active-record --skip-javascript --skip-bootsnap " \ "--skip-sprockets --skip-git --skip-test-unit --skip-listen --skip-bundle --skip-spring " \ - "--template=example_app_generator/generate_app.rb" + "--skip-action-text --template=example_app_generator/generate_app.rb" in_example_app(app_dir: example_app_dir) do sh "./ci_retry_bundle_install.sh 2>&1" diff --git a/example_app_generator/no_active_record/app/models/in_memory/model.rb b/example_app_generator/no_active_record/app/models/in_memory/model.rb index 79ae3d83ed..9b440e2c72 100644 --- a/example_app_generator/no_active_record/app/models/in_memory/model.rb +++ b/example_app_generator/no_active_record/app/models/in_memory/model.rb @@ -1,5 +1,7 @@ raise "ActiveRecord is defined but should not be!" if defined?(::ActiveRecord) +require 'active_model' + module InMemory module Persistence def all diff --git a/example_app_generator/spec/support/default_preview_path b/example_app_generator/spec/support/default_preview_path index 5ce38ec73c..32e918ad35 100755 --- a/example_app_generator/spec/support/default_preview_path +++ b/example_app_generator/spec/support/default_preview_path @@ -28,7 +28,7 @@ require_file_stub 'config/environment' do require "action_view/railtie" if Rails::VERSION::STRING >= '6' require "action_cable/engine" - require "action_mailbox/engine" + require "active_job/railtie" end # Require the gems listed in Gemfile, including any gems From 64d7e33043708c5668bbb698a64d7d546d4e13df Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 23 Jan 2022 14:45:59 +0300 Subject: [PATCH 3/6] wip: Extract arg condition method --- lib/rspec/rails/feature_check.rb | 7 +++++-- lib/rspec/rails/matchers/have_enqueued_mail.rb | 10 +--------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/lib/rspec/rails/feature_check.rb b/lib/rspec/rails/feature_check.rb index a866acc79b..7bb9008b06 100644 --- a/lib/rspec/rails/feature_check.rb +++ b/lib/rspec/rails/feature_check.rb @@ -43,8 +43,11 @@ def has_action_mailbox? defined?(::ActionMailbox) end - def ruby_3_1? - RUBY_VERSION >= "3.1" + # TODO: add a self-explanatory method name. See https://github.com/rspec/rspec-rails/pull/2552#issuecomment-1009508693 for hints + def rails_6_1_and_ruby_3_1? + return false if RUBY_VERSION < "3.1" + + ::Rails::VERSION::STRING >= "6.1" && ::Rails::VERSION::STRING < "7" end def type_metatag(type) diff --git a/lib/rspec/rails/matchers/have_enqueued_mail.rb b/lib/rspec/rails/matchers/have_enqueued_mail.rb index 3489c0c1ba..6b68b7670b 100644 --- a/lib/rspec/rails/matchers/have_enqueued_mail.rb +++ b/lib/rspec/rails/matchers/have_enqueued_mail.rb @@ -102,19 +102,11 @@ def process_arguments(job, given_mail_args) def use_given_mail_args?(job) return true if FeatureCheck.has_action_mailer_parameterized? && job[:job] <= ActionMailer::Parameterized::DeliveryJob - return false if rails_6_1_and_ruby_3_1? + return false if FeatureCheck.rails_6_1_and_ruby_3_1? !(FeatureCheck.has_action_mailer_unified_delivery? && job[:job] <= ActionMailer::MailDeliveryJob) end - # TODO: move to FeatureCheck - def rails_6_1_and_ruby_3_1? - return false unless RUBY_VERSION >= "3.1" - return false unless ::Rails::VERSION::STRING >= '6.1' - - ::Rails::VERSION::STRING < '7' - end - def base_mailer_args [mailer_class_name, @method_name.to_s, MAILER_JOB_METHOD] end From 8a550cf9295d5c8ce8ea11181aa9aea361eb3e60 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 23 Jan 2022 14:55:19 +0300 Subject: [PATCH 4/6] fixup! Remove hack that prevented active_record to be preloaded when action_mailbox was --- example_app_generator/generate_stuff.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/example_app_generator/generate_stuff.rb b/example_app_generator/generate_stuff.rb index dc1f20fea1..2431720d62 100644 --- a/example_app_generator/generate_stuff.rb +++ b/example_app_generator/generate_stuff.rb @@ -168,9 +168,4 @@ def using_source_path(path) 'skip("Add a hash of attributes valid for your model")', '{}' -if Rails.version >= '7.0.0' - # Some gems (ActionMailBox, ActionCable, etc.) are not used when running `example_app_generator/spec/verify_mailer_preview_path_spec.rb`, so `eager_load` must be false. - gsub_file "config/environments/test.rb", 'ENV["CI"].present?', "false" -end - final_tasks From d23598e07d39967b3c4488bce71388330c31ac26 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Mon, 24 Jan 2022 00:30:23 +0300 Subject: [PATCH 5/6] Skip Action Mailbox for No AR example app Action Mailbox requires Active Record --- example_app_generator/generate_action_mailer_specs.rb | 1 + example_app_generator/spec/support/default_preview_path | 1 + 2 files changed, 2 insertions(+) diff --git a/example_app_generator/generate_action_mailer_specs.rb b/example_app_generator/generate_action_mailer_specs.rb index 06833fc1a5..b66f052f49 100644 --- a/example_app_generator/generate_action_mailer_specs.rb +++ b/example_app_generator/generate_action_mailer_specs.rb @@ -31,6 +31,7 @@ if skip_active_record? comment_lines 'spec/support/default_preview_path', /active_record/ comment_lines 'spec/support/default_preview_path', /active_storage/ + comment_lines 'spec/support/default_preview_path', /action_mailbox/ end copy_file 'spec/verify_mailer_preview_path_spec.rb' end diff --git a/example_app_generator/spec/support/default_preview_path b/example_app_generator/spec/support/default_preview_path index 32e918ad35..3d64f7487b 100755 --- a/example_app_generator/spec/support/default_preview_path +++ b/example_app_generator/spec/support/default_preview_path @@ -29,6 +29,7 @@ require_file_stub 'config/environment' do if Rails::VERSION::STRING >= '6' require "action_cable/engine" require "active_job/railtie" + require "action_mailbox/engine" end # Require the gems listed in Gemfile, including any gems From c7bba85b7d4c4dce2a54a391a8cc41b1e7c80871 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Mon, 24 Jan 2022 01:13:40 +0300 Subject: [PATCH 6/6] Skip "action mailer missing" example for Rails 7 See https://github.com/rails/rails/pull/43508 --- example_app_generator/spec/verify_mailer_preview_path_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/example_app_generator/spec/verify_mailer_preview_path_spec.rb b/example_app_generator/spec/verify_mailer_preview_path_spec.rb index 869cecb72c..96bfaee787 100644 --- a/example_app_generator/spec/verify_mailer_preview_path_spec.rb +++ b/example_app_generator/spec/verify_mailer_preview_path_spec.rb @@ -111,6 +111,8 @@ def have_no_preview end it 'handles action mailer not being available' do + skip "Rails 7 forces eager loading on CI, loads app/mailers and fails" if Rails::VERSION::STRING.to_f >= 7.0 + expect( capture_exec( custom_env.merge('NO_ACTION_MAILER' => 'true'),