From 509221c1e0f62c7b0fbb62f2a8e31275c1a4be87 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Thu, 24 Dec 2015 12:21:51 -0600 Subject: [PATCH] Only call railtie when Rails is defined; assume controller loaded Isolated Testing - Rake test inspired by https://github.com/rails/rails/blob/v5.0.0.beta1/activejob/Rakefile - Isolated unit inspired by - https://github.com/rails/rails/blob/v5.0.0.beta1/railties/test/isolation/abstract_unit.rb - https://github.com/rails/rails/blob/v5.0.0.beta1/activemodel/test/cases/railtie_test.rb Misc - Turns out `mattr_accessor(:logger) { ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(STDOUT)) }` was always nil until the Railtie was loaded, since mattr_accessor block defaults don't really work on modules, but on the classes that include them. - Commented on important on Rails being required first for caching to work. - In isolated tests, `active_support/core_ext/object/with_options` is required. --- Rakefile | 27 ++++++- lib/active_model_serializers.rb | 9 +-- lib/active_model_serializers/railtie.rb | 27 ++++--- .../railtie_test_isolated.rb | 69 +++++++++++++++++ test/support/isolated_unit.rb | 77 +++++++++++++++++++ 5 files changed, 194 insertions(+), 15 deletions(-) create mode 100644 test/active_model_serializers/railtie_test_isolated.rb create mode 100644 test/support/isolated_unit.rb diff --git a/Rakefile b/Rakefile index 58a49598..4106d987 100644 --- a/Rakefile +++ b/Rakefile @@ -44,7 +44,32 @@ Rake::TestTask.new do |t| t.verbose = true end -task default: [:test, :rubocop] +desc 'Run isolated tests' +task isolated: ['test:isolated:railtie'] +namespace :test do + namespace :isolated do + desc 'Run isolated tests for Railtie' + task :railtie do + dir = File.dirname(__FILE__) + file = "#{dir}/test/active_model_serializers/railtie_test_isolated.rb" + + # https://github.com/rails/rails/blob/3d590add45/railties/lib/rails/generators/app_base.rb#L345-L363 + _bundle_command = Gem.bin_path('bundler', 'bundle') + require 'bundler' + Bundler.with_clean_env do + command = "-w -I#{dir}/lib -I#{dir}/test #{file}" + full_command = %("#{Gem.ruby}" #{command}) + system(full_command) or fail 'Failures' # rubocop:disable Style/AndOr + end + end + end +end + +if ENV['RAILS_VERSION'].to_s > '4.0' && RUBY_ENGINE == 'ruby' + task default: [:isolated, :test, :rubocop] +else + task default: [:test, :rubocop] +end desc 'CI test task' task :ci => [:default] diff --git a/lib/active_model_serializers.rb b/lib/active_model_serializers.rb index 3562f34a..d92823b5 100644 --- a/lib/active_model_serializers.rb +++ b/lib/active_model_serializers.rb @@ -1,7 +1,6 @@ require 'active_model' require 'active_support' -require 'action_controller' -require 'action_controller/railtie' +require 'active_support/core_ext/object/with_options' module ActiveModelSerializers extend ActiveSupport::Autoload autoload :Model @@ -10,7 +9,8 @@ module ActiveModelSerializers autoload :Logging autoload :Test - mattr_accessor(:logger) { ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(STDOUT)) } + class << self; attr_accessor :logger; end + self.logger = ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(STDOUT)) def self.config ActiveModel::Serializer.config @@ -18,7 +18,6 @@ module ActiveModelSerializers require 'active_model/serializer/version' require 'active_model/serializer' - require 'active_model_serializers/railtie' require 'active_model/serializable_resource' - require 'action_controller/serialization' + require 'active_model_serializers/railtie' if defined?(::Rails) end diff --git a/lib/active_model_serializers/railtie.rb b/lib/active_model_serializers/railtie.rb index 6d2ddfb6..02852624 100644 --- a/lib/active_model_serializers/railtie.rb +++ b/lib/active_model_serializers/railtie.rb @@ -1,4 +1,7 @@ require 'rails/railtie' +require 'action_controller' +require 'action_controller/railtie' +require 'action_controller/serialization' module ActiveModelSerializers class Railtie < Rails::Railtie @@ -7,10 +10,8 @@ module ActiveModelSerializers end initializer 'active_model_serializers.action_controller' do - ActiveSupport.on_load(:action_controller) do - ActiveSupport.run_load_hooks(:active_model_serializers, ActiveModelSerializers) - include ::ActionController::Serialization - end + ActiveSupport.run_load_hooks(:active_model_serializers, ActiveModelSerializers) + ActionController::Base.send(:include, ::ActionController::Serialization) end initializer 'active_model_serializers.logger' do @@ -19,11 +20,19 @@ module ActiveModelSerializers end end - initializer 'active_model_serializers.caching' do - ActiveSupport.on_load(:action_controller) do - ActiveModelSerializers.config.cache_store = ActionController::Base.cache_store - ActiveModelSerializers.config.perform_caching = Rails.configuration.action_controller.perform_caching - end + # To be useful, this hook must run after Rails has initialized, + # BUT before any serializers are loaded. + # Otherwise, the call to 'cache' won't find `cache_store` or `perform_caching` + # defined, and serializer's `_cache_store` will be nil. + # IF the load order cannot be changed, then in each serializer that that defines a `cache`, + # manually specify e.g. `PostSerializer._cache_store = Rails.cache` any time + # before the serializer is used. (Even though `ActiveModel::Serializer._cache_store` is + # inheritable, we don't want to set it on `ActiveModel::Serializer` directly unless + # we want *every* serializer to be considered cacheable, regardless of specifying + # `cache # some options` in a serializer or not. + initializer 'active_model_serializers.caching' => :after_initialize do + ActiveModelSerializers.config.cache_store = ActionController::Base.cache_store + ActiveModelSerializers.config.perform_caching = Rails.configuration.action_controller.perform_caching end generators do diff --git a/test/active_model_serializers/railtie_test_isolated.rb b/test/active_model_serializers/railtie_test_isolated.rb new file mode 100644 index 00000000..60d8f900 --- /dev/null +++ b/test/active_model_serializers/railtie_test_isolated.rb @@ -0,0 +1,69 @@ +# Execute this test in isolation +require 'support/isolated_unit' + +class RailtieTest < ActiveSupport::TestCase + include ActiveSupport::Testing::Isolation + + class WithRails < RailtieTest + setup do + require 'rails' + require 'active_model_serializers' + make_basic_app + end + + test 'mixes ActionController::Serialization into ActionController::Base' do + assert ActionController.const_defined?(:Serialization), + "ActionController::Serialization should be defined, but isn't" + assert ::ActionController::Base.included_modules.include?(::ActionController::Serialization), + "ActionController::Serialization should be included in ActionController::Base, but isn't" + end + + test 'sets the ActiveModelSerializers.logger to Rails.logger' do + refute_nil Rails.logger + refute_nil ActiveModelSerializers.logger + assert_equal Rails.logger, ActiveModelSerializers.logger + end + + test 'it is configured for caching' do + assert_equal ActionController::Base.cache_store, ActiveModelSerializers.config.cache_store + assert_equal Rails.configuration.action_controller.perform_caching, ActiveModelSerializers.config.perform_caching + end + + test 'it runs the load hook' do + loaded = false + ActiveSupport.on_load(:active_model_serializers) { loaded = true } + assert loaded + end + end + + class WithoutRails < RailtieTest + setup do + require 'active_model_serializers' + make_basic_app + end + + test 'does not mix ActionController::Serialization into ActionController::Base' do + refute ActionController.const_defined?(:Serialization), + 'ActionController::Serialization should not be defined, but is' + end + + test 'has its own logger at ActiveModelSerializers.logger' do + refute_nil Rails.logger + refute_nil ActiveModelSerializers.logger + refute_equal Rails.logger, ActiveModelSerializers.logger + end + + test 'it is not configured for caching' do + refute_nil ActionController::Base.cache_store + assert_nil ActiveModelSerializers.config.cache_store + refute Rails.configuration.action_controller.perform_caching + refute ActiveModelSerializers.config.perform_caching + end + + test "it hasn't run the load hook" do + loaded = false + ActiveSupport.on_load(:active_model_serializers) { loaded = true } + refute loaded + end + end +end diff --git a/test/support/isolated_unit.rb b/test/support/isolated_unit.rb new file mode 100644 index 00000000..50362239 --- /dev/null +++ b/test/support/isolated_unit.rb @@ -0,0 +1,77 @@ +# https://github.com/rails/rails/blob/v5.0.0.beta1/railties/test/isolation/abstract_unit.rb + +# Usage Example: +# +# require 'support/isolated_unit' +# +# class RailtieTest < ActiveSupport::TestCase +# include ActiveSupport::Testing::Isolation +# +# class WithRailsDefinedOnLoad < RailtieTest +# setup do +# require 'rails' +# require 'active_model_serializers' +# make_basic_app +# end +# +# # some tests +# end +# +# class WithoutRailsDefinedOnLoad < RailtieTest +# setup do +# require 'active_model_serializers' +# make_basic_app +# end +# +# # some tests +# end +# end +# +# Note: +# It is important to keep this file as light as possible +# the goal for tests that require this is to test booting up +# rails from an empty state, so anything added here could +# hide potential failures +# +# It is also good to know what is the bare minimum to get +# Rails booted up. +require 'bundler/setup' unless defined?(Bundler) +require 'active_support' +require 'active_support/core_ext/string/access' + +# These files do not require any others and are needed +# to run the tests +require 'active_support/testing/autorun' +require 'active_support/testing/isolation' + +module TestHelpers + module Generation + # Make a very basic app, without creating the whole directory structure. + # Is faster and simpler than generating a Rails app in a temp directory + def make_basic_app + require 'rails' + require 'action_controller/railtie' + + @app = Class.new(Rails::Application) do + config.eager_load = false + config.session_store :cookie_store, key: '_myapp_session' + config.active_support.deprecation = :log + config.active_support.test_order = :parallel + ActiveSupport::TestCase.respond_to?(:test_order=) && ActiveSupport::TestCase.test_order = :parallel + config.root = File.dirname(__FILE__) + config.log_level = :info + # Set a fake logger to avoid creating the log directory automatically + fake_logger = Logger.new(nil) + config.logger = fake_logger + end + @app.respond_to?(:secrets) && @app.secrets.secret_key_base = '3b7cd727ee24e8444053437c36cc66c4' + + yield @app if block_given? + @app.initialize! + end + end +end + +class ActiveSupport::TestCase + include TestHelpers::Generation +end