Assert.Throws: Should it be exact type?

[Charlie Poole]

Hi All,

A feature request asks that Assert.Throws be modified so that
it succeeds if an exception derived from the type specified
is thrown. The submitter says that this is in keeping with the
Liskov substitution principle.

My own thought in implementing it was to provide a more localized
substitute for ExpectedException, which has always required the
exact type. The newer syntax allows the user to either require
an exact type or allow derived types by specifying
Assert.That( delegate{...}, Throws.Typeof(...) OR
Assert.That( delegate(...), Throws.InstanceOf(...)
so the question here is what is the best behavior for the
simpler Assert.Throws.
Thoughts please?

Charlie

[Simone Busoli]

I'd like to be able to choose if I want the exact type or a subclass, so I'd go for TypeOf and InstanceOf.

[Charlie Poole]

Hi Simone,

 

We already have that using the new syntax. However, the request is that the

semantics of the "classic" Assert.Throws form be changed.

 

Charlie

---

From: nun...@googlegroups.com [mailto:nun...@googlegroups.com] On Behalf Of Simone Busoli
Sent: Sunday, September 28, 2008 2:41 PM
To: nun...@googlegroups.com
Subject: [nunitv3] Re: Assert.Throws: Should it be exact type?

[Simone Busoli]

Oh sorry, I'm not up to date with the new stuff in 2.5 since I'm using 2.4.7 in production.

While experimenting with the a new syntax, I approached it in a different way:

Assert.That(() => { throw new InvalidOperationException(); }, ex => ex.Is.OfType<InvalidOperationException>());

The argument to the delegate is the exception itself, so you can do any assertion you want on it. This is more consistent with how the other assertions are performed.

[Charlie Poole]

I thought that was in 2.4 - but probably not. Anyway, minus the syntactic sugar, that's

basically how the 2.5 constraint expression works.

 

I don't generally add a new classic form for new constraints, but it seemed reasonable

to add one for exceptions. I suppose I could remove it - especially if it seems ambiguous.

 

I'm hoping the OP will show up in this discussion to push his request himself. Otherwise,

I'll leave it as is.

 

Charlie

---

From: nun...@googlegroups.com [mailto:nun...@googlegroups.com] On Behalf Of Simone Busoli

Sent: Sunday, September 28, 2008 11:59 PM

[christianacca]

Hi guys,
Thanks for discussing this in my absence.

I actually didn't realise the new constraints syntax supports
Exceptions!

Anyway, had a go at using the constraint model, but unless I'm missing
something obvious (just not to me), this doesn't work in the 2.0 world
without some real ugly cast being thrown in. Eg:
Assert.That( (TestDelegate) delegate(...), Throws.InstanceOf(...)

Also something that I need to do is compare the Exception.Data
dictionary. So I would like to do something like this:

ArgumentException actual =
Assert.ThrowsInstanceOf<ArgumentException>(delegate {
SomeMethodThatThrowsExceptionDerivingFromArgumentException();
});
ArgumentOutOfRangeException expected =
NewExpectedExceptionFor("Property1");
AssertThat.AreEqual(actual, expected);

where the AssertThat.AreEqual is a custom assert class that compares
exception instances and asserts that the message and Data are equal.

This gives in some ways simpilar syntax, while allowing the Exception
to be returned for further comparison with an expected instance.

What do you think?
Christian

On Sep 29, 3:36 pm, "Charlie Poole" <char...@nunit.com> wrote:
> I thought that was in 2.4 - but probably not. Anyway, minus the syntactic
> sugar, that's
> basically how the 2.5 constraint expression works.
>
> I don't generally add a new classic form for new constraints, but it seemed
> reasonable
> to add one for exceptions. I suppose I could remove it - especially if it
> seems ambiguous.
>
> I'm hoping the OP will show up in this discussion to push his request
> himself. Otherwise,
> I'll leave it as is.
>
> Charlie
>
>   _____  
>

> From: nun...@googlegroups.com [mailto:nun...@googlegroups.com] On Behalf
> Of Simone Busoli
> Sent: Sunday, September 28, 2008 11:59 PM
> To: nun...@googlegroups.com
> Subject: [nunitv3] Re: Assert.Throws: Should it be exact type?
>
> Oh sorry, I'm not up to date with the new stuff in 2.5 since I'm using 2.4.7
> in production.
>
> While experimenting with the a new syntax, I approached it in a different
> way:
>
> Assert.That(() => { throw new InvalidOperationException(); }, ex =>
> ex.Is.OfType<InvalidOperationException>());
>
> The argument to the delegate is the exception itself, so you can do any
> assertion you want on it. This is more consistent with how the other
> assertions are performed.
>

> On Mon, Sep 29, 2008 at 2:13 AM, Charlie Poole <char...@nunit.com> wrote:
>
> Hi Simone,
>
> We already have that using the new syntax. However, the request is that the
> semantics of the "classic" Assert.Throws form be changed.
>
> Charlie
>
>   _____  
>

> From: nun...@googlegroups.com [mailto:nun...@googlegroups.com] On Behalf
> Of Simone Busoli
> Sent: Sunday, September 28, 2008 2:41 PM
> To: nun...@googlegroups.com
> Subject: [nunitv3] Re: Assert.Throws: Should it be exact type?
>
> I'd like to be able to choose if I want the exact type or a subclass, so I'd
> go for TypeOf and InstanceOf.
>

[Charlie Poole]

Hi Christian,

> Hi guys,
> Thanks for discussing this in my absence.

We're glad you're here. Your first post had to wait till I OK'd
it, but future ones will go right through.

> I actually didn't realise the new constraints syntax supports
> Exceptions!
>
> Anyway, had a go at using the constraint model, but unless
> I'm missing something obvious (just not to me), this doesn't
> work in the 2.0 world without some real ugly cast being thrown in. Eg:
> Assert.That( (TestDelegate) delegate(...), Throws.InstanceOf(...)

You should only need that cast if you provide something that requires
conversion to a test delegate. For 2.0, I generally just use an anonymous
delegae. In any case, this is not a difference in the two approaches,
since both currently use a TestDelegate.

> Also something that I need to do is compare the
> Exception.Data dictionary. So I would like to do something like this:
>
> ArgumentException actual =
> Assert.ThrowsInstanceOf<ArgumentException>(delegate {
> SomeMethodThatThrowsExceptionDerivingFromArgumentException();
> });
> ArgumentOutOfRangeException expected =
> NewExpectedExceptionFor("Property1");
> AssertThat.AreEqual(actual, expected);
>
> where the AssertThat.AreEqual is a custom assert class that
> compares exception instances and asserts that the message and
> Data are equal.

Interesting. You *could* do this with the new syntax...

Assert.That( {...}, Throws.InstanceOf<ArgumentException>
.With.Property("Data").EqualTo(expected.Data);
if we simply added code to compare two dictionaries for equality.

Is this part of the same problem? That is, are you processing
exceptions for which you cannot predict the exact type? If so,
could you explain how that comes about in a test?

> Exception to be returned for further comparison with an
> expected instance.
>
> What do you think?

I generally like to use the existing facilities like so:

1. Assert.Throws(...) for the very simplest cases

2. Assert.That( {...}, Throws(...)) with added constraints for
more complex cases.

3. Assert.Throws with further tests on the return value for
really complicated tests that are hard to read in the second
form and work better as sequential code.

Of course, I keep trying to think of new constructs to push
the #3 items up to #2. :-)

I wouldn't mind adding Assert.ThrowsInstanceOf to work
like Assert.Throws except for allowing derived exceptions so
long as nobody else objects.

What do folks think?

Charlie

[Kelly Anderson]

I would go back to the principle of least surprise on this one. For
me, I have always asserted that it would be a particular type, but
hadn't considered what would happen if a subclass of that exception
were thrown. Now that I consider it though, I think that I would be
surprised if a test failed because it was a subclass of the base
exception. In other words, I think that the simpler Assert.Throws
should succeed even if a subclass of the original tested for Exception
type were thrown.

If they want the more specific behavior, they can use the TypeOf
syntax, so nothing is really lost. I can't imagine a scenario where
you would be sad if a subclass of the exception were thrown instead of
the base, but perhaps I have a limited imagination, as I don't create
subclasses of Exception terribly often.

-Kelly

[Jim Newkirk]

I'll just chime in with why in my opinion it should not work if its a subclass.

When doing TDD the purpose of the test is to document the specific behavior expected. Allowing a subclass makes it easy to be sloppy and just say that this code throws some exception which if done reduces the readability of the test.

Jim

[Charlie Poole]

This is definitely how I think one should write tests. When examining requests

that go against one's own view of best practices, it's always hard to decide how

firm a line should be drawn.

 

In this case, I only see a looser implementation being useful in a few limited

situations, so I don't want to provide it as the default.

 

Charlie

---

From: nun...@googlegroups.com [mailto:nun...@googlegroups.com] On Behalf Of Jim Newkirk
Sent: Tuesday, September 30, 2008 10:37 AM

To: nun...@googlegroups.com
Subject: [nunitv3] Re: Assert.Throws: Should it be exact type?

[Charlie Poole]

Hi Kelly,

I'd really be surprised myself if you were writing a test and
didn't know what kind of exception was being thrown. :-)

Wouldn't it violate the same principle if you convert from use
of ExpectedException to Assert.Throws and find you are
getting different results?

Charlie

> -----Original Message-----
> From: nun...@googlegroups.com
> [mailto:nun...@googlegroups.com] On Behalf Of Kelly Anderson
> Sent: Tuesday, September 30, 2008 10:03 AM
> To: nun...@googlegroups.com
> Subject: [nunitv3] Re: Assert.Throws: Should it be exact type?
>
>

[shovavnik]

As I see it, when you're unit testing, you're generally testing state,
behavior or some domain. You're not testing whether your code is
object-oriented.

Further, typed exceptions have very specific meanings, and it is often
important to know exactly which kind of exception was thrown. By
catching an ancestor exception type in a unit test, you could be
incorrectly responding to specific exceptions thrown by dependent
objects.

And once you change the semantics of the syntactic sugar so it behaves
differently from the "unsweetened" syntax, it ceases to be "syntactic
sugar"; it becomes a new semantic paradigm altogether, which incurs
its own learning curve overhead.

My vote is for "exact type" as the default, but leave the inheritance
option available.

[Kelly Anderson]

Yes, I suppose that would surprise me. :-)

I don't have very strong feelings on this subject, and can see both
sides of the argument. Clearly not changing behavior from one version
to the next is also very important.

-Kelly

[christianacca]

Interesting discussion everyone.

I'll specifically address Charlie's questions to me however as I'm a
bit pressed for time.

>You should only need that cast if you provide something that requires conversion to a test delegate. For 2.0, I generally just use an anonymous delegate.

For me using an anonymous method, 2.0 is requiring me to cast the
anonymous method into the specific delegate type. Omitting it, the
compiler is telling me it "cannot resolve method" ie it cannot select
the appropriate overload of the Assert.That operation.

>interesting. You *could* do this with the new syntax...

Assert.That( {...}, Throws.InstanceOf<ArgumentException>
.With.Property("Data").EqualTo(expected.Data);
if we simply added code to compare two dictionaries for equality.

My little AssertThat.AreEqual(Exception actual, Exception expected)
was trying to overcome this issue - it uses the IsEquivalentTo to
compare the two Data dictionaries.

>Is this part of the same problem? That is, are you processing exceptions for which you cannot predict the exact type? If so, could you explain how that comes about in a test?

Well actually comparing the Data dictionary and wanting to compare the
exception type where I cannot predict (well actually don't want to
predicte) the exact exception type are two separate requirements for
me at least. In the case of comparing exception type, I'm "testing" a
third party framework and am just interested that it raises an
exception that belongs to a particular exception hierarchy.

> I generally like to use the existing facilities like so...

I like your approach.

>I wouldn't mind adding Assert.ThrowsInstanceOf to work like Assert.Throws except for allowing derived exceptions so long as nobody else objects.

Please do!

Thanks for an interesting discussion.
Christian

[christianacca]

Interesting discussion everyone.

I'll specifically address Charlie's questions to me however as I'm a
bit pressed for time.

>You should only need that cast if you provide something that requires conversion to a test delegate. For 2.0, I generally just use an anonymous delegate.

For me using an anonymous method, 2.0 is requiring me to cast the
anonymous method into the specific delegate type. Omitting it, the
compiler is telling me it "cannot resolve method" ie it cannot select
the appropriate overload of the Assert.That operation.

>interesting. You *could* do this with the new syntax...

Assert.That( {...}, Throws.InstanceOf<ArgumentException>
.With.Property("Data").EqualTo(expected.Data);
if we simply added code to compare two dictionaries for equality.

My little AssertThat.AreEqual(Exception actual, Exception expected)
was trying to overcome this issue - it uses the IsEquivalentTo to
compare the two Data dictionaries.

>Is this part of the same problem? That is, are you processing exceptions for which you cannot predict the exact type? If so, could you explain how that comes about in a test?

Well actually comparing the Data dictionary and wanting to compare the
exception type where I cannot predict (well actually don't want to
predicte) the exact exception type are two separate requirements for
me at least. In the case of comparing exception type, I'm "testing" a
third party framework and am just interested that it raises an
exception that belongs to a particular exception hierarchy.

> I generally like to use the existing facilities like so...

I like your approach.

>I wouldn't mind adding Assert.ThrowsInstanceOf to work like Assert.Throws except for allowing derived exceptions so long as nobody else objects.

Please do!

Thanks for an interesting discussion.
Christian

[Charlie Poole]

Hi Christian,

> Interesting discussion everyone.
>
> I'll specifically address Charlie's questions to me however
> as I'm a bit pressed for time.
>
> >You should only need that cast if you provide something that
> requires conversion to a test delegate. For 2.0, I generally
> just use an anonymous delegate.
>
> For me using an anonymous method, 2.0 is requiring me to cast
> the anonymous method into the specific delegate type.
> Omitting it, the compiler is telling me it "cannot resolve
> method" ie it cannot select the appropriate overload of the
> Assert.That operation.

Odd... It appears that I removed all tests with anonymous methods
at some point when I was reorganizing the code - since the tests
need to work with .NET 1.0 and 1.1, any such tests have to be
conditionally compiled, so they are not used in general.

As you might expect, in the absence of such a test, anonymous
delegates no longer work - as you observed. However, this is
a bug, which can be fixed, now that I know about it.

> >interesting. You *could* do this with the new syntax...
>
> Assert.That( {...}, Throws.InstanceOf<ArgumentException>
> .With.Property("Data").EqualTo(expected.Data);
> if we simply added code to compare two dictionaries for equality.
>
>
> My little AssertThat.AreEqual(Exception actual, Exception
> expected) was trying to overcome this issue - it uses the
> IsEquivalentTo to compare the two Data dictionaries.

Yes, that's a simple approach I hadn't thought of.

> >Is this part of the same problem? That is, are you
> processing exceptions for which you cannot predict the exact
> type? If so, could you explain how that comes about in a test?
>
> Well actually comparing the Data dictionary and wanting to
> compare the exception type where I cannot predict (well
> actually don't want to
> predicte) the exact exception type are two separate
> requirements for me at least. In the case of comparing
> exception type, I'm "testing" a third party framework and am
> just interested that it raises an exception that belongs to a
> particular exception hierarchy.

OK, I can see some advantage in that situation.

Charlie

[Charlie Poole]

Hi all,

 

I have tried to keep to the spirit of TDD while allowing an optional variation where needed.

 

The forms that use a Type as an argument still require an exact match.

 

The constraint-based assert model has always let you use TypeOf or InstanceOf

as the constraint.

 

I added a new overload to Assert.That, taking a constraint expression as its

first argument, so now you can use any of the following

 

  Assert.Throws<MyException>( del );

  Assert.Throws( Is.TypeOf<MyException>, del);

  Assert.Throws( Is.InstanceOf<MyException>, del);

 

The first two are equivalent.,

 

Charlie

---

From: nun...@googlegroups.com [mailto:nun...@googlegroups.com] On Behalf Of Jim Newkirk
Sent: Tuesday, September 30, 2008 10:37 AM

To: nun...@googlegroups.com
Subject: [nunitv3] Re: Assert.Throws: Should it be exact type?

[Simone Busoli]

I like it, it's intuitive.