Tearing down when SetUp fails?

Hans Christian Falkenberg <[email protected]> Fri, 12 Sep 2008 14:32:41 +0200 (CEST)
Newsgroups gmane.comp.windows.dotnet.nunit.devel
Message-ID <[email protected]>
  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--0-948110371-1221220460=:3635
Content-Type: TEXT/PLAIN; CHARSET=US-ASCII; FORMAT=flowed
Content-ID: <[email protected]>

Hi,

I was about to post a bug about this - but then I read the
documentation and realized this might be something up
for discussion. Please let me know if it's there's a previous
conclusion on this and I'll just submit the bug report...

Issues:
1. TearDown behavior is inconsistent between
    [TearDown] and [TestFixtureTearDown]
2. The documentation specifies that it should
    do the Wrong Thing (imho) :p

What I think should happen:
   If there is an exception in any [SetUp] method, still run
   all [TearDown] methods.
   If there is an exception in any [TestFixtureSetUp] method,
   still run all [TestFixtureTearDown] methods.
(I realize only one of each method is allowed, but the doc says "any")

What the documentation says (in both 2.4.8 and 2.5 alpha 3):
   So long as any SetUp method runs without error, the TearDown method is
   guaranteed to run. It will not run if a SetUp method fails or throws an
   exception.
and
   So long as any TestFixtureSetUp method runs without error, the
   TestFixtureTearDown method is guaranteed to run. It will not run if a
   TestFixtureSetUp method fails or throws an exception.

What actually happens (in both 2.4.8 and 2.5 alpha 3):
Running attached code:
   c:\>"c:\Program Files\NUnit 2.4.8\bin\nunit-console.exe"
     /nologo bin\Debug\Dummy.dll
   Fixture Setting up
   .Setting up
   Tearing down
   FFixture Tearing down

   Tests run: 1, Failures: 1, Not run: 0, Time: 0.030 seconds
Uncommenting FixtureSetUp exception:
   c:\>"c:\Program Files\NUnit 2.4.8\bin\nunit-console.exe"
    /nologo bin\Debug\Dummy.dll
   Fixture Setting up
   .F
   Tests run: 1, Failures: 1, Not run: 0, Time: 0.028 seconds


So [TearDown] doesn't behave as specified when a [SetUp] method throws an 
exception, that's a bug report, right?

Except I think it *should* behave that way, so I was going to submit
a bug report on [TestFixtureTearDown] :)

Here's my reasoning:
Say you are acquiring two resources in the SetUp method:
[SetUp] public void SetUp() {
   r1 = Resources.Acquire1();
   r2 = Resources.Acquire2();
}

And of course the must be released:
[TearDown] public void TearDown() {
   Resources.Release(r1);
   r1 = null;
   Resources.Release(r2);
   r2 = null;
}

Now, since Resource.Release will just return if its argument
is null and never throws, everything is fine. Except if the
documentation is to believed: If an exception can occur while
acquiring r2 (and it can), we'll have to write this:

[SetUp] public void SetUp() {
   r1 = Resources.Acquire1();
   try {
     r2 = Resources.Acquire2();
   } catch {
     Resources.Release(r1);
     throw;
   }
}

Which I think is bad, but I guess not *that* bad... until you
have 4-5 resources. Then there's suddenly a crazy amount of TearDown 
code duplication in SetUp. So here is how I decided to work around
this problem:

[SetUp] public void SetUp() {
   setUpCalled = true;
   r1 = Resources.Acquire1();
   r2 = Resources.Acquire2();
}

[TearDown] public void TearDown() {
   if (!setUpCalled) return;
   setUpCalled = false;
   Resources.Release(r1);
   r1 = null;
   Resources.Release(r2);
   r2 = null;
}

Not the worst of solution's, but now I'm basically doing NUnit's
work myself. And when the methods in TearDown *can* throw Exceptions
they will now be reported as if they were thrown in SetUp,
which is just to ask for someone being confused later.
But at least the code will release resources properly in both
NUnit 2.4.8 (and 2.5 alpha 3), ReSharper 4.1.933.3 (which behaves
like the NUnit doc says it should) and future NUnit versions
which might adhere to the documentation.

So... could we instead please change the NUnit behavior so that
other people don't have to write workarounds like this?

I know junit doesn't run TearDown (or didn't when I last used it a
year back) when SetUp throws, but junit didn't report test
exceptions when TearDown throwed either, so not much cause
to let them set the standard here...

A better argument against might be that people didn't write code
to check for null in their [TearDown] and [TestFixtureTearDown]
methods. But then their [TearDown] methods are already failing,
and they'll just have to fix their [TestFixtureTearDown] methods
when upgrading to 2.5, won't they?

+ Hans Christian



PS: If you are wondering wth. I'm thinking wrt. acquiring 4-5
resources before a unit tests... Well, the resources are files
(with Acquire==Create and Release==Delete) and it would hardly
be proper to use mocked files for unit testing when testing
writing to the OS - that would kind of void the entire testing.

PPS: Why should a unit test need files? Well - it does.
--0-948110371-1221220460=:3635
Content-Type: TEXT/PLAIN; CHARSET=US-ASCII; NAME="ReSharperTest.cs"
Content-Transfer-Encoding: BASE64
Content-ID: <[email protected]>
Content-Description: 
Content-Disposition: ATTACHMENT; FILENAME="ReSharperTest.cs"

dXNpbmcgU3lzdGVtOw0KdXNpbmcgTlVuaXQuRnJhbWV3b3JrOw0KDQpuYW1l
c3BhY2UgRHVtbXkNCnsNCiAgICBbVGVzdEZpeHR1cmVdDQogICAgcHVibGlj
IGNsYXNzIFJlU2hhcnBlclRlc3QNCiAgICB7DQogICAgICAgIFtUZXN0Rml4
dHVyZVNldFVwXQ0KICAgICAgICBwdWJsaWMgdm9pZCBGaXh0dXJlU2V0VXAo
KQ0KICAgICAgICB7DQogICAgICAgICAgICBDb25zb2xlLldyaXRlTGluZSgi
Rml4dHVyZSBTZXR0aW5nIHVwIik7DQogICAgICAgICAgICAvL3Rocm93IG5l
dyBFeGNlcHRpb24oIkZhaWxlZCB0byBhY3F1aXJlIHJlc291cmNlIik7DQog
ICAgICAgIH0NCg0KICAgICAgICBbU2V0VXBdDQogICAgICAgIHB1YmxpYyB2
b2lkIFNldFVwKCkNCiAgICAgICAgew0KICAgICAgICAgICAgQ29uc29sZS5X
cml0ZUxpbmUoIlNldHRpbmcgdXAiKTsNCiAgICAgICAgICAgIHRocm93IG5l
dyBFeGNlcHRpb24oIkZhaWxlZCB0byBhY3F1aXJlIHJlc291cmNlIik7DQog
ICAgICAgIH0NCg0KICAgICAgICBbVGVzdF0NCiAgICAgICAgcHVibGljIHZv
aWQgTm9vcFRlc3QoKQ0KICAgICAgICB7DQogICAgICAgIH0NCg0KICAgICAg
ICBbVGVhckRvd25dDQogICAgICAgIHB1YmxpYyB2b2lkIFRlYXJEb3duKCkN
CiAgICAgICAgew0KICAgICAgICAgICAgQ29uc29sZS5Xcml0ZUxpbmUoIlRl
YXJpbmcgZG93biIpOw0KICAgICAgICB9DQoNCiAgICAgICAgW1Rlc3RGaXh0
dXJlVGVhckRvd25dDQogICAgICAgIHB1YmxpYyB2b2lkIEZpeHR1cmVUZWFy
RG93bigpDQogICAgICAgIHsNCiAgICAgICAgICAgIENvbnNvbGUuV3JpdGVM
aW5lKCJGaXh0dXJlIFRlYXJpbmcgZG93biIpOw0KICAgICAgICB9DQogICAg
fQ0KfQ0K

--0-948110371-1221220460=:3635
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
--0-948110371-1221220460=:3635
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
nunit-developer mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/nunit-developer

--0-948110371-1221220460=:3635--