On Wed, Aug 22, 2012 at 12:56 AM, Bas Vodde <b...@odd-e.com> wrote:
>
> Hiya all,
>
> I was trying to get and_raise to raise an exception filled with a message and 
> I was struggling with the API for a while (not on the latest RSpec, but 
> assume it didn't change).
>
> Based on that, I have a suggestion for improvement. My first attempt was to 
> mirror how I use raise, so I tried:
>
>     subject.should_receive(:do_system).and_raise(Osaka::SystemCommandFailed, 
> "execution error: System Events got an error: Access for assistive devices is 
> disabled. (-25211)")
>
> This didn't work as the and_raise only has one parameter. Eventually I 
> figured out this works:
>
>     
> subject.should_receive(:do_system).and_raise(Osaka::SystemCommandFailed.new 
> ("execution error: System Events got an error: Access for assistive devices 
> is disabled. (-25211)"))
>
> And it does what I want it to do. Still… I would have prefered the first one 
> to work too :) Implementing that ought to be reasonably trivial, correct?
> (didn't implement it myself yet this time).
>
> Comments? Or is there anyone way of doing this?
>
> Thanks!
>
> Bas

Documentation here:
http://rubydoc.info/gems/rspec-mocks/RSpec/Mocks/MessageExpectation#and_raise-instance_method.

That has been the API for something like 5 years so I'm not sure I
want to change it. It is used to prepare an exception to raise, not
compare with one that was already raised, so we'd have to support n
args, e.g.

and_raise(AnException, with_one_arg)
and_raise(AnException, with_one_arg, and_another_arg)

etc. I think this would be friendly, but it might also be confusing
because we'd effectively have 2 completely different APIs for the same
method. Also, I'm not sure that either of those examples are much
better than:

and_raise(AnException.new with_one_arg)
and_raise(AnException.new with_one_arg, and_another_arg)

I'm open to the idea though for one reason: the rspec-expectations
raise_exception method can accept 1 or two args, so I can imagine
something like:

describe Foo do
  it "raises when x happens" do
    foo = Foo.new(Bar.new)
    expect { Foo.do_something_bad }.to raise_exception(FooException,
"with this message")
  end

  it "does x when its bar raises" do
    bar = double
    foo = Foo.new(bar)
    bar.should_receive(:msg).and_raise(BarException, "with this message")
  end
end

Your suggestion makes these two APIs align better if the exception
initializer accepts one argument and raise_exception gets a String.
But raise_exception can also take a regex, and exception initializers
can take more than one arg. Consider: InsufficientFunds.new(50, :USD).
The way things are today, you might see these two near each other
(hopefully not in the same example):

  expect { transfer.execute }.to raise_exception(InsufficientFunds, /50 USD/)
  source_account.should_receive(:withdraw).and_raise(InsufficientFunds.new
50, :USD)

With what you propose it would be this:

  expect { transfer.execute }.to raise_exception(InsufficientFunds, /50 USD/)
  source_account.should_receive(:withdraw).and_raise(InsufficientFunds,
50, :USD)

I think the way it is now tells a better story about what's really
going on (i.e. that you're supplying an initialized object to
and_raise) than the proposed change. But that's one opinion.

Any others out there?
_______________________________________________
rspec-users mailing list
rspec-users@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users

Reply via email to