From eba26c0bf66451f0d0f44e1a8f6c8d5509cb5747 Mon Sep 17 00:00:00 2001 From: Peter Goldstein Date: Sat, 8 Jan 2022 15:51:01 -0800 Subject: [PATCH 1/2] 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 +++ features/view_specs/view_spec.feature | 6 ++--- .../rspec/scaffold/templates/index_spec.rb | 3 ++- lib/rspec/rails/example/view_example_group.rb | 2 +- .../rails/matchers/have_enqueued_mail.rb | 25 +++++++++++++++++-- .../rspec/scaffold/scaffold_generator_spec.rb | 8 +++--- .../rails/example/view_example_group_spec.rb | 6 ++--- 10 files changed, 58 insertions(+), 17 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/features/view_specs/view_spec.feature b/features/view_specs/view_spec.feature index 724bc4cae2..76f99846b1 100644 --- a/features/view_specs/view_spec.feature +++ b/features/view_specs/view_spec.feature @@ -80,7 +80,7 @@ Feature: view spec it "displays the widget" do assign(:widget, Widget.create!(:name => "slicer")) - render :template => "widgets/widget.html.erb" + render :template => "widgets/widget" expect(rendered).to match /slicer/ end @@ -103,7 +103,7 @@ Feature: view spec it "displays the widget" do assign(:widget, Widget.create!(:name => "slicer")) - render :template => "widgets/widget.html.erb", :layout => "layouts/inventory" + render :template => "widgets/widget", :layout => "layouts/inventory" expect(rendered).to match /slicer/ end @@ -162,7 +162,7 @@ Feature: view spec it "displays the widget" do widget = Widget.create!(:name => "slicer") - render :partial => "widgets/widget.html.erb", :locals => {:widget => widget} + render :partial => "widgets/widget", :locals => {:widget => widget} expect(rendered).to match /slicer/ end 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/example/view_example_group.rb b/lib/rspec/rails/example/view_example_group.rb index 2240dd830a..186d238e93 100644 --- a/lib/rspec/rails/example/view_example_group.rb +++ b/lib/rspec/rails/example/view_example_group.rb @@ -150,7 +150,7 @@ def _default_render_options match = path_regex.match(_default_file_to_render) render_options = {template: match[:template]} - render_options[:handlers] = [match[:handler]] if match[:handler] + render_options[:handlers] = [match[:handler].to_sym] if match[:handler] render_options[:formats] = [match[:format].to_sym] if match[:format] render_options[:locales] = [match[:locale]] if match[:locale] render_options[:variants] = [match[:variant]] if match[:variant] diff --git a/lib/rspec/rails/matchers/have_enqueued_mail.rb b/lib/rspec/rails/matchers/have_enqueued_mail.rb index 92d7f1e9fb..38388b0a8d 100644 --- a/lib/rspec/rails/matchers/have_enqueued_mail.rb +++ b/lib/rspec/rails/matchers/have_enqueued_mail.rb @@ -91,7 +91,7 @@ def arguments_match?(job) def process_arguments(job, given_mail_args) # Old matcher behavior working with all builtin classes but ActionMailer::MailDeliveryJob - return given_mail_args unless defined?(ActionMailer::MailDeliveryJob) && job[:job] <= ActionMailer::MailDeliveryJob + return given_mail_args if use_given_mail_args?(job) # If matching args starts with a hash and job instance has params match with them if given_mail_args.first.is_a?(Hash) && job[:args][3]['params'].present? @@ -101,6 +101,20 @@ def process_arguments(job, given_mail_args) end end + def use_given_mail_args?(job) + return true if defined?(ActionMailer::Parameterized::DeliveryJob) && job[:job] <= ActionMailer::Parameterized::DeliveryJob + return false if rails_6_1_and_ruby_3_1? + + !(defined?(ActionMailer::MailDeliveryJob) && job[:job] <= ActionMailer::MailDeliveryJob) + end + + 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 @@ -143,13 +157,20 @@ def mail_job_message(job) mailer_args = deserialize_arguments(job)[3..-1] mailer_args = mailer_args.first[:args] if unified_mail?(job) msg_parts = [] - msg_parts << "with #{mailer_args}" if mailer_args.any? + display_args = display_mailer_args(mailer_args) + msg_parts << "with #{display_args}" if display_args.any? msg_parts << "on queue #{job[:queue]}" if job[:queue] && job[:queue] != 'mailers' msg_parts << "at #{Time.at(job[:at])}" if job[:at] "#{mailer_method} #{msg_parts.join(', ')}".strip end + def display_mailer_args(mailer_args) + return mailer_args unless mailer_args.first.is_a?(Hash) && mailer_args.first.key?(:args) + + mailer_args.first[:args] + end + def legacy_mail?(job) RSpec::Rails::FeatureCheck.has_action_mailer_legacy_delivery_job? && job[:job] <= ActionMailer::DeliveryJob 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 diff --git a/spec/rspec/rails/example/view_example_group_spec.rb b/spec/rspec/rails/example/view_example_group_spec.rb index 701b982882..1137fddd7f 100644 --- a/spec/rspec/rails/example/view_example_group_spec.rb +++ b/spec/rspec/rails/example/view_example_group_spec.rb @@ -151,19 +151,19 @@ def _default_file_to_render; end # Stub method it "converts the filename components into render options" do allow(view_spec).to receive(:_default_file_to_render) { "widgets/new.en.html.erb" } view_spec.render - expect(view_spec.received.first).to eq([{template: "widgets/new", locales: ['en'], formats: [:html], handlers: ['erb']}, {}, nil]) + expect(view_spec.received.first).to eq([{template: "widgets/new", locales: ['en'], formats: [:html], handlers: [:erb]}, {}, nil]) end it "converts the filename with variant into render options" do allow(view_spec).to receive(:_default_file_to_render) { "widgets/new.en.html+fancy.erb" } view_spec.render - expect(view_spec.received.first).to eq([{template: "widgets/new", locales: ['en'], formats: [:html], handlers: ['erb'], variants: ['fancy']}, {}, nil]) + expect(view_spec.received.first).to eq([{template: "widgets/new", locales: ['en'], formats: [:html], handlers: [:erb], variants: ['fancy']}, {}, nil]) end it "converts the filename without format into render options" do allow(view_spec).to receive(:_default_file_to_render) { "widgets/new.en.erb" } view_spec.render - expect(view_spec.received.first).to eq([{template: "widgets/new", locales: ['en'], handlers: ['erb']}, {}, nil]) + expect(view_spec.received.first).to eq([{template: "widgets/new", locales: ['en'], handlers: [:erb]}, {}, nil]) end end From a4660efdcd87dd24fd2ee4468322b18d92f0e4d6 Mon Sep 17 00:00:00 2001 From: Peter Goldstein Date: Tue, 18 Jan 2022 10:18:25 -0800 Subject: [PATCH 2/2] Update requires to only pull in needed files --- 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