Re: [rspec-users] and_raise with a message
Hola,
I went ahead and wrote some code instead of emails and implemented it and send
a pull request.
The way I implemented was just pretty simple:
- and_raise has a second parameter which is an empty string by default
- when raise_exception is called and the exception passed is a Class, then it
will pass the message
- The spec that didn't allow for exception classes with constructor arguments
has been changed to allow for 1, the message
The pull request stayed close to the original code. However, an alternative
implementation would be:
- The and_raise itself creates an object when a class is passed. This allows
for the "too much arguments" check to be done earlier and it would fail earlier.
- The raise_exception would simply raise an object (always) and never a Class
This does change the behavior from late fail to early fail. I'm not sure if
there is a good reason to the current late fail, so I sticked with the original
implementation.
Pull request done. Feel free to decline it if you want it implemented
differently. If you want to see the alternative, I can make a new pull request
with the alternative implementation.
Thanks,
Bas
ps. If you want to ignore it completely, thats also ok with me. It is not a
major point, it just was one of the surprises I had working with rspec, and
surprise usually suggest a potential API improvement :)
On 22 Aug, 2012, at 7:59 PM, David Chelimsky wrote:
> On Wed, Aug 22, 2012 at 6:55 AM, David Chelimsky wrote:
>> On Wed, Aug 22, 2012 at 6:43 AM, Bas Vodde wrote:
>>>
>>> On 22 Aug, 2012, at 7:25 PM, David Chelimsky wrote:
>>>
On Wed, Aug 22, 2012 at 12:56 AM, Bas Vodde 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(Insufficient
Re: [rspec-users] and_raise with a message
On Wed, Aug 22, 2012 at 6:55 AM, David Chelimsky wrote:
> On Wed, Aug 22, 2012 at 6:43 AM, Bas Vodde wrote:
>>
>> On 22 Aug, 2012, at 7:25 PM, David Chelimsky wrote:
>>
>>> On Wed, Aug 22, 2012 at 12:56 AM, Bas Vodde 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?
>>
>> 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
>
> Unfortunately, the Kernel#raise API also accepts a 3rd argument, which
> is an Array of caller information (the backtrace):
> http://www.ruby-doc.org/core-1.9.3/Kernel.html#method-i-raise.
>
> What that doc doesn't tell you is that the first arg can actually be
> an initialized exception:
>
> raise InsufficientFunds.new(49, :USD)
>
> I can't
Re: [rspec-users] and_raise with a message
On Wed, Aug 22, 2012 at 6:43 AM, Bas Vodde wrote:
>
> On 22 Aug, 2012, at 7:25 PM, David Chelimsky wrote:
>
>> On Wed, Aug 22, 2012 at 12:56 AM, Bas Vodde 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?
>
> 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
Unfortunately, the Kernel#raise API also accepts a 3rd argument, which
is an Array of caller information (the backtrace):
http://www.ruby-doc.org/core-1.9.3/Kernel.html#method-i-raise.
What that doc doesn't tell you is that the first arg can actually be
an initialized exception:
raise InsufficientFunds.new(49, :USD)
I can't remember where I heard this but there was some advice from a
respected software developer that said something like "APIs should be
loose with what they accept and strict with what they return." Based
on
Re: [rspec-users] and_raise with a message
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 wrote:
> On Wed, Aug 22, 2012 at 12:56 AM, Bas Vodde 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
Re: [rspec-users] and_raise with a message
On Wed, Aug 22, 2012 at 12:56 AM, Bas Vodde 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
