Merge pull request #689 from owenr/pass_ar5_tests

Pass ar5 tests

.travis.yml

@@ -30,8 +30,6 @@ gemfile:
 
 matrix:
   fast_finish: true
-  allow_failures:
-    - gemfile: gemfiles/ar5.gemfile
   exclude:
     - gemfile: gemfiles/ar5.gemfile
       rvm: 1.9.3

Appraisals

@@ -35,4 +35,7 @@ appraise "ar5" do
   gem "rspec-expectations", github: "rspec/rspec-expectations"
   gem "rspec-mocks", github: "rspec/rspec-mocks"
   gem "rspec-support", github: "rspec/rspec-support"
+  gem 'rails-controller-testing'
+  # Sinatra stable conflicts with AR5's rack dependency
+  gem 'sinatra', github: 'sinatra/sinatra'
 end

CHANGELOG.md

@@ -15,7 +15,8 @@
 
 ### Added
 
-None
+- [#689](https://github.com/airblade/paper_trail/pull/689) -
+  Rails 5 compatibility
 
 ### Fixed
 

gemfiles/ar3.gemfile

@@ -2,24 +2,16 @@
 
 source "https://rubygems.org"
 
-gem "activerecord", "~> 3.2"
+gem "activerecord", "~> 3.2.22"
 gem "i18n", "~> 0.6.11"
 gem "request_store", "~> 1.1.0"
 
 group :development, :test do
-  gem "railties", "~> 3.0"
-  gem "test-unit", "~> 3.0"
+  gem "railties", "~> 3.2.22"
+  gem "test-unit", "~> 3.1.5"
 
   platforms :ruby do
     gem "mysql2", "~> 0.3.20"
-    gem "pg", "~> 0.17.1"
-  end
-
-  platforms :jruby do
-    gem "activerecord-jdbcsqlite3-adapter", "~> 1.3"
-    gem "activerecord-jdbcpostgresql-adapter", "~> 1.3"
-    gem "activerecord-jdbcmysql-adapter", "~> 1.3"
-    gem "activerecord-jdbc-adapter", "1.3.15"
   end
 end
 

gemfiles/ar5.gemfile

@@ -11,5 +11,7 @@ gem "rspec-core", :github => "rspec/rspec-core"
 gem "rspec-expectations", :github => "rspec/rspec-expectations"
 gem "rspec-mocks", :github => "rspec/rspec-mocks"
 gem "rspec-support", :github => "rspec/rspec-support"
+gem "rails-controller-testing"
+gem "sinatra", :github => "sinatra/sinatra"
 
 gemspec :path => "../"

lib/paper_trail/record_history.rb

@@ -16,7 +16,7 @@ module PaperTrail
     # Returns ordinal position of `version` in `sequence`.
     # @api private
     def index(version)
-      sequence.index(version)
+      sequence.to_a.index(version)
     end
 
     private

lib/paper_trail/reifier.rb

@@ -154,7 +154,9 @@ module PaperTrail
               else
                 child = version.reify(options.merge(:has_many => false, :has_one => false))
                 model.appear_as_new_record do
-                  model.send "#{assoc.name}=", child
+                  without_persisting(child) do
+                    model.send "#{assoc.name}=", child
+                  end
                 end
               end
             end
@@ -265,6 +267,18 @@ module PaperTrail
           where("id IN (#{version_id_subquery})").
           inject({}) { |acc, v| acc.merge!(v.item_id => v) }
       end
+
+      # Temporarily suppress #save so we can reassociate with the reified
+      # master of a has_one relationship. Since ActiveRecord 5 the related
+      # object is saved when it is assigned to the association. ActiveRecord
+      # 5 also happens to be the first version that provides #suppress.
+      def without_persisting(record)
+        if record.class.respond_to? :suppress
+          record.class.suppress { yield }
+        else
+          yield
+        end
+      end
     end
   end
 end

spec/models/gadget_spec.rb

@@ -15,7 +15,8 @@ describe Gadget, :type => :model do
     end
 
     it "should still generate a version when only the `updated_at` attribute is updated" do
-      expect { gadget.update_attribute(:updated_at, Time.now) }.to change{gadget.versions.size}.by(1)
+      # Plus 1 second because MySQL lacks sub-second resolution
+      expect { gadget.update_attribute(:updated_at, Time.now + 1) }.to change{gadget.versions.size}.by(1)
     end
   end
 

spec/models/not_on_update_spec.rb

@@ -12,7 +12,10 @@ describe NotOnUpdate, :type => :model do
 
     it "increments the `:updated_at` timestamp" do
       before = record.updated_at
-      record.touch_with_version
+      # Travel 1 second because MySQL lacks sub-second resolution
+      Timecop.travel(1) do
+        record.touch_with_version
+      end
       expect(record.updated_at).to be > before
     end
   end

spec/models/widget_spec.rb

@@ -43,7 +43,10 @@ describe Widget, :type => :model do
         subject { widget.versions.last.reify }
 
         it "should reset the value for the timestamp attrs for update so that value gets updated properly" do
-          expect { subject.save! }.to change(subject, :updated_at)
+          # Travel 1 second because MySQL lacks sub-second resolution
+          Timecop.travel(1) do
+            expect { subject.save! }.to change(subject, :updated_at)
+          end
         end
       end
     end
@@ -253,13 +256,19 @@ describe Widget, :type => :model do
 
         it "creates a version" do
           count = widget.versions.size
-          widget.touch_with_version
+          # Travel 1 second because MySQL lacks sub-second resolution
+          Timecop.travel(1) do
+            widget.touch_with_version
+          end
           expect(widget.versions.size).to eq(count + 1)
         end
 
         it "increments the `:updated_at` timestamp" do
           time_was = widget.updated_at
-          widget.touch_with_version
+          # Travel 1 second because MySQL lacks sub-second resolution
+          Timecop.travel(1) do
+            widget.touch_with_version
+          end
           expect(widget.updated_at).to be > time_was
         end
       end

spec/rails_helper.rb

@@ -12,6 +12,7 @@ require 'rspec/rails'
 require 'paper_trail/frameworks/rspec'
 require 'shoulda/matchers'
 require 'ffaker'
+require 'timecop'
 
 # prevent Test::Unit's AutoRunner from executing during RSpec's rake task
 Test::Unit.run = true if defined?(Test::Unit) && Test::Unit.respond_to?(:run=)

spec/requests/articles_spec.rb

@@ -8,7 +8,7 @@ describe "Articles management", :type => :request, :order => :defined do
 
     it "should not create a version" do
       expect(PaperTrail).to be_enabled_for_controller
-      expect { post(articles_path, valid_params) }.to_not change(PaperTrail::Version, :count)
+      expect { post articles_path, params_wrapper(valid_params) }.to_not change(PaperTrail::Version, :count)
     end
 
     it "should not leak the state of the `PaperTrail.enabled_for_controller?` into the next test" do
@@ -21,7 +21,7 @@ describe "Articles management", :type => :request, :order => :defined do
 
     context "`current_user` method returns a `String`" do
       it "should set that value as the `whodunnit`" do
-        expect { post articles_path, valid_params }.to change(PaperTrail::Version, :count).by(1)
+        expect { post articles_path, params_wrapper(valid_params) }.to change(PaperTrail::Version, :count).by(1)
         expect(article.title).to eq('Doh')
         expect(article.versions.last.whodunnit).to eq('foobar')
       end

spec/spec_helper.rb

@@ -102,3 +102,14 @@ RSpec.configure do |config|
   Kernel.srand config.seed
 =end
 end
+
+# Wrap args in a hash to support the ActionController::TestCase and
+# ActionDispatch::Integration HTTP request method switch to keyword args
+# (see https://github.com/rails/rails/blob/master/actionpack/CHANGELOG.md)
+def params_wrapper(args)
+  if defined?(::Rails) && Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new('5.0.0.beta1')
+    { params: args }
+  else
+    args
+  end
+end

test/dummy/config/application.rb

@@ -69,6 +69,9 @@ module Dummy
       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")
+        config.active_record.time_zone_aware_types = [:datetime]
+      end
     end
 
     # Set test order for Test::Unit if possible

test/functional/controller_test.rb

@@ -15,32 +15,32 @@ class ControllerTest < ActionController::TestCase
 
   test 'disable on create' do
     @request.env['HTTP_USER_AGENT'] = 'Disable User-Agent'
-    post :create, :widget => { :name => 'Flugel' }
+    post :create, params_wrapper({ widget: { name: 'Flugel' } })
     assert_equal 0, assigns(:widget).versions.length
   end
 
   test 'disable on update' do
     @request.env['HTTP_USER_AGENT'] = 'Disable User-Agent'
-    post :create, :widget => { :name => 'Flugel' }
+    post :create, params_wrapper({ widget: { name: 'Flugel' } })
     w = assigns(:widget)
     assert_equal 0, w.versions.length
-    put :update, :id => w.id, :widget => { :name => 'Bugle' }
+    put :update, params_wrapper({ id: w.id, widget: { name: 'Bugle' } })
     widget = assigns(:widget)
     assert_equal 0, widget.versions.length
   end
 
   test 'disable on destroy' do
     @request.env['HTTP_USER_AGENT'] = 'Disable User-Agent'
-    post :create, :widget => { :name => 'Flugel' }
+    post :create, params_wrapper({ widget: { name: 'Flugel' } })
     w = assigns(:widget)
     assert_equal 0, w.versions.length
-    delete :destroy, :id => w.id
+    delete :destroy, params_wrapper({ id: w.id })
     widget = assigns(:widget)
     assert_equal 0, PaperTrail::Version.with_item_keys('Widget', w.id).size
   end
 
   test 'create' do
-    post :create, :widget => { :name => 'Flugel' }
+    post :create, params_wrapper({ widget: { name: 'Flugel' } })
     widget = assigns(:widget)
     assert_equal 1, widget.versions.length
     assert_equal 153, widget.versions.last.whodunnit.to_i
@@ -51,7 +51,7 @@ class ControllerTest < ActionController::TestCase
   test 'update' do
     w = Widget.create :name => 'Duvel'
     assert_equal 1, w.versions.length
-    put :update, :id => w.id, :widget => { :name => 'Bugle' }
+    put :update, params_wrapper({ id: w.id, widget: { name: 'Bugle' } })
     widget = assigns(:widget)
     assert_equal 2, widget.versions.length
     assert_equal 153, widget.versions.last.whodunnit.to_i
@@ -62,7 +62,7 @@ class ControllerTest < ActionController::TestCase
   test 'destroy' do
     w = Widget.create :name => 'Roundel'
     assert_equal 1, w.versions.length
-    delete :destroy, :id => w.id
+    delete :destroy, params_wrapper({ id: w.id })
     widget = assigns(:widget)
     assert_equal 2,               widget.versions.length
     assert_equal '127.0.0.1',     widget.versions.last.ip
@@ -72,7 +72,7 @@ class ControllerTest < ActionController::TestCase
 
   test "controller metadata methods should get evaluated if paper trail is enabled for controller" do
     @request.env['HTTP_USER_AGENT'] = 'User-Agent'
-    post :create, :widget => { :name => 'Flugel' }
+    post :create, params_wrapper({ widget: { name: 'Flugel' } })
     assert PaperTrail.enabled_for_controller?
     assert_equal 153, PaperTrail.whodunnit
     assert PaperTrail.controller_info.present?
@@ -82,7 +82,7 @@ class ControllerTest < ActionController::TestCase
 
   test "controller metadata methods should not get evaluated if paper trail is disabled for controller" do
     @request.env['HTTP_USER_AGENT'] = 'Disable User-Agent'
-    post :create, :widget => { :name => 'Flugel' }
+    post :create, params_wrapper({ widget: { name: 'Flugel' } })
     assert_equal 0, assigns(:widget).versions.length
     assert !PaperTrail.enabled_for_controller?
     assert PaperTrail.whodunnit.nil?

test/functional/enabled_for_controller_test.rb

@@ -6,7 +6,7 @@ class EnabledForControllerTest < ActionController::TestCase
   context "`PaperTrail.enabled? == true`" do
     should 'enabled_for_controller?.should == true' do
       assert PaperTrail.enabled?
-      post :create, :article => { :title => 'Doh', :content => FFaker::Lorem.sentence }
+      post :create, params_wrapper({ article: { title: 'Doh', content: FFaker::Lorem.sentence } })
       assert_not_nil assigns(:article)
       assert PaperTrail.enabled_for_controller?
       assert_equal 1, assigns(:article).versions.length
@@ -18,7 +18,7 @@ class EnabledForControllerTest < ActionController::TestCase
 
     should 'enabled_for_controller?.should == false' do
       assert !PaperTrail.enabled?
-      post :create, :article => { :title => 'Doh', :content => FFaker::Lorem.sentence }
+      post :create, params_wrapper({ article: { title: 'Doh', content: FFaker::Lorem.sentence } })
       assert !PaperTrail.enabled_for_controller?
       assert_equal 0, assigns(:article).versions.length
     end

test/test_helper.rb

@@ -21,6 +21,11 @@ require 'shoulda'
 require 'ffaker'
 require 'database_cleaner'
 
+if Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new('5.0.0.beta1')
+  # See https://github.com/rails/rails-controller-testing/issues/5
+  ActionController::TestCase.send(:include, Rails::Controller::Testing::TestProcess)
+end
+
 Rails.backtrace_cleaner.remove_silencers!
 
 # Run any available migration
@@ -35,7 +40,11 @@ DatabaseCleaner.strategy = :truncation
 # global setup block resetting Thread.current
 class ActiveSupport::TestCase
   if using_mysql?
-    self.use_transactional_fixtures = false
+    if respond_to? :use_transactional_tests=
+      self.use_transactional_tests = false
+    else
+      self.use_transactional_fixtures = false
+    end
     setup { DatabaseCleaner.start }
   end
 
@@ -89,6 +98,22 @@ def change_schema
   reset_version_class_column_info!
 end
 
+# Wrap args in a hash to support the ActionController::TestCase and
+# ActionDispatch::Integration HTTP request method switch to keyword args
+# (see https://github.com/rails/rails/blob/master/actionpack/CHANGELOG.md)
+def params_wrapper(args)
+  if defined?(::Rails) && Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new('5.0.0.beta1')
+    { params: args }
+  else
+    args
+  end
+end
+
+def reset_version_class_column_info!
+  PaperTrail::Version.connection.schema_cache.clear!
+  PaperTrail::Version.reset_column_information
+end
+
 def restore_schema
   ActiveRecord::Migration.verbose = false
   ActiveRecord::Schema.define do
@@ -97,9 +122,4 @@ def restore_schema
   end
   ActiveRecord::Migration.verbose = true
   reset_version_class_column_info!
-end
-
-def reset_version_class_column_info!
-  PaperTrail::Version.connection.schema_cache.clear!
-  PaperTrail::Version.reset_column_information
-end
+end

test/unit/associations_test.rb

@@ -19,7 +19,11 @@ class AssociationsTest < ActiveSupport::TestCase
 
   # These would have been done in test_helper.rb if using_mysql? is true
   unless using_mysql?
-    self.use_transactional_fixtures = false
+    if self.respond_to? :use_transactional_tests=
+      self.use_transactional_tests = false
+    else
+      self.use_transactional_fixtures = false
+    end
     setup { DatabaseCleaner.start }
   end