Skip to content

Commit

Permalink
Restore test deliveries properly in ActionMailer.
Browse files Browse the repository at this point in the history 
`ActionMailer::Base.delivery_method` and
`ActionMailer::Base.perform_deliveries` have leaked states.

"delivery method can be customized per instance" and "delivery method
can be customized in subclasses not changing the parent" in
delivery_methods_test.rb will fail if test_helper_test.rb (in which
TestHelperMailerTest is inherited from ActionMailer::TestCase) runs
before it.
  • Loading branch information
@zuhao
zuhao committed Jun 6, 2014
1 parent ecd4151 commit c4f4123
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions actionmailer/lib/action_mailer/test_case.rb
View file
Expand Up @@ -20,6 +20,7 @@ module Behavior
class_attribute :_mailer_class
setup :initialize_test_deliveries
setup :set_expected_mail
teardown :restore_test_deliveries
end

module ClassMethods
Expand Down Expand Up @@ -54,8 +55,15 @@ def determine_default_mailer(name)
protected

def initialize_test_deliveries
@old_delivery_method = ActionMailer::Base.delivery_method
@old_perform_deliveries = ActionMailer::Base.perform_deliveries
ActionMailer::Base.delivery_method = :test
ActionMailer::Base.perform_deliveries = true
end

def restore_test_deliveries
ActionMailer::Base.delivery_method = @old_delivery_method
ActionMailer::Base.perform_deliveries = @old_perform_deliveries
ActionMailer::Base.deliveries.clear
end

Expand Down

1 comment on commit c4f4123

@mgodwin
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi - could more context be provided on this changeset?

I'm noticing that I'm getting intermittent test failures with this change after upgrading from rails 4.1 b/c the deliveries array isn't cleared out prior to tests running. I had written quite a few tests that asserted that the deliveries array is empty? since the guides and other docs say that the actionmailer tests are reset automatically. (Although, combing through the docs now, it doesn't seem extremely clear that it states when they're reset...)

It seems surprising to me that more people haven't run into test failures here, since if you turn on randomized test ordering the following sequence is a definite possibility:

  1. A controller test adds mail to the deliveries array
  2. The next test that runs is in a mailer test, asserts ActionMailer::Base.deliveries.empty?
  3. Failure because deliveries is not cleared in initialization anymore

I'm willing to concede that maybe I'm doing something wrong, so - should I not be using assert ActionMailer::Base.deliveries.empty? anymore?

The thought does occur to me though - it seems more problematic that the example provided in the rails guides could now result in a false positive (test passing when it should have failed), since the deliveries array may not actually be empty.

example from rails guides:

require 'test_helper'

class UserMailerTest < ActionMailer::TestCase
  test "invite" do
    # Send the email, then test that it got queued
    email = UserMailer.create_invite('me@example.com',
                                     'friend@example.com', Time.now).deliver_now
    assert_not ActionMailer::Base.deliveries.empty?

    # Test the body of the sent email contains what we expect it to
    assert_equal ['me@example.com'], email.from
    assert_equal ['friend@example.com'], email.to
    assert_equal 'You have been invited by me@example.com', email.subject
    assert_equal read_fixture('invite').join, email.body.to_s
  end
end

Please sign in to comment.