From b2cb837c8170f30f60a1f250b501a199f8b89b2e Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Mon, 14 Dec 2020 15:49:48 -0500 Subject: [PATCH 1/4] Tests: use load_defaults in dummy_app Suggested in https://github.com/paper-trail-gem/paper_trail/pull/1273#issuecomment-744357944 --- spec/dummy_app/app/models/family/celebrity_family.rb | 2 +- spec/dummy_app/config/application.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/dummy_app/app/models/family/celebrity_family.rb b/spec/dummy_app/app/models/family/celebrity_family.rb index 3eadda09..3e4bcd84 100644 --- a/spec/dummy_app/app/models/family/celebrity_family.rb +++ b/spec/dummy_app/app/models/family/celebrity_family.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true module Family - class CelebrityFamily < Family::Family + class CelebrityFamily < ::Family::Family end end diff --git a/spec/dummy_app/config/application.rb b/spec/dummy_app/config/application.rb index 71ff09c3..cfbdd923 100644 --- a/spec/dummy_app/config/application.rb +++ b/spec/dummy_app/config/application.rb @@ -11,6 +11,8 @@ require "paper_trail" module Dummy class Application < Rails::Application + config.load_defaults(::Rails.gem_version.segments.take(2).join(".")) + config.encoding = "utf-8" config.filter_parameters += [:password] config.active_support.escape_html_entities_in_json = true @@ -38,7 +40,6 @@ module Dummy config.active_record.time_zone_aware_types = [:datetime] end if v >= Gem::Version.new("5.1") - config.load_defaults "5.1" config.active_record.time_zone_aware_types = [:datetime] end end From 329ae89a8fa1593aa36ba6888df33a92a4b55aff Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Mon, 14 Dec 2020 15:54:08 -0500 Subject: [PATCH 2/4] Tests: dummy_app: remove config for old rails versions --- spec/dummy_app/config/application.rb | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/spec/dummy_app/config/application.rb b/spec/dummy_app/config/application.rb index cfbdd923..e7630928 100644 --- a/spec/dummy_app/config/application.rb +++ b/spec/dummy_app/config/application.rb @@ -17,32 +17,8 @@ module Dummy config.filter_parameters += [:password] config.active_support.escape_html_entities_in_json = true config.active_support.test_order = :sorted - - # Disable assets in rails 4.2. In rails 5, config does not respond to - # assets, probably because it was moved out of railties to some other gem, - # and we only have dev. dependencies on railties, not all of rails. When - # we drop support for rails 4.2, we can remove this whole conditional. - if config.respond_to?(:assets) - config.assets.enabled = false - end - config.secret_key_base = "A fox regularly kicked the screaming pile of biscuits." - - # `raise_in_transactional_callbacks` was added in rails 4, then deprecated - # in rails 5. Oh, how fickle are the gods. - if ActiveRecord.respond_to?(:gem_version) - v = ActiveRecord.gem_version - if v >= Gem::Version.new("4.2") && v < Gem::Version.new("5.0.0.beta1") - config.active_record.raise_in_transactional_callbacks = true - end - if v >= Gem::Version.new("5.0.0.beta1") && v < Gem::Version.new("5.1") - config.active_record.belongs_to_required_by_default = true - config.active_record.time_zone_aware_types = [:datetime] - end - if v >= Gem::Version.new("5.1") - config.active_record.time_zone_aware_types = [:datetime] - end - end + config.active_record.time_zone_aware_types = [:datetime] # In rails >= 6.0, "`.represent_boolean_as_integer=` is now always true, # so setting this is deprecated and will be removed in Rails 6.1." From d9474bdc499dfd52467a8e34d51847c33a509139 Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Mon, 14 Dec 2020 16:03:52 -0500 Subject: [PATCH 3/4] Tests: Explicitly specify rails as a dev. dependency spec/dummy_app needs rails. Technically, it only needs actionpack, but in case that changes in the future, I think we should just have rails as an explicit dependency. You may ask, if dummy_app has always needed actionpack, and our gemspec didn't explicitly specify the later, how did it ever work? It looks like we were getting actionpack as a transitive dependency. --- Appraisals | 7 ++++--- gemfiles/ar_5.2.gemfile | 2 +- gemfiles/ar_6.0.gemfile | 2 +- paper_trail.gemspec | 8 +++++++- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/Appraisals b/Appraisals index 2622668c..17762f0a 100644 --- a/Appraisals +++ b/Appraisals @@ -8,13 +8,14 @@ # > appraisal. If something is specified in both the Gemfile and an appraisal, # > the version from the appraisal takes precedence. # > https://github.com/thoughtbot/appraisal - +# +# appraise "ar-5.2" do - gem "activerecord", "~> 5.2.4" + gem "rails", "~> 5.2.4" gem "rails-controller-testing", "~> 1.0.2" end appraise "ar-6.0" do - gem "activerecord", "~> 6.0.3" + gem "rails", "~> 6.0.3" gem "rails-controller-testing", "~> 1.0.3" end diff --git a/gemfiles/ar_5.2.gemfile b/gemfiles/ar_5.2.gemfile index a34a25f2..4fd4d624 100644 --- a/gemfiles/ar_5.2.gemfile +++ b/gemfiles/ar_5.2.gemfile @@ -2,7 +2,7 @@ source "https://rubygems.org" -gem "activerecord", "~> 5.2.4" +gem "rails", "~> 5.2.4" gem "rails-controller-testing", "~> 1.0.2" gemspec path: "../" diff --git a/gemfiles/ar_6.0.gemfile b/gemfiles/ar_6.0.gemfile index 9e94f4f3..9cfa16c9 100644 --- a/gemfiles/ar_6.0.gemfile +++ b/gemfiles/ar_6.0.gemfile @@ -2,7 +2,7 @@ source "https://rubygems.org" -gem "activerecord", "~> 6.0.3" +gem "rails", "~> 6.0.3" gem "rails-controller-testing", "~> 1.0.3" gemspec path: "../" diff --git a/paper_trail.gemspec b/paper_trail.gemspec index 78b8b2ee..05506707 100644 --- a/paper_trail.gemspec +++ b/paper_trail.gemspec @@ -31,7 +31,7 @@ has been destroyed. # https://www.ruby-lang.org/en/news/2019/10/02/ruby-2-4-9-released/ s.required_ruby_version = ">= 2.4.0" - # We no longer specify a maximum rails version. + # We no longer specify a maximum activerecord version. # See discussion in paper_trail/compatibility.rb s.add_dependency "activerecord", ::PaperTrail::Compatibility::ACTIVERECORD_GTE s.add_dependency "request_store", "~> 1.1" @@ -41,6 +41,12 @@ has been destroyed. s.add_development_dependency "ffaker", "~> 2.11" s.add_development_dependency "generator_spec", "~> 0.9.4" s.add_development_dependency "memory_profiler", "~> 0.9.14" + + # For `spec/dummy_app`. Technically, we only need `actionpack` (as of 2020). + # However, that might change in the future, and the advantages of specifying a + # subset (e.g. actionpack only) are unclear. + s.add_development_dependency "rails", ::PaperTrail::Compatibility::ACTIVERECORD_GTE + s.add_development_dependency "rake", "~> 13.0" s.add_development_dependency "rspec-rails", "~> 4.0" s.add_development_dependency "rubocop", "~> 0.89.1" From 31862462cebd6bae3eceba43f6858d99827bdaa6 Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Mon, 14 Dec 2020 17:07:58 -0500 Subject: [PATCH 4/4] Tests: Remove defunct config: time_zone_aware_types We started setting this config in rails 5, to preserve the behavior in rails 4, ie. `time` columns being "zone-unaware". If we rewrite our `time` column tests to focus on the time only (and not the date) then we don't need to set time_zone_aware_types. --- spec/dummy_app/config/application.rb | 1 - spec/paper_trail/model_spec.rb | 8 +++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/spec/dummy_app/config/application.rb b/spec/dummy_app/config/application.rb index e7630928..1626dd27 100644 --- a/spec/dummy_app/config/application.rb +++ b/spec/dummy_app/config/application.rb @@ -18,7 +18,6 @@ module Dummy config.active_support.escape_html_entities_in_json = true config.active_support.test_order = :sorted config.secret_key_base = "A fox regularly kicked the screaming pile of biscuits." - config.active_record.time_zone_aware_types = [:datetime] # In rails >= 6.0, "`.represent_boolean_as_integer=` is now always true, # so setting this is deprecated and will be removed in Rails 6.1." diff --git a/spec/paper_trail/model_spec.rb b/spec/paper_trail/model_spec.rb index 0e1ec3ed..5953752f 100644 --- a/spec/paper_trail/model_spec.rb +++ b/spec/paper_trail/model_spec.rb @@ -335,8 +335,9 @@ RSpec.describe(::PaperTrail, versioning: true) do expect(previous_widget.a_datetime.to_time.utc.to_i).to(eq(t0.to_time.utc.to_i)) end - it "handle times" do - expect(previous_widget.a_time.utc.to_i).to(eq(t0.utc.to_i)) + it "handle times (time only, no date)" do + format = ->(t) { t.utc.strftime "%H:%M:%S" } + expect(format[previous_widget.a_time]).to eq(format[t0]) end it "handle dates" do @@ -362,7 +363,8 @@ RSpec.describe(::PaperTrail, versioning: true) do assert_in_delta(153.01, reified.a_float, 0.001) assert_in_delta(2.7183, reified.a_decimal, 0.0001) expect(reified.a_datetime.to_time.utc.to_i).to(eq(t0.to_time.utc.to_i)) - expect(reified.a_time.utc.to_i).to(eq(t0.utc.to_i)) + format = ->(t) { t.utc.strftime "%H:%M:%S" } + expect(format[reified.a_time]).to eq(format[t0]) expect(reified.a_date).to(eq(d0)) expect(reified.a_boolean).to(be_truthy) end