From f812ea5f45895e74db0ae87767a2840a589e16d5 Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Mon, 4 Jan 2016 22:32:39 -0500 Subject: [PATCH 1/2] Manage gemfiles with appraisal --- .gitignore | 1 + .travis.yml | 7 +++++-- Appraisals | 45 ++++++++++++++++++++++++++++++++++++++++++++ gemfiles/3.0.gemfile | 40 --------------------------------------- gemfiles/ar3.gemfile | 26 +++++++++++++++++++++++++ gemfiles/ar4.gemfile | 7 +++++++ gemfiles/ar5.gemfile | 15 +++++++++++++++ paper_trail.gemspec | 1 + 8 files changed, 100 insertions(+), 42 deletions(-) create mode 100644 Appraisals delete mode 100644 gemfiles/3.0.gemfile create mode 100644 gemfiles/ar3.gemfile create mode 100644 gemfiles/ar4.gemfile create mode 100644 gemfiles/ar5.gemfile diff --git a/.gitignore b/.gitignore index 6eac51464..ace5640e0 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ +gemfiles/*.lock NOTES test/debug.log test/paper_trail_plugin.sqlite3.db diff --git a/.travis.yml b/.travis.yml index 83f58faa7..992008579 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,11 +24,14 @@ before_script: - sh -c "if [ \"$DB\" = 'postgres' ]; then psql -c 'create database paper_trail_foo;' -U postgres; fi" gemfile: - - Gemfile - - gemfiles/3.0.gemfile + - gemfiles/ar3.gemfile + - gemfiles/ar4.gemfile + - gemfiles/ar5.gemfile matrix: fast_finish: true + exclude: + - gemfile: gemfiles/ar5.gemfile addons: postgresql: "9.4" diff --git a/Appraisals b/Appraisals new file mode 100644 index 000000000..4993b15fc --- /dev/null +++ b/Appraisals @@ -0,0 +1,45 @@ +# Specify here only version constraints that differ from +# `paper_trail.gemspec`. +# +# > The dependencies in your Appraisals file are combined with dependencies in +# > your Gemfile, so you don"t need to repeat anything that"s the same for each +# > 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 "ar3" do + gem "activerecord", "~> 3.2" + gem "i18n", "~> 0.6.11" + gem "request_store", "~> 1.1.0" + + group :development, :test do + gem 'railties', '~> 3.0' + gem 'test-unit', '~> 3.0' + 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 +end + +appraise "ar4" do + gem "activerecord", "~> 4.2" +end + +appraise "ar5" do + gem "activerecord", "5.0.0.beta1" + gem "activemodel", "5.0.0.beta1" + gem "actionpack", "5.0.0.beta1" + gem "railties", "5.0.0.beta1" + gem "rspec-rails", github: "rspec/rspec-rails" + 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" +end diff --git a/gemfiles/3.0.gemfile b/gemfiles/3.0.gemfile deleted file mode 100644 index a877e5c76..000000000 --- a/gemfiles/3.0.gemfile +++ /dev/null @@ -1,40 +0,0 @@ -source 'https://rubygems.org' - -gem 'activerecord', '~> 3.0' -gem 'i18n', '~> 0.6.11' -gem 'request_store', '~> 1.1.0' - -group :development, :test do - gem 'rake', '~> 10.4.2' - gem 'rubocop', '~> 0.35.1' - gem 'shoulda', '~> 3.5' - gem 'ffaker', '~> 2.1.0' - gem 'timecop' - - # Testing of Rails - gem 'railties', '~> 3.0' - - # Testing of Sinatra - gem 'sinatra', '~> 1.0' - gem 'rack-test', '>= 0.6' - - # RSpec testing - gem 'rspec-rails', '~> 3.4.0' - gem 'generator_spec' - - # To do proper transactional testing with ActiveSupport::TestCase on MySQL - gem 'database_cleaner', '~> 1.2.0' - - platforms :ruby do - gem 'sqlite3', '~> 1.2' - gem 'mysql2', '~> 0.3.20' # activerecord < 4.2.5 must use mysql2 < 0.4 - 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 diff --git a/gemfiles/ar3.gemfile b/gemfiles/ar3.gemfile new file mode 100644 index 000000000..c3b2bb828 --- /dev/null +++ b/gemfiles/ar3.gemfile @@ -0,0 +1,26 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "activerecord", "~> 3.2" +gem "i18n", "~> 0.6.11" +gem "request_store", "~> 1.1.0" + +group :development, :test do + gem "railties", "~> 3.0" + gem "test-unit", "~> 3.0" + + 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 + +gemspec :path => "../" diff --git a/gemfiles/ar4.gemfile b/gemfiles/ar4.gemfile new file mode 100644 index 000000000..c03c79079 --- /dev/null +++ b/gemfiles/ar4.gemfile @@ -0,0 +1,7 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "activerecord", "~> 4.2" + +gemspec :path => "../" diff --git a/gemfiles/ar5.gemfile b/gemfiles/ar5.gemfile new file mode 100644 index 000000000..9b2fed950 --- /dev/null +++ b/gemfiles/ar5.gemfile @@ -0,0 +1,15 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "activerecord", "5.0.0.beta1" +gem "activemodel", "5.0.0.beta1" +gem "actionpack", "5.0.0.beta1" +gem "railties", "5.0.0.beta1" +gem "rspec-rails", :github => "rspec/rspec-rails" +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" + +gemspec :path => "../" diff --git a/paper_trail.gemspec b/paper_trail.gemspec index 23c77826a..8a05a2ed7 100644 --- a/paper_trail.gemspec +++ b/paper_trail.gemspec @@ -24,6 +24,7 @@ Gem::Specification.new do |s| s.add_dependency 'activesupport', ['>= 3.0', '< 6.0'] s.add_dependency 'request_store', '~> 1.1' + s.add_development_dependency 'appraisal', '~> 2.1' s.add_development_dependency 'rake', '~> 10.4.2' s.add_development_dependency 'shoulda', '~> 3.5' s.add_development_dependency 'ffaker', '~> 2.1.0' From 9da84fbbe97953862a3d595e6df2a6c4a2e24f25 Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Mon, 4 Jan 2016 23:53:52 -0500 Subject: [PATCH 2/2] Begin testing (not passing) against AR 5 --- .travis.yml | 7 +- CONTRIBUTING.md | 11 ++- .../app/controllers/application_controller.rb | 7 +- test/dummy/app/models/song.rb | 12 ++- test/dummy/config/application.rb | 16 +++- test/dummy/config/environments/test.rb | 14 +++- test/functional/modular_sinatra_test.rb | 76 ++++++++++-------- test/functional/sinatra_test.rb | 80 +++++++++++-------- 8 files changed, 141 insertions(+), 82 deletions(-) diff --git a/.travis.yml b/.travis.yml index 992008579..e7ca6acbd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,6 @@ language: ruby rvm: - - 2.0.0 + - 2.2.4 - 1.9.3 - jruby-19mode env: @@ -30,8 +30,13 @@ gemfile: matrix: fast_finish: true + allow_failures: + - gemfile: gemfiles/ar5.gemfile exclude: - gemfile: gemfiles/ar5.gemfile + rvm: 1.9.3 + - gemfile: gemfiles/ar5.gemfile + rvm: jruby-19mode addons: postgresql: "9.4" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4c08fed29..50bc5e2fc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,8 +20,15 @@ Please use our [bug report template][1]. Testing is a little awkward because the test suite: -1. contains a rails app with three databases (test, foo, and bar) -1. supports three different RDBMS': sqlite, mysql, and postgres +1. Supports three major versions of rails: 3, 4, 5 +1. Sontains a rails app with three databases (test, foo, and bar) +1. Supports three different RDBMS': sqlite, mysql, and postgres + +Test against rails 3: + +``` +bundle exec appraisal ar3 rake +``` Run tests with sqlite: diff --git a/test/dummy/app/controllers/application_controller.rb b/test/dummy/app/controllers/application_controller.rb index 72e59a053..ab1cc6918 100644 --- a/test/dummy/app/controllers/application_controller.rb +++ b/test/dummy/app/controllers/application_controller.rb @@ -1,15 +1,18 @@ class ApplicationController < ActionController::Base protect_from_forgery + # Rails 5 deprecates `before_filter` + name_of_before_callback = respond_to?(:before_action) ? :before_action : :before_filter + # Some applications and libraries modify `current_user`. Their changes need # to be reflected in `whodunnit`, so the `set_paper_trail_whodunnit` below # must happen after this. - before_filter :modify_current_user + send(name_of_before_callback, :modify_current_user) # Going forward, we'll no longer add this `before_filter`, requiring people # to do so themselves, allowing them to control the order in which this filter # happens. - before_filter :set_paper_trail_whodunnit + send(name_of_before_callback, :set_paper_trail_whodunnit) def rescue_action(e) raise e diff --git a/test/dummy/app/models/song.rb b/test/dummy/app/models/song.rb index cef53a07f..1e9409667 100644 --- a/test/dummy/app/models/song.rb +++ b/test/dummy/app/models/song.rb @@ -19,7 +19,11 @@ def attributes_with_name attributes_without_name end end - alias_method_chain :attributes, :name + + # `alias_method_chain` is deprecated in rails 5, but we cannot use the + # suggested replacement, `Module#prepend`, because we still support ruby 1.9. + alias_method :attributes_without_name, :attributes + alias_method :attributes, :attributes_with_name def changed_attributes_with_name if name @@ -28,5 +32,9 @@ def changed_attributes_with_name changed_attributes_without_name end end - alias_method_chain :changed_attributes, :name + + # `alias_method_chain` is deprecated in rails 5, but we cannot use the + # suggested replacement, `Module#prepend`, because we still support ruby 1.9. + alias_method :changed_attributes_without_name, :changed_attributes + alias_method :changed_attributes, :changed_attributes_with_name end diff --git a/test/dummy/config/application.rb b/test/dummy/config/application.rb index bd0340521..0b38150a5 100644 --- a/test/dummy/config/application.rb +++ b/test/dummy/config/application.rb @@ -51,8 +51,10 @@ class Application < Rails::Application # parameters by using an attr_accessible or attr_protected declaration. config.active_record.whitelist_attributes = false if ::PaperTrail.active_record_protected_attributes? - # Enable the asset pipeline - config.assets.enabled = false + # `config.assets` is a `NoMethodError` in rails 5. + if config.respond_to?(:assets) + config.assets.enabled = false + end # Version of your assets, change this if you want to expire all your assets # config.assets.version = '1.0' @@ -60,8 +62,14 @@ class Application < Rails::Application # Rails 4 key for generating secret key config.secret_key_base = 'A fox regularly kicked the screaming pile of biscuits.' - # supress warnings about raises in transactional callbacks on AR 4.2+ - config.active_record.raise_in_transactional_callbacks = true if ActiveRecord::VERSION::STRING >= '4.2' + # `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 + end # Set test order for Test::Unit if possible config.active_support.test_order = :sorted if config.active_support.respond_to?(:test_order=) diff --git a/test/dummy/config/environments/test.rb b/test/dummy/config/environments/test.rb index 0652e4645..563b56752 100644 --- a/test/dummy/config/environments/test.rb +++ b/test/dummy/config/environments/test.rb @@ -10,13 +10,21 @@ # Eager loads all registered namespaces config.eager_load = true - # Configure static asset server for tests with Cache-Control for performance - if config.respond_to?(:serve_static_files=) + if config.respond_to?(:public_file_server) + config.public_file_server.enabled = true + elsif config.respond_to?(:serve_static_files=) config.serve_static_files = true else config.serve_static_assets = true end - config.static_cache_control = "public, max-age=3600" + + if config.respond_to?(:public_file_server) + config.public_file_server.headers = { + 'Cache-Control' => 'public, max-age=3600' + } + else + config.static_cache_control = "public, max-age=3600" + end # Show full error reports and disable caching config.consider_all_requests_local = true diff --git a/test/functional/modular_sinatra_test.rb b/test/functional/modular_sinatra_test.rb index b56a76196..6c5e6a7c7 100644 --- a/test/functional/modular_sinatra_test.rb +++ b/test/functional/modular_sinatra_test.rb @@ -1,45 +1,55 @@ -require 'test_helper' -require 'sinatra/base' +# Our ActiveRecord 5 gemfile (see `Appraisals`) must use `rack 2.0.0.alpha` +# which renames several files that sinatra 1 depended on. Until there is +# a released version of sinatra that supports `rack 2.0.0.alpha`, we +# must exclude sinatra from our test suite. This is done in two files: +# +# - test/functional/sinatra_test.rb +# - test/functional/modular_sinatra_test.rb +# +if Gem::Version.new(Rack.release) < Gem::Version.new("2.0.0.alpha") + require 'test_helper' + require 'sinatra/base' # --- Tests for modular `Sinatra::Base` style ---- -class BaseApp < Sinatra::Base - configs = YAML.load_file(File.expand_path('../../dummy/config/database.yml', __FILE__)) - ActiveRecord::Base.configurations = configs - ActiveRecord::Base.establish_connection(:test) - register PaperTrail::Sinatra - - get '/test' do - Widget.create!(:name => 'foo') - 'Hello' - end + class BaseApp < Sinatra::Base + configs = YAML.load_file(File.expand_path('../../dummy/config/database.yml', __FILE__)) + ActiveRecord::Base.configurations = configs + ActiveRecord::Base.establish_connection(:test) + register PaperTrail::Sinatra + + get '/test' do + Widget.create!(:name => 'foo') + 'Hello' + end - def current_user - @current_user ||= OpenStruct.new(id: 'foobar') + def current_user + @current_user ||= OpenStruct.new(id: 'foobar') + end end -end -class ModularSinatraTest < ActionDispatch::IntegrationTest - include Rack::Test::Methods + class ModularSinatraTest < ActionDispatch::IntegrationTest + include Rack::Test::Methods - def app - @app ||= BaseApp - end + def app + @app ||= BaseApp + end - test 'baseline' do - assert_nil Widget.create.versions.first.whodunnit - end + test 'baseline' do + assert_nil Widget.create.versions.first.whodunnit + end - context "`PaperTrail::Sinatra` in a `Sinatra::Base` application" do + context "`PaperTrail::Sinatra` in a `Sinatra::Base` application" do - should "sets the `user_for_paper_trail` from the `current_user` method" do - get '/test' - assert_equal 'Hello', last_response.body - widget = Widget.last - assert_not_nil widget - assert_equal 'foo', widget.name - assert_equal 1, widget.versions.size - assert_equal 'foobar', widget.versions.first.whodunnit - end + should "sets the `user_for_paper_trail` from the `current_user` method" do + get '/test' + assert_equal 'Hello', last_response.body + widget = Widget.last + assert_not_nil widget + assert_equal 'foo', widget.name + assert_equal 1, widget.versions.size + assert_equal 'foobar', widget.versions.first.whodunnit + end + end end end diff --git a/test/functional/sinatra_test.rb b/test/functional/sinatra_test.rb index 25675c215..095218f9c 100644 --- a/test/functional/sinatra_test.rb +++ b/test/functional/sinatra_test.rb @@ -1,46 +1,56 @@ -require 'test_helper' -# require 'sinatra/main' - -# --- Tests for non-modular `Sinatra::Application` style ---- -class Sinatra::Application - configs = YAML.load_file(File.expand_path('../../dummy/config/database.yml', __FILE__)) - ActiveRecord::Base.configurations = configs - ActiveRecord::Base.establish_connection(:test) - register PaperTrail::Sinatra # we shouldn't actually need this line if I'm not mistaken but the tests seem to fail without it ATM - - get '/test' do - Widget.create!(:name => 'bar') - 'Hai' - end +# Our ActiveRecord 5 gemfile (see `Appraisals`) must use `rack 2.0.0.alpha` +# which renames several files that sinatra 1 depended on. Until there is +# a released version of sinatra that supports `rack 2.0.0.alpha`, we +# must exclude sinatra from our test suite. This is done in two files: +# +# - test/functional/sinatra_test.rb +# - test/functional/modular_sinatra_test.rb +# +if Gem::Version.new(Rack.release) < Gem::Version.new("2.0.0.alpha") + require 'test_helper' + # require 'sinatra/main' + + # --- Tests for non-modular `Sinatra::Application` style ---- + class Sinatra::Application + configs = YAML.load_file(File.expand_path('../../dummy/config/database.yml', __FILE__)) + ActiveRecord::Base.configurations = configs + ActiveRecord::Base.establish_connection(:test) + register PaperTrail::Sinatra # we shouldn't actually need this line if I'm not mistaken but the tests seem to fail without it ATM + + get '/test' do + Widget.create!(:name => 'bar') + 'Hai' + end + + def current_user + @current_user ||= OpenStruct.new(id: 'raboof') + end - def current_user - @current_user ||= OpenStruct.new(id: 'raboof') end -end + class SinatraTest < ActionDispatch::IntegrationTest + include Rack::Test::Methods -class SinatraTest < ActionDispatch::IntegrationTest - include Rack::Test::Methods + def app + @app ||= Sinatra::Application + end - def app - @app ||= Sinatra::Application - end + test 'baseline' do + assert_nil Widget.create.versions.first.whodunnit + end - test 'baseline' do - assert_nil Widget.create.versions.first.whodunnit - end + context "`PaperTrail::Sinatra` in a `Sinatra::Application` application" do - context "`PaperTrail::Sinatra` in a `Sinatra::Application` application" do + should "sets the `user_for_paper_trail` from the `current_user` method" do + get '/test' + assert_equal 'Hai', last_response.body + widget = Widget.last + assert_not_nil widget + assert_equal 'bar', widget.name + assert_equal 1, widget.versions.size + assert_equal 'raboof', widget.versions.first.whodunnit + end - should "sets the `user_for_paper_trail` from the `current_user` method" do - get '/test' - assert_equal 'Hai', last_response.body - widget = Widget.last - assert_not_nil widget - assert_equal 'bar', widget.name - assert_equal 1, widget.versions.size - assert_equal 'raboof', widget.versions.first.whodunnit end - end end