[rspec-users] no should raise_exception

6 views
Skip to first unread message

rogerdpack

unread,
Dec 20, 2009, 12:12:48 AM12/20/09
to rspec...@rubyforge.org
I know there's a raise_error
however...exceptions are not always errors, so I had expected a
raise_exception matcher...would this be a reasonable addition (I'd be
happy to do it).
Thanks.
-r
_______________________________________________
rspec-users mailing list
rspec...@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users

Elliot Winkler

unread,
Dec 20, 2009, 1:07:14 AM12/20/09
to rspec-users
raise_error already catches any type of exception, error or not:

  class BlahException < Exception; end
  class BlahError < StandardError; end

lambda { raise BlahException }.should raise_error(BlahException)
lambda { raise BlahError }.should raise_error(BlahError)
lambda { raise "blah" }.should raise_error(RuntimeError, "blah")

-- Elliot

Ashley Moran

unread,
Dec 20, 2009, 1:51:23 PM12/20/09
to rspec-users

On Dec 20, 2009, at 6:07 am, Elliot Winkler wrote:

> raise_error already catches any type of exception, error or not:
>
> class BlahException < Exception; end
> class BlahError < StandardError; end
>
> lambda { raise BlahException }.should raise_error(BlahException)
> lambda { raise BlahError }.should raise_error(BlahError)
> lambda { raise "blah" }.should raise_error(RuntimeError, "blah")

Although it would be unusual to catch non-Error Exceptions in most cases? Most indicate unrecoverable failure; only the SignalException looks like something you might want to catch - I don't know, though. I assume you'd normally register a handler for that.

Ashley

--
http://www.patchspace.co.uk/
http://www.linkedin.com/in/ashleymoran

rogerdpack

unread,
Dec 22, 2009, 10:33:30 AM12/22/09
to rspec...@rubyforge.org
> raise_error already catches any type of exception, error or not:
>
>   class BlahException < Exception; end
>   class BlahError < StandardError; end
>
>   lambda { raise BlahException }.should raise_error(BlahException)
>   lambda { raise BlahError }.should raise_error(BlahError)
>   lambda { raise "blah" }.should raise_error(RuntimeError, "blah")

Thanks for the response. I think my request was more of a "why call
them errors--in my head one doesn't raise errors--one raises
exceptions and interprets them as errors, so allowing for the syntax
raise_exception would be more mind friendly to me."

David Chelimsky

unread,
Dec 22, 2009, 5:08:49 PM12/22/09
to rspec-users
On Tue, Dec 22, 2009 at 9:33 AM, rogerdpack <rogerp...@gmail.com> wrote:
> raise_error already catches any type of exception, error or not:
>
>   class BlahException < Exception; end
>   class BlahError < StandardError; end
>
>   lambda { raise BlahException }.should raise_error(BlahException)
>   lambda { raise BlahError }.should raise_error(BlahError)
>   lambda { raise "blah" }.should raise_error(RuntimeError, "blah")

Thanks for the response.  I think my request was more of a "why call
them errors--in my head one doesn't raise errors--one raises
exceptions and interprets them as errors, so allowing for the syntax
raise_exception would be more mind friendly to me."

What I really want to say is "should raise(Blah)" but Ruby already defines raise as a keyword :)

I'd be open to aliasing raise_error with raise_exception, renaming it to raise_exception and aliasing raise_error for compatibility, but I think this might just add confusion rather than clarifying intent. Thoughts?

David
 
-r

Pat Maddox

unread,
Dec 23, 2009, 11:35:03 AM12/23/09
to rspec-users
is perfectly readable.  I don't see why you would need to change anything to make it more so.  An alias provides no extra value.  And raise_error should only pass on StandardError or subclasses if no class is specified.  Ruby makes you specify the class if it's an Exception, and Ruby semantics should be RSpec defaults.

Pat

p.s. I don't know if raise_error should even allow you to skip the error type.  I've seen way too many tests that look like

it "should raise an error" do
  lambda {
    foo.stupid_typo_that_will_raise_a(NoMethodError)
    foo.some_method_that_raises(TheRealError)
  }.should raise_error
end

But that's another thread

rogerdpack

unread,
Dec 28, 2009, 3:08:19 PM12/28/09
to rspec...@rubyforge.org
> What I really want to say is "should raise(Blah)" but Ruby already defines
> raise as a keyword :)
>
> I'd be open to aliasing raise_error with raise_exception, renaming it to
> raise_exception and aliasing raise_error for compatibility, but I think this
> might just add confusion rather than clarifying intent. Thoughts?

(obviously) I like raise_exception

The only drawback would be that I suppose in keeping with the new name
it "could" be redefined to catch Exception by default, if it currently
catches StandardError(?)

raise_error seems less readable to me, as there is no Error class.

Having both would be fine by me, too.

Happy holidays.

David Chelimsky

unread,
Dec 28, 2009, 3:33:06 PM12/28/09
to rspec-users
On Mon, Dec 28, 2009 at 2:08 PM, rogerdpack <rogerp...@gmail.com> wrote:
> What I really want to say is "should raise(Blah)" but Ruby already defines
> raise as a keyword :)
>
> I'd be open to aliasing raise_error with raise_exception, renaming it to
> raise_exception and aliasing raise_error for compatibility, but I think this
> might just add confusion rather than clarifying intent. Thoughts?

(obviously) I like raise_exception

The only drawback would be that I suppose in keeping with the new name
it "could" be redefined to catch Exception by default, if it currently
catches StandardError(?)

As luck (irony?) would have it, the default _is_ Exception. So if you alias it in your project, you'll get what you're after.
 
raise_error seems less readable to me, as there is no Error class.

This is sound logic, but I think it would do more harm than good at this point.

If we were starting today, I'd probably want to have raise_exception and raise_error, with the defaults being Exception and StandardError respectively. That would certainly improve the accuracy of the intent expressed in specs. The problem at this point is that it changes the meaning of raise_error, which would lead to new failures (which cause pain), and false positives (which cause pain you don't even know is being caused).

If we just alias raise_error with raise_exception, now we have two methods that do the same thing, and the fact that they have names that might mean different things to different people, I think we'd end up with more confusion.

If we replace raise_error with raise_exception, we'd have to deprecate raise_error, which causes pain that would be difficult to justify to many.

So we're going to leave it as raise_error for now. If you'd like to push on this (which you're welcome to), please add a ticket to lighthouse so it's easy to find the discussions around it. If we can get general consensus that this would be a good move in spite of the negatives I just outlined, I'm happy to do it.

Cheers,
David

Rodrigo Rosenfeld Rosas

unread,
Dec 28, 2009, 8:12:13 PM12/28/09
to rspec-users
David Chelimsky escreveu:
Maybe a voting process would be a good idea... But where to get visibility?

I don't think a deprecation would cause that much of pain, once it is
justified in a changelog. Maybe it would be a change for the next major
version, where people are more willing to accept major changes...

My vote would be to include raise_exception (independently to aliasing
it or not) as the specs turns out more concise when the exception is not
an error.

Best regards and have a great new year!

Rodrigo.
__________________________________________________
Faça ligações para outros computadores com o novo Yahoo! Messenger
http://br.beta.messenger.yahoo.com/

rogerdpack

unread,
Dec 29, 2009, 10:26:24 AM12/29/09
to rspec...@rubyforge.org
> So we're going to leave it as raise_error for now. If you'd like to push on
> this (which you're welcome to), please add a ticket to lighthouse so it's
> easy to find the discussions around it. If we can get general consensus that
> this would be a good move in spite of the negatives I just outlined, I'm
> happy to do it.

All righty let the voting begin.

https://rspec.lighthouseapp.com/projects/5645-rspec/tickets/933-add-raise_exception

My current leaning is actually (re a previous comment), to add a new
method raise_exception, which requires them to pass in the
ExceptionClass to be raised, but feel free to discuss this suggestion
at the ticket above.

dchel...@gmail.com

unread,
Dec 29, 2009, 10:27:25 AM12/29/09
to rspec...@rubyforge.org
On Dec 28, 2:33 pm, David Chelimsky <dchelim...@gmail.com> wrote:
> So we're going to leave it as raise_error for now. If you'd like to push on
> this (which you're welcome to), please add a ticket to lighthouse so it's
> easy to find the discussions around it. If we can get general consensus that
> this would be a good move in spite of the negatives I just outlined, I'm
> happy to do it.

Hey all - Roger submitted https://rspec.lighthouseapp.com/projects/5645/tickets/933
on this, so if anybody else has more thoughts on it, please add them
to the ticket.

Cheers,
David

Reply all
Reply to author
Forward
0 new messages