How to accept an exception in a unit test to test something that is supposed to happen afterwards

[Kevinst]

Hello everyone,

"the art of unit testing" taught me that a try/catch block in a unit
test is bad practice.

So I am hoping someone can help me to find the right approach.

Let's say I have a class "Container" and a class "Foo" whereas the
Container contains a foo class and multiple container classes can
contain the same foo instance.

Container is defined as following:
class Container
{
private Foo foo;
bool closeFooOnClose;

public Container(Foo foo)
{
this.foo = foo;
}

public void Open()
{
if(foo.IsOpenedExclusively)
{
throw new Exception();
}
foo.OpenExclusively();
closeFooOnClose = true;
}

public void Close()
{
if(closeFooOnClose)
foo.Close();
}
}

Foo:

class Foo
{
public bool IsOpenedExclusively();
public void OpenExclusively(){};
public void Close(){};
}

Now I want to test that Container will not try to close the Foo
instance in case the Open scenario failed.
So what comes to my mind first is:

[TestMethod]
public void Close_FailedToOpenFoo_DoesNotTryToClose()
{
var foo = MockRepository.GenerateStrictMock<Foo>();
foo.Expect(f => f.IsOpenedExclusively).Return(true);

var uut = new Container(foo);
try { uut.Open(); }
catch(Exception { }

uut.Close();

foo.VerifyAllExpectations();
}

But.. as explained earlier I think try/catch blocks are to be avoided.
Maybe this is an exception? Or is there a way offered by rhino mocks
to deal with those scenarios?
Or do you know a way to refactor this to make it testable?

Thank you,
Kevin

[Tim Barcz]

I personally don't see the error with the test below.  I would ask if you are feeling pain with this type of test.  If the answer is no, I would proceed until such a point that you do rather than adhering to a list of rules arbitrarily.  There are a lot of "rules" around unit testing and many of them make sense to me because of how I've experienced not adhering to the rules.  Likewise, there are many "rules" that I've broken and never felt pain from breaking...and those are the rules I worry least about and wonder if they should have ever been rules in the first place.

--
You received this message because you are subscribed to the Google Groups "Rhino.Mocks" group.
To post to this group, send email to rhino...@googlegroups.com.
To unsubscribe from this group, send email to rhinomocks+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/rhinomocks?hl=en.

--
Tim Barcz
Microsoft C# MVP
Microsoft ASPInsider
http://timbarcz.devlicio.us
http://www.twitter.com/timbarcz

[bill richards]

I'm going to agree with Tim on this one.

I cannot see anything wrong with your test ... except maybe renaming
the method (because it doesn't really mean much to me at the moment),
and also maybe removing the explicit test for Exception in the catch
(this is because you are not actually interested in what exception was
thrown), however, I would be tempted to create a new exception type
for Container to throw and then assert that my new exception type was
thrown ...

[TestMethod]
public void Close_FailedToOpenFoo_DoesNotTryToClose()
{
var exceptionWasThrown = false;
var foo = MockRepository.GenerateStub<Foo>();
foo.Stub(f => f.IsOpenedExclusively).Return(true);

var container = new Container(foo);
try { container.Open(); }
catch(MyNewExceptionType) { exceptionWasThrown = true; }
finally { uut.Close(); }

Assert.That(exceptionWasThrown, Is.False);
}

As Tim pointed out, guidelines are guidelines, and generally if you
are just expecting an exception to be throwand you don't really care
about the internals of that exception then one would re-write the
above test as follows:

[TestMethod]
[ExpectedException(typeof(MyNewExceptionType))]
public void Close_FailedToOpenFoo_DoesNotTryToClose()
{
var exceptionWasThrown = false;
var foo = MockRepository.GenerateStub<Foo>();
foo.Stub(f => f.IsOpenedExclusively).Return(true);

var container = new Container(foo);
container.Open();
}

and in this case you really should create a derived exception type
because Exception (as we know) is the base type for all exceptions, so
if we jut expectthis type to be thrown the test will pass no matter
what actual exception is thrown, but we are expecting a specific
typeof exception to be thrown.

So the rule of thumb for me is .... if I just want to assert that a
specific type of exception is thrown when I expect it and I don't care
about the exception details then I will use [ExpectedException()],
however, if I do care about the exception instance and I want to check
the internals of it then I will use a try/catch/finally block.

Hope that helps ;o)

[bill richards]

Whoops .... the above line

Assert.That(exceptionWasThrown, Is.False);

should read

Assert.That(exceptionWasThrown, Is.True);

sorry about that.

On Feb 10, 10:00 pm, bill richards <bill.richa...@greyskin.co.uk>
wrote:

> > Kevin- Hide quoted text -
>
> - Show quoted text -

[Kevinst]

Thank you guys. I think I will just live with it.

I would have used that ExpectedException attribute but I have to call
mock.VerifyAllExpectations() after the exception was thrown to make
sure no Close was executed.. so I have to catch it I guess?
The Exception is a custom one.. I just didnt want to overload my
thread ;-)

What would your new name suggestion be?

[bill richards]

Off the top of my head, I would say something like ...

WhenFooIsOpenedExclusivelyAndOpenIsCalledOnContainer_ShouldThrowMyNewException

and with regards to your *need* to test that Close() was not called :
this is not really required, since the Open() method throws an
exception and Close() is only invoked when an exception is not thrown
(in your original code), however this is not th case if you should
change your try/catch block to include a finally clause, i.e.

try
{
container.Open();
}
catch(MyNewExceptionType)
{
exceptionWasThrown = true;
}
finally
{
uut.Close();
}

under these circumstances, you will need to assert that Close() was
not called.

try
{
container.Open();
}
catch(MyNewExceptionType)
{
exceptionWasThrown = true;
}

uut.Close();

under these circumstances, you will not need to assert that Close()
was not called.

[Tim Barcz]

I agree with what Bill is saying...If you want to guard against someone calling the Close() method you need to check that it wasn't called.  Setting a bool in the catch isn't enough.

...tread carefully though, making sure things weren't called, etc can lead you down a path of overspecification of tests...

--
You received this message because you are subscribed to the Google Groups "Rhino.Mocks" group.
To post to this group, send email to rhino...@googlegroups.com.
To unsubscribe from this group, send email to rhinomocks+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/rhinomocks?hl=en.

[Aaron Alexander]

With some test frameworks you can assert that a specific exception
type was thrown and (optionally) capture the exception so you can do
further assertions on it. This allows you to verify that a call threw
an exception and then continue performing assertions (such as the
VerifyAllExpectations() call) without needing to put try/catch/finally
blocks in the test method.

In MbUnit v3, you would change your test code to:

var uut = new Container(foo);

Assert.Throws<Exception>(() => uut.Open());

uut.Close();

foo.VerifyAllExpectations();

I think that NUnit 2.5 has similar capabilities. I'm not sure which
other test frameworks have similar syntax available.

[Tim Barcz]

additionally you could rather than using a StrictMock (which I try to shy away from) use a DynamicMock (GenerateMock()) and do foo.AssertWasNotCalled().

From an expresiveness standpoint, I think foo.AssertWasNotCalled would give future readers of the test (yourself included) a better clue on what exactly is going on.

--
You received this message because you are subscribed to the Google Groups "Rhino.Mocks" group.
To post to this group, send email to rhino...@googlegroups.com.
To unsubscribe from this group, send email to rhinomocks+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/rhinomocks?hl=en.

[Kevinst]

I did not know this AssertWasNotCalled :)
Thats pretty nice!

Thanks!

However I think your proposed name is not enough as it does not state
the test verifies that Close was not called. ( I test that the
exception was thrown in another test.. maybe these two tests should be
put together?)

Kevin

On 11 Feb., 15:03, Tim Barcz <timba...@gmail.com> wrote:
> additionally you could rather than using a StrictMock (which I try to shy
> away from) use a DynamicMock (GenerateMock()) and do
> foo.AssertWasNotCalled().
>
> From an expresiveness standpoint, I think foo.AssertWasNotCalled would give
> future readers of the test (yourself included) a better clue on what exactly
> is going on.
>

> > rhinomocks+...@googlegroups.com<rhinomocks%2Bunsubscribe@googlegrou ps.com>