Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass ar5 tests #689

Merged
merged 1 commit into from
Jan 26, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ gemfile:

matrix:
fast_finish: true
allow_failures:
- gemfile: gemfiles/ar5.gemfile
exclude:
- gemfile: gemfiles/ar5.gemfile
rvm: 1.9.3
Expand Down
3 changes: 3 additions & 0 deletions Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@

### Added

None
- [#689](https://github.com/airblade/paper_trail/pull/689) -
Rails 5 compatibility

### Fixed

Expand Down
14 changes: 3 additions & 11 deletions gemfiles/ar3.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions gemfiles/ar5.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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 => "../"
2 changes: 1 addition & 1 deletion lib/paper_trail/record_history.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def initialize(versions, version_class)
# Returns ordinal position of `version` in `sequence`.
# @api private
def index(version)
sequence.index(version)
sequence.to_a.index(version)
end

private
Expand Down
16 changes: 15 additions & 1 deletion lib/paper_trail/reifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ def reify_has_ones(transaction_id, model, options = {})
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
Expand Down Expand Up @@ -265,6 +267,18 @@ def versions_by_id(klass, version_id_subquery)
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
3 changes: 2 additions & 1 deletion spec/models/gadget_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
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

Expand Down
5 changes: 4 additions & 1 deletion spec/models/not_on_update_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@

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
Expand Down
15 changes: 12 additions & 3 deletions spec/models/widget_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@
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
Expand Down Expand Up @@ -253,13 +256,19 @@

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
Expand Down
1 change: 1 addition & 0 deletions spec/rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
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=)
Expand Down
4 changes: 2 additions & 2 deletions spec/requests/articles_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

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
Expand All @@ -21,7 +21,7 @@

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
Expand Down
11 changes: 11 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,14 @@
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment explaining the change to ".. keyword arguments syntax in ActionController::TestCase and ActionDispatch::Integration HTTP request methods." and include a link to the ActionPack changelog (https://github.com/tgxworld/rails/blob/master/actionpack/CHANGELOG.md)

3 changes: 3 additions & 0 deletions test/dummy/config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ class Application < Rails::Application
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
Expand Down
20 changes: 10 additions & 10 deletions test/functional/controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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?
Expand All @@ -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?
Expand Down
4 changes: 2 additions & 2 deletions test/functional/enabled_for_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
34 changes: 27 additions & 7 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ def using_mysql?
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
Expand All @@ -35,7 +40,11 @@ def using_mysql?
# 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

Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment explaining why this params_wrapper method is necessary.


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
Expand All @@ -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
6 changes: 5 additions & 1 deletion test/unit/associations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down