"Tricks" for testing after_create callback?

rykkers · September 21, 2018, 2:20am

I’ve got a model Message, which needs to send an email using action
mailer after it’s first saved in the database.

I want to pass the model to the mailer which then uses methods on the
message model to render the email.

So the natural way to do this is in an after_create callback on the
Message model.

But I can’t see an easy way to test this. Here’s my spec

describe Message, “from anyone” do

it “should be sent on save” do
msg_creation_parms = {
:subject => “Subj”,
:body => “hi”,
:sender => people(:rick),
:recipient => people(:john)
}
SantasMailbox.should_receive(:deliver_secret_santa).with(Message.new(msg_creation_parms))
Message.create(msg_creation_parms)
end

end

This fails, but only because the model object has an id and time
stamps assigned as it’s saved.

Spec::Mocks::MockExpectationError in ‘Message from anyone should be
sent on save’
Mock ‘Class’ expected :deliver_secret_santa with (#<Message id: nil,
subject: “Subj”, body: “hi”, sender_id: 343839476, recipient_id:
21341157, message_type: nil, created_at: nil, updated_at: nil>) but
received it with (#<Message id: 1, subject: “Subj”, body: “hi”,
sender_id: 343839476, recipient_id: 21341157, message_type: nil,
created_at: “2007-12-12 21:53:16”, updated_at: “2007-12-12 21:53:16”>)

I figured I’d through this out to the list for ideas on how best to
approach this before I go to bed and sleep on it.

–
Rick DeNatale

My blog on Ruby
http://talklikeaduck.denhaven2.com/

rykkers · September 22, 2018, 12:13am

On Dec 12, 2007 9:09 PM, Rick DeNatale [email protected] wrote:

Message.create(msg_creation_parms)
subject: “Subj”, body: “hi”, sender_id: 343839476, recipient_id:
21341157, message_type: nil, created_at: nil, updated_at: nil>) but
received it with (#<Message id: 1, subject: “Subj”, body: “hi”,
sender_id: 343839476, recipient_id: 21341157, message_type: nil,
created_at: “2007-12-12 21:53:16”, updated_at: “2007-12-12 21:53:16”>)

I figured I’d through this out to the list for ideas on how best to
approach this before I go to bed and sleep on it.

Someone else may have a more elegant approach, but the block syntax to
should_receive() could allow something like:

expected_message = Message.new(msg_creation_params)
SantasMailbox.should_receive(:deliver_secret_santa) do |msg|
msg.body.should == expected_message.body
msg.subject.should == expected_message.subject

etc. if it’s necessary

end
Message.create(msg_creation_params)

Feels clunky, though.

Kyle

rykkers · September 22, 2018, 12:13am

On Dec 12, 2007 7:09 PM, Rick DeNatale [email protected] wrote:

Message.create(msg_creation_parms)
subject: “Subj”, body: “hi”, sender_id: 343839476, recipient_id:

My blog on Ruby
http://talklikeaduck.denhaven2.com/

rspec-users mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/rspec-users

I would not mock the call, and would instead just let the mailer do
its thing. You can verify that a message was sent, match the
subject/content, etc. It’s very lightweight so there’s no reason not
to use it.

Pat

rykkers · September 22, 2018, 12:13am

On 12/12/07, Pat M. [email protected] wrote:

I would not mock the call, and would instead just let the mailer do
its thing. You can verify that a message was sent, match the
subject/content, etc. It’s very lightweight so there’s no reason not
to use it.

The problem with this is that it combines testing the sending of the
message with testing how it’s rendered.

I prefer to do the latter in the context of testing the mailer.

–
Rick DeNatale

My blog on Ruby
http://talklikeaduck.denhaven2.com/

rykkers · September 22, 2018, 12:13am

On Dec 12, 2007 9:48 PM, Kyle H. [email protected] wrote:

  }
sent on save’
Someone else may have a more elegant approach, but the block syntax to
should_receive() could allow something like:
expected_message = Message.new(msg_creation_params)
SantasMailbox.should_receive(:deliver_secret_santa) do |msg|
msg.body.should == expected_message.body
msg.subject.should == expected_message.subject

etc. if it’s necessary

end
Message.create(msg_creation_params)

Not only could it support this, it DOES.

That said, I’d go for a lesser known feature: custom mock argument
matchers. Something like this (completely off the top of my head and
not tested or guaranteed bug-free - but this will give you the idea):

class EquivalentMessage
def initialize(message)
@message = message
end

def ==(other)
other.subject == @message.subject &&
other.body == @message.body &&
other.sender == @message.sender &&
other.recipient == @message.recipient
end
end

def message_equivalent_to(message)
EquivalentMessage.new(message)
end

it “should be sent on save” do
msg_creation_parms = {
:subject => “Subj”,
:body => “hi”,
:sender => people(:rick),
:recipient => people(:john)
}
SantasMailbox.should_receive(:deliver_secret_santa).
with(message_equivalent_to(Message.new(msg_creation_parms)))
Message.create(msg_creation_parms)
end

Try that out and see what you think.

Cheers,
David

rykkers · September 22, 2018, 12:13am

On 12/13/07, Daniel T. [email protected] wrote:

sent. Basically, I think the Mailer is a different tier.
I’m not testing the validity of the email, but.

In this case the email which gets send depends largely on what’s in
the message object. The mailer extracts attributes from the object to
create the email, so it’s crucial that the correct message is passed
and that’s what I’m trying to spec.

The actual contents of the message are specified in tests against the
mailer as I indicated in my earlier reply to Pat.

–
Rick DeNatale

My blog on Ruby
http://talklikeaduck.denhaven2.com/

rykkers · September 22, 2018, 12:13am

Hi Rick,

I’m probably a heretic on this point, but I would test
that :deliver_xyz is being called but not specify what parameters
it’s called with.

What’s my reasoning?

What I’m really testing in the Message spec is not the validity of
the email that’s being sent, but the fact that an email is being
sent. Basically, I think the Mailer is a different tier.
I have integration tests that ensure that the system is working as
a whole. If the params are wrong in a business-meaningful way, those
should tell me what’s wrong
Possibly most important point:
How is this piece of code likely to break? The most likely way that
this code would break would be if I change the mailer method so that
it requires different parameters. Will the spec the way you’re
attempting to write it catch that? Nope. Only the integration test
will catch that. What will you catch by specifying the exact
parameters that the mailer is called with, then? Not much - typos in
the after_create callback, perhaps. As it happens, that would also be
caught by the integration test.

I like to focus my speccing effort on where the errors are likely to
occur.

As such, my approach would be to simplify the spec to:

describe Message, “from anyone” do

it “should send an email on save” do
msg_creation_parms = {
:subject => “Subj”,
:body => “hi”,
:sender => people(:rick),
:recipient => people(:john)
}
SantasMailbox.should_receive(:deliver_secret_santa)
Message.create(msg_creation_parms)
end

end

But, as I said, I’m probably a heretic Others will probably
disagree violently (and they’ll probably be right).

I hope this helps,

Daniel

PS: If I decided that the mailer is actually the same tier as the
model that’s calling it, then I would not mock any of it, and do what
Pat suggested. However, at the moment, I tend to see mailing as a
separate tier from BL/CRUD.

rykkers · September 22, 2018, 12:13am

On 12/13/07, David C. [email protected] wrote:

other.subject == @message.subject &&
it “should be sent on save” do

Try that out and see what you think.

I like this but, I’m running into a snag or two.

When I tried this as-is I’m getting a no method exception in
EquivalentMessage#== because it’s running into a case where other is
:no_args. I put in some tracing and determined that it was also
getting other with the right message.

So (biting my lip because I don’t like class tests I changed this to:

def ==(other)
Message === other &&
other.subject == @message.subject &&
other.body == @message.body &&
other.sender == @message.sender &&
other.recipient == @message.recipient
end

Now it fails with:

Spec::Mocks::MockExpectationError in ‘Message from anyone should be
sent on save’
Mock ‘Class’ expected :deliver_secret_santa with
(#<EquivalentMessage:0x3519bec @message=#<Message id: nil, subject:
“Subj”, body: “hi”, sender_id: 343839476, recipient_id: 21341157,
message_type: 3, created_at: nil, updated_at: nil>>) once, but
received it twice

Now I’m interpreting this to mean that

the deliver_secret_santa message actually got called at least once
with no arguments, hence the :no_args

and

It got called twice with matching args.

As to the second, there’s only one call to that method, in the
after_create call-back, and it passes the message as an argument.

So I put some tracing into the callback to print out a back trace, and
it does seem to be called twice, BUT it also seems that the
Message.create call IN THE SPEC, is being called twice?!?

**** after_create #Message:0x3385808
/Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/callbacks.rb:311:in
call' /Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/callbacks.rb:311:incallback’
/Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/callbacks.rb:304:in
each' /Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/callbacks.rb:304:incallback’
/Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/callbacks.rb:227:in
create_without_timestamps' /Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/timestamp.rb:29:increate’
/Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/base.rb:2165:in
create_or_update_without_callbacks' /Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/callbacks.rb:213:increate_or_update’
/Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/base.rb:1899:in
save_without_validation' /Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/validations.rb:901:insave_without_transactions’
/Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/transactions.rb:108:in
save' /Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:66:intransaction’
/Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/transactions.rb:80:in
transaction' /Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/transactions.rb:100:intransaction’
/Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/transactions.rb:108:in
save' /Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/transactions.rb:120:inrollback_active_record_state!’
/Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/transactions.rb:108:in
save' /Users/rick/ssanta/vendor/rails/activerecord/lib/active_record/base.rb:522:increate’
./spec/models/message_spec.rb:177
/Users/rick/ssanta/vendor/plugins/rspec/lib/spec/example/example.rb:18:in
instance_eval' /Users/rick/ssanta/vendor/plugins/rspec/lib/spec/example/example.rb:18:inrun_in’
/Users/rick/ssanta/vendor/plugins/rspec/lib/spec/matchers.rb:143:in
capture_generated_description' /Users/rick/ssanta/vendor/plugins/rspec/lib/spec/example/example.rb:17:inrun_in’
/Users/rick/ssanta/vendor/plugins/rspec/lib/spec/example/example_methods.rb:14:in
execute' /opt/local/lib/ruby/1.8/timeout.rb:48:intimeout’
/Users/rick/ssanta/vendor/plugins/rspec/lib/spec/example/example_methods.rb:11:in
execute' /Users/rick/ssanta/vendor/plugins/rspec/lib/spec/example/example_group_methods.rb:260:inexecute_examples’
/Users/rick/ssanta/vendor/plugins/rspec/lib/spec/example/example_group_methods.rb:258:in
each' /Users/rick/ssanta/vendor/plugins/rspec/lib/spec/example/example_group_methods.rb:258:inexecute_examples’
/Users/rick/ssanta/vendor/plugins/rspec/lib/spec/example/example_group_methods.rb:115:in
run' /Users/rick/ssanta/vendor/plugins/rspec/lib/spec/runner/example_group_runner.rb:22:inrun’
/Users/rick/ssanta/vendor/plugins/rspec/lib/spec/runner/example_group_runner.rb:21:in
each' /Users/rick/ssanta/vendor/plugins/rspec/lib/spec/runner/example_group_runner.rb:21:inrun’
/Users/rick/ssanta/vendor/plugins/rspec/lib/spec/runner/options.rb:86:in
run_examples' /Users/rick/ssanta/vendor/plugins/rspec/lib/spec/runner/command_line.rb:19:inrun’
/Users/rick/ssanta/vendor/plugins/rspec/bin/spec:3

Not sure why this is happening, but it seems to be the rspec machinery
that’s doing it unless I’m missing something.

Here’s the complete description from the spec:

class EquivalentMessage
def initialize(msg)
@message = msg
end

def ==(other)
Message === other &&
other.subject == @message.subject &&
other.body == @message.body &&
other.sender == @message.sender &&
other.recipient == @message.recipient
end
end

def message_equivalent_to(message)
EquivalentMessage.new(message)
end

describe Message, “from anyone” do

t.string :subject

t.text :body

t.integer :from_id

t.integer :to_id

t.integer :type # Normal, FromSecretSanta, ToSecretSanta

fixtures :people

before(:each) do
end

it “should be sent on save” do
msg_creation_parms = {
:subject => “Subj”,
:body => “hi”,
:sender => people(:rick),
:recipient => people(:john),
:message_type => Message::FromSecretSanta
}
SantasMailbox.should_receive(:deliver_secret_santa).with(message_equivalent_to(Message.new(msg_creation_parms)))
Message.create(msg_creation_parms)
end

end

Rick DeNatale

My blog on Ruby
http://talklikeaduck.denhaven2.com/

rykkers · September 22, 2018, 12:13am

Hi Rick,

Could you paste us what your after_save and your deliver_xyz methods
look like? I think it would be helpful to make the discussion more
concrete.

I’ve found that whenever my code is hard to spec, it’s usually poorly
designed in the first place, and a better split of responsibilities
helps make the code both clearer and easily spec’ed.

In this specific case, I’m a bit perplexed that you’re sending a pre-
generated body/subject/sender/etc to the Mailer. Shouldn’t that code
be in the Mailer’s deliver_xyz method?

E.g., from my own code:

password_change_observer.rb

class PasswordChangeObserver < ActiveRecord::Observer
observe User

def after_update(user)
if user.password_modified?
UserMailer.deliver_new_password(user)
end
end

end

user_mailer.rb

class UserMailer < ActionMailer::Base

def new_password(user)
@subject = ‘Your new password’
@body = {“user” => user}
@recipients = user.email
@sent_on = Time.now
@from = MAIL_FROM
end

end

##new_password.erb
Dear user,

You have just updated your password. This is a reminder, so that you
don’t forget it.

Your username (email) is: <%= @user.email %>
Your password is: <%= @user.password %>

If you lose this email and forget your password, don’t worry - you
can get a new password
generated by going to http://<%= SERVER_NAME %>/forgot_password/<%=
@user.id %>"> and entering your email.

Thanks,

The Team

I’m a bit confused because your code seems to say:
msg_creation_parms = {
:subject => “Subj”,
:body => “hi”,
:sender => people(:rick),
:recipient => people(:john)
}
SantasMailbox.should_receive(:deliver_secret_santa).with(Message.new
(msg_creation_parms))

Which would imply you’re building the subject, body, sender and
recipient before passing things on to the mailer? Maybe I got this
all wrong though…

Daniel

rykkers · September 22, 2018, 12:13am

On 12/13/07, Rick DeNatale [email protected] wrote:

class Message < ActiveRecord::Base

belongs_to :sender, :class_name => “Person”, :foreign_key => :sender_id
belongs_to :recipient, :class_name => “Person”, :foreign_key => :recipient_id

after_create do |msg|
puts “\n**** after_create #{msg}\n#{caller.join(”\n")}\n"

Of course that puts isn’t really there, it’s a remnant of my attempts
to debug David’s suggestion.

–
Rick DeNatale

My blog on Ruby
http://talklikeaduck.denhaven2.com/

rykkers · September 22, 2018, 12:13am

Then I definitely think there’s little point in specifying what
“deliver_secret_santa” is called with. The chances that you’ll get
that wrong are extremely small, and even if you do it will be picked
up instantly by your integration tests, and be very easy to fix.

What’s much more likely to break is SantasMailbox - so focus the
time, the effort, and the lines of code on speccing that quite
thoroughly, so you know it can take every likely Message that you
might throw at it.

(/opinion)

Daniel

rykkers · September 22, 2018, 12:13am

On 12/13/07, Daniel T. [email protected] wrote:

(/opinion)
Well IMHO, I think that it’s important to spec both. Even though the
callback is simple, it’s a crucial design point, and I’m not unknown
to break such assumptions as code progresses.

It shouldn’t be so hard to do either, I just can’t figure out now,
why rspec seems to be calling the create in my spec twice.

–
Rick DeNatale

My blog on Ruby
http://talklikeaduck.denhaven2.com/

rykkers · September 22, 2018, 12:13am

On Dec 13, 2007 4:57 AM, Rick DeNatale [email protected] wrote:

I prefer to do the latter in the context of testing the mailer.
So then I would write two specs. One verifies that it was sent, and
one verifies the content. You’re combining them here as well, albeit
in a less flexible (and apparently painful way.

Pat

rykkers · September 22, 2018, 12:13am

On 12/13/07, Daniel T. [email protected] wrote:

In this specific case, I’m a bit perplexed that you’re sending a pre-
generated body/subject/sender/etc to the Mailer. Shouldn’t that code
be in the Mailer’s deliver_xyz method?

In this case the bulk of the message is coming from, surprise, the
message object.

This is a secret santa site, the idea is to allow folks to communicate
without giving away who is who.

class Message < ActiveRecord::Base

belongs_to :sender, :class_name => “Person”, :foreign_key =>
:sender_id
belongs_to :recipient, :class_name => “Person”, :foreign_key =>
:recipient_id

after_create do |msg|
puts “\n**** after_create #{msg}\n#{caller.join(”\n")}\n"

SantasMailbox.deliver_secret_santa(msg)

end
…
end

class SantasMailbox < ActionMailer::Base
…
def secret_santa(message)
subject message.subject
body( {:sender => message.sender_name,
:recipient => message.recipient_first_name, :text
=> body})
recipients “#{recipient.full_name} <#{recipient.email}>”
from “#{message.sender_full_name}
<#{message.sender_name}@denhaven2.com”
sent_on message.created_at
content_type ‘text/html’
headers {}
end
…
end

Where methods like sender_name etc. on the Message object produce
appropriate clear or obfuscated (e.g. ‘Your Secret Santa’ depending
on the message state.

–
Rick DeNatale

My blog on Ruby
http://talklikeaduck.denhaven2.com/

rykkers · September 22, 2018, 12:13am

On Dec 13, 2007 8:59 AM, Rick DeNatale [email protected] wrote:

(/opinion)

Well IMHO, I think that it’s important to spec both. Even though the
callback is simple, it’s a crucial design point, and I’m not unknown
to break such assumptions as code progresses.

It shouldn’t be so hard to do either, I just can’t figure out now,
why rspec seems to be calling the create in my spec twice.

This is a bit scary

Rick - would you mind sticking this information, a bit filtered, into
the tracker?

rykkers · September 22, 2018, 12:13am

On 12/13/07, David C. [email protected] wrote:

On Dec 13, 2007 8:59 AM, Rick DeNatale [email protected] wrote:

On 12/13/07, Daniel T. [email protected] wrote:

Then I definitely think there’s little point in specifying what
“deliver_secret_santa” is called with. The chances that you’ll get
that wrong are extremely small, and even if you do it will be picked
up instantly by your integration tests, and be very easy to fix.

Rick - would you mind sticking this information, a bit filtered, into
the tracker?

Well, as I was doing just that, I looked a bit closer at those
walkbacks, and it occurred to me that I was assuming that rspec was
calling the method twice.

As it turns out, it was actually ActiveRecord calling the callback
twice, and it went away when I changed from

after_create do |msg|
…
end

to

def after_create
…
end

So I guess I need to follow up on Rails core! Although that change
fixed things for this case.

And it just goes to show that it was important to spec this, since it
would be a bad thing ™ if the email got sent twice.

–
Rick DeNatale

My blog on Ruby
http://talklikeaduck.denhaven2.com/

rykkers · September 22, 2018, 12:13am

On 12/13/07, Rick DeNatale [email protected] wrote:

On 12/13/07, David C. [email protected] wrote:

twice, and it went away when I changed from

So I guess I need to follow up on Rails core! Although that change
fixed things for this case.

Okay, more info. As I was investigating this for a possible ticket
for Rails Trac, it dawned on me that maybe, just maybe, the
after_create do…end was being executed twice. So I added a puts
“**** loading Message” in the class definition for the Message AR
class, and sure enough it was being loaded twice!

It turns out that I had

require ‘message’

at the top of the spec file. and a few other descriptions in the same
file which were using the messages fixture.

When I remove the require ‘message’ from my spec file, it seems to
work with either form of the callback definition.

So I’m not sure if this is an Rspec bug, an AR fixtures bug, both or
neither. <G?>

Rick DeNatale

My blog on Ruby
http://talklikeaduck.denhaven2.com/

"Tricks" for testing after_create callback?

etc. if it’s necessary

etc. if it’s necessary

t.string :subject

t.text :body

t.integer :from_id

t.integer :to_id

t.integer :type # Normal, FromSecretSanta, ToSecretSanta

end

password_change_observer.rb

user_mailer.rb

So I’m not sure if this is an Rspec bug, an AR fixtures bug, both or neither. <G?>

So I’m not sure if this is an Rspec bug, an AR fixtures bug, both or
neither. <G?>