Merge pull request #1389 from rspec/backport-1388

Back port pull request #1388 from rspec/fix-regression-mailer-preview-path

Author: cupakromer

Changelog.md

@@ -1,3 +1,10 @@
+### Development
+
+Bug Fixes:
+
+* Fix regression with the railtie resulting in undefined method `preview_path=`
+  on Rails 3.x and 4.0 (Aaron Kromer, #1388)
+
 ### 3.2.2 / 2015-06-03
 [Full Changelog](http://github.com/rspec/rspec-rails/compare/v3.2.1...v3.2.2)
 

example_app_generator/generate_action_mailer_specs.rb

@@ -14,11 +14,25 @@ def comment_lines(path, flag, *args)
 
   initializer 'action_mailer.rb', <<-CODE
     if ENV['DEFAULT_URL']
-      Rails.application.configure do
-        config.action_mailer.default_url_options = { :host => ENV['DEFAULT_URL'] }
+      if ::Rails::VERSION::STRING < '4.1'
+        ExampleApp::Application.configure do
+          config.action_mailer.default_url_options = { :host => ENV['DEFAULT_URL'] }
+        end
+      else
+        Rails.application.configure do
+          config.action_mailer.default_url_options = { :host => ENV['DEFAULT_URL'] }
+        end
       end
     end
+
+    if defined?(ActionMailer)
+      # This will force the loading of ActionMailer settings
+      ActionMailer::Base.smtp_settings
+    end
   CODE
+  gsub_file 'config/initializers/action_mailer.rb',
+            /ExampleApp/,
+            Rails.application.class.parent.to_s
 
   copy_file 'spec/support/default_preview_path'
   chmod 'spec/support/default_preview_path', 0755

example_app_generator/spec/support/default_preview_path

@@ -32,6 +32,7 @@ require_file_stub 'config/environment' do
     module ExampleApp
       class Application < Rails::Application
         config.eager_load = false
+        config.eager_load_paths.clear
 
         # Don't care if the mailer can't send.
         config.action_mailer.raise_delivery_errors = false unless ENV['NO_ACTION_MAILER']
@@ -58,3 +59,7 @@ if ENV['DEFAULT_URL']
 elsif defined?(::ActionMailer::Preview)
   puts Rails.application.config.action_mailer.preview_path
 end
+
+# This will force the loading of ActionMailer settings to ensure we do not
+# accicentally set something we should not
+ActionMailer::Base.smtp_settings

example_app_generator/spec/verify_mailer_preview_path_spec.rb

@@ -8,17 +8,25 @@ def ==(str)
     end
   end
 
+  def as_commandline(ops)
+    cmd, ops = ops.reverse
+    ops ||= {}
+    cmd_parts = ops.map { |k, v| "#{k}=#{v}" } << cmd << '2>&1'
+    cmd_parts.join(' ')
+  end
+
   def capture_exec(*ops)
     io = if RUBY_VERSION.to_f < 1.9
-           IO.popen(ops.join(' '))
+           IO.popen(as_commandline(ops))
          else
            ops << { :err => [:child, :out] }
            IO.popen(ops)
          end
     # Necessary to ignore warnings from Rails code base
-    out =  io.readlines.reject { |line|
-      line =~ /warning: circular argument reference/
-    }.join.chomp
+    out =  io.readlines.
+              reject { |line| line =~ /warning: circular argument reference/ }.
+              join.
+              chomp
     CaptureExec.new(out, $?.exitstatus)
   end
 
@@ -176,8 +184,58 @@ def have_no_preview
       end
     end
   else
-    it 'handles no action mailer preview' do
-      expect(capture_exec(exec_script)).to have_no_preview
+    context 'in the development environment' do
+      let(:custom_env) { { 'RAILS_ENV' => rails_env } }
+      let(:rails_env) { 'development' }
+
+      it 'handles no action mailer preview' do
+        expect(capture_exec(custom_env, exec_script)).to have_no_preview
+      end
+
+      it 'allows initializers to set options' do
+        expect(
+          capture_exec(
+            custom_env.merge('DEFAULT_URL' => 'test-host'),
+            exec_script
+          )
+        ).to eq('test-host')
+      end
+
+      it 'handles action mailer not being available' do
+        expect(
+          capture_exec(
+            custom_env.merge('NO_ACTION_MAILER' => 'true'),
+            exec_script
+          )
+        ).to have_no_preview
+      end
+    end
+
+    context 'in a non-development environment' do
+      let(:custom_env) { { 'RAILS_ENV' => rails_env } }
+      let(:rails_env) { 'test' }
+
+      it 'handles no action mailer preview' do
+        expect(capture_exec(custom_env, exec_script)).to have_no_preview
+      end
+
+      it 'allows initializers to set options' do
+        expect(
+          capture_exec(
+            custom_env.merge('DEFAULT_URL' => 'test-host'),
+            exec_script
+          )
+        ).to eq('test-host')
+      end
+
+      it 'handles action mailer not being available' do
+        expect(
+          capture_exec(
+            custom_env.merge('NO_ACTION_MAILER' => 'true'),
+            exec_script
+          )
+        ).to have_no_preview
+      end
     end
   end
 end

lib/rspec-rails.rb

@@ -29,19 +29,12 @@ class Railtie < ::Rails::Railtie
     private
 
       def setup_preview_path(app)
-        # If the action mailer railtie isn't loaded the config will not respond
         return unless supports_action_mailer_previews?(app.config)
         options = app.config.action_mailer
         config_default_preview_path(options) if config_preview_path?(options)
       end
 
       def config_preview_path?(options)
-        # This string version check avoids loading the ActionMailer class, as
-        # would happen using `defined?`. This is necessary because the
-        # ActionMailer class only loads it's settings once, at load time. If we
-        # load the class now any settings declared in a config block in an
-        # initializer will be ignored.
-        #
         # We cannot use `respond_to?(:show_previews)` here as it will always
         # return `true`.
         if ::Rails::VERSION::STRING < '4.2'
@@ -59,8 +52,18 @@ def config_default_preview_path(options)
       end
 
       def supports_action_mailer_previews?(config)
-        config.respond_to?(:action_mailer) &&
-          config.action_mailer.respond_to?(:preview_path)
+        # These checks avoid loading `ActionMailer`. Using `defined?` has the
+        # side-effect of the class getting loaded if it is available. This is
+        # problematic because loading `ActionMailer::Base` will cause it to
+        # read the config settings; this is the only time the config is read.
+        # If the config is loaded now, any settings declared in a config block
+        # in an initializer will be ignored.
+        #
+        # If the action mailer railtie has not been loaded then `config` will
+        # not respond to the method. However, we cannot use
+        # `config.action_mailer.respond_to?(:preview_path)` here as it will
+        # always return `true`.
+        config.respond_to?(:action_mailer) && ::Rails::VERSION::STRING > '4.1'
       end
     end
   end