From: nathaniel@...
Date: 2003-02-06T11:15:39+09:00
Subject: Re: Return values from assertions

Matt Armstrong [mailto:matt@lickey.com] wrote:

> > 1) Thoughts on the overall idea of assertions returning values,
> > particularly pitfalls you see with it.
> 
> No thoughts here, though I find having assert_raises return
> the exception raised useful for further testing on the 
> exception object (below).  Similar arguments for 
> assert_match, though I had not thought to do that until now.

First of all, thanks for replying, Matt! I'm sorry it has taken so long
for me to get back to this.

In response to your statement about finding the exception return value
useful, I guess the question is, is it more readable (and not too
difficult) to do one of the following instead:

  begin
    raise "exception message"
    flunk("Should have thrown an exception")
  rescue Exception => e
    assert_equal("exception message", e.message, "Expected a message")
  end

or,

  assert_raises_with_message(Exception, "exception message", "Expected
exception") do
    raise "exception message"
  end

I realize the name in the latter example is perhaps not the best, but
it's the idea of using a more specific assertion for the case of wanting
to check the message. Of course, there are other potentially interesting
bits of an exception, which is what I personally don't like about it
(and I don't like the verbosity of the first way).


> > 2) Code examples using the current return values from #assert_match
> > and #assert_raises.
> 
> This is using RUnit's assert_exception, but you get the idea.
>  I use it all the time to make sure the expected exception is 
> the one I think it is by examining the message.
> 
>     defer_reason = "I might not like you any more"
>     lda, log = new_lda
>     e = assert_exception(RFilter::DeliveryAgent::DeliveryDefer) {
>       lda.defer(defer_reason)
>     }
>     assert_equal(e.message, defer_reason)

Let me rewrite this with my two alternatives above:

  defer_reason = "I might not like you any more"
  lda, log = new_lda
  begin
    lda.defer(defer_reason)
    flunk
  rescue RFilter::DeliveryAgent::DeliveryDefer => e
    assert_equal(e.message, defer_reason)
  end

or

  defer_reason = "I might not like you any more"
  lda, log = new_lda
  assert_exception(RFilter::DeliveryAgent::DeliveryDefer, defer_reason)
{
    lda.defer(defer_reason)
  }

Hmmm... after seeing that, I'm leaning towards leaving the return value
to #assert_raises. The first way is just too much work, and the second
way doesn't give me enough power. It just seems more robust to allow the
ability to inspect anything in the exception, instead of being
constrained by what the framework designer thought you should be able to
check.


> > 3) Code examples showing the use of return values from other
> > assertions.
> 
> I don't know if Test::Unit has the equivalent of RUnit's
> assert_no_exception, but I have used it this way:
> 
>     h = RMail::Header.new
>     # This time is out of the range that can be represented by a
>     # Time object.
>     h.add_raw("Date: Sun, 14 Jun 2065 05:51:55 +0200")
>     t = assert_no_exception {
>       h.date
>     }
>     assert_nil(t)

I don't really understand what you're testing here...
#assert_no_exception (or, in Test::Unit's case, #assert_nothing_raised)
will fail already if an exception is thrown, so why the extra check
afterwards? From looking at RubyUnit's source, there doesn't even seem
to be an explicit return value from this method.


What I'm thinking at the moment is:

1) Leave in the return value from #assert_raises.
2) Remove the return value from #assert_match.
3) In general, avoid returning (documented) values from assertions.

To clarify #3, I don't plan on returning nil from all the assertions,
but I do plan on not explicitly returning something useful from them,
and saying that if you depend on something that is returned as a side
effect, too bad for you if it changes.

Thoughts?


Nathaniel

<:((><
+ - -
| RoleModel Software, Inc.
| EQUIP VI