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)
On Feb 10, 7:55 pm, Kevinst <[email protected]> wrote:
> 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
--
You received this message because you are subscribed to the Google Groups
"Rhino.Mocks" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/rhinomocks?hl=en.
