Hi David, My main thinking was to make it consistent with the Kernel.raise.
Like, in my production code, I have: raise Osaka::SystemCommandFailed, output_message so, it would make sense to the mock to work the same with: and_raise Osaka::SystemCommandFailed, "Fake output message" I figured it would be a relative minor change as it would add one extra parameter which could default to an empty string. (as reference, see Kernel raise: http://www.ruby-doc.org/core-1.9.3/Kernel.html) I had not considered the consistency with and_raise_exception, but I just noticed that this was one of the cases where my default excepted behavior from RSpec was different from what the API wanted. Therefore, I had to dive in the RSpec doc to see how I could pass the message…. Bas On 22 Aug, 2012, at 7:25 PM, David Chelimsky <dchelim...@gmail.com> wrote: > 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 _______________________________________________ rspec-users mailing list rspec-users@rubyforge.org http://rubyforge.org/mailman/listinfo/rspec-users
