rspec users - Dec 2007 - "Tricks" for testing after_create callback???

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. <G>

-- 
Rick DeNatale

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

On Dec 12, 2007 9:09 PM, Rick DeNatale <rick.denatale at gmail.com>
wrote:
> 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. <G>
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

On Dec 12, 2007 7:09 PM, Rick DeNatale <rick.denatale at gmail.com>
wrote:
> 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. <G>
>
> --
> Rick DeNatale
>
> My blog on Ruby
> http://talklikeaduck.denhaven2.com/
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> 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

On Dec 12, 2007 9:48 PM, Kyle Hargraves <philodespotos at gmail.com>
wrote:
>
> On Dec 12, 2007 9:09 PM, Rick DeNatale <rick.denatale at gmail.com>
wrote:
> > 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. <G>
>
> 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


>
> Feels clunky, though.
>
> Kyle
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

On 12/12/07, Pat Maddox <pergesu at gmail.com> 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/

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.

On 13 Dec 2007, at 12:57 13 Dec 2007, Rick DeNatale wrote:
> On 12/12/07, Pat Maddox <pergesu at gmail.com> 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/
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users

On 12/13/07, David Chelimsky <dchelimsky at gmail.com> wrote:
> 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.
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

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

and

2) 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:in
`callback''
/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:in
`callback''
/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:in
`create''
/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:in
`create_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:in
`save_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:in
`transaction''
/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:in
`transaction''
/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:in
`rollback_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:in
`create''
./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:in
`run_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:in
`run_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:in `timeout''
/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:in
`execute_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:in
`execute_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:in
`run''
/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:in
`run''
/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:in
`run''
/Users/rick/ssanta/vendor/plugins/rspec/bin/spec:3

**** 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:in
`callback''
/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:in
`callback''
/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:in
`create''
/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:in
`create_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:in
`save_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:in
`transaction''
/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:in
`transaction''
/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:in
`rollback_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:in
`create''
./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:in
`run_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:in
`run_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:in `timeout''
/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:in
`execute_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:in
`execute_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:in
`run''
/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:in
`run''
/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:in
`run''
/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/

On 12/13/07, Daniel Tenner <daniel.ruby at tenner.org>
wrote:
> 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''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/

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

On 13 Dec 2007, at 13:43 13 Dec 2007, Rick DeNatale wrote:
> On 12/13/07, Daniel Tenner <daniel.ruby at tenner.org> wrote:
>> 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''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/
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users

On 12/13/07, Daniel Tenner <daniel.ruby at tenner.org>
wrote:
> 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?
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/

On 12/13/07, Rick DeNatale <rick.denatale at gmail.com> 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/

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

On 13 Dec 2007, at 14:44 13 Dec 2007, Rick DeNatale wrote:
> On 12/13/07, Daniel Tenner <daniel.ruby at tenner.org> wrote:
>> 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?
>
> 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/
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users

On 12/13/07, Daniel Tenner <daniel.ruby at tenner.org>
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.
>
> 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)
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/

On Dec 13, 2007 4:57 AM, Rick DeNatale <rick.denatale at gmail.com>
wrote:
> On 12/12/07, Pat Maddox <pergesu at gmail.com> 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.
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

On Dec 13, 2007 8:59 AM, Rick DeNatale <rick.denatale at gmail.com>
wrote:
> On 12/13/07, Daniel Tenner <daniel.ruby at tenner.org> 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.
> >
> > 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)
>
> 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?
>
>
> --
> Rick DeNatale
>
> My blog on Ruby
> http://talklikeaduck.denhaven2.com/
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

On 12/13/07, David Chelimsky <dchelimsky at gmail.com>
wrote:
> On Dec 13, 2007 8:59 AM, Rick DeNatale <rick.denatale at gmail.com>
wrote:
> > On 12/13/07, Daniel Tenner <daniel.ruby at tenner.org> 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.
> > 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?
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 (tm) if the email got sent twice.

-- 
Rick DeNatale

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

On 12/13/07, Rick DeNatale <rick.denatale at gmail.com>
wrote:
> On 12/13/07, David Chelimsky <dchelimsky at gmail.com> wrote:
> > This is a bit scary :(
> >
> > 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.
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/