Using IDisposable and constructor in addition to SetUp and TearDown methods

421 views
Skip to first unread message

Fabian Schmied

unread,
Aug 29, 2012, 4:51:14 AM8/29/12
to nunit-...@googlegroups.com
Hi,

One thing that is often bugging me when writing tests using NUnit is
that you cannot, effectively, use the constructor of test fixture
classes to do any initialization work since the constructor is not
guaranteed to run once per test (and thus, tests might influence each
other if they rely on ctor-initialized data). The way to initialize
test fixute data in NUnit instead is to use SetUp methods.

This bugs me because:
- It's a trap waiting to happen for people not yet knowing about the rule above.
- You can't use simple field initializer expressions.
- You can't make fields readonly to express that tests are not
supposed to change the values of those fields.
- It's cumbersome as most Refactoring tools (e.g., ReSharper) will
usually move initialization code to either the field initializer or
the constructor when refactoring an expression or local variable into
a field.

Is this something that may be changed for NUnit 3.0 (or maybe even before)?
If so, I'd suggest also allowing to use IDisposable instead of or in
addition to TearDown methods, as the Dispose method is the "natural"
way of disposing the members initialized from within the constructor.

Best regards,
Fabian

Charlie Poole

unread,
Aug 29, 2012, 6:22:25 PM8/29/12
to nunit-...@googlegroups.com
Hi Fabian,

On Wed, Aug 29, 2012 at 1:51 AM, Fabian Schmied
<fabian....@gmail.com> wrote:
> Hi,
>
> One thing that is often bugging me when writing tests using NUnit is
> that you cannot, effectively, use the constructor of test fixture
> classes to do any initialization work since the constructor is not
> guaranteed to run once per test (and thus, tests might influence each
> other if they rely on ctor-initialized data). The way to initialize
> test fixute data in NUnit instead is to use SetUp methods.

Yes... with the addition that the constructor is actually guaranteed _not_ to
run once per test! NUnit historically chose the pattern of use where
one instance of the fixture is used for all tests.

> This bugs me because:
> - It's a trap waiting to happen for people not yet knowing about the rule above.

I agree.

> - You can't use simple field initializer expressions.

Except as below...

> - You can't make fields readonly to express that tests are not
> supposed to change the values of those fields.

Actually, there's no harm in using simple readonly fields initialized in the
constructor. I do that all the time. You want to avoid readonly instances
(e.g. collections) where a test could change the contents.

> - It's cumbersome as most Refactoring tools (e.g., ReSharper) will
> usually move initialization code to either the field initializer or
> the constructor when refactoring an expression or local variable into
> a field.

I hadn't thought of that, but I see it's an annoyance.

> Is this something that may be changed for NUnit 3.0 (or maybe even before)?

Maybe for 3.0. Such a major change would justify calling the release (at least)
2.7, and we don't plan for it. BTW, I say "major" with respect to the
implications
for users, not because it's a lot of work. Actually, it should be easy.

One possibility for NUnit 3.0 is to allow either model - single instance or one
per test - with one of them selected as the default. What do folks think of
this idea?

> If so, I'd suggest also allowing to use IDisposable instead of or in
> addition to TearDown methods, as the Dispose method is the "natural"
> way of disposing the members initialized from within the constructor.

IDisposable is already supported. If you implement it on your fixture class, it
will be called right after TestFixtureTearDown.

Charlie

> Best regards,
> Fabian
>
> --
> You received this message because you are subscribed to the Google Groups "NUnit-Discuss" group.
> To post to this group, send email to nunit-...@googlegroups.com.
> To unsubscribe from this group, send email to nunit-discus...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/nunit-discuss?hl=en.
>

cliff

unread,
Aug 29, 2012, 7:38:58 PM8/29/12
to nunit-...@googlegroups.com
i like the idea of choice, but only because of a need i have that tends to drive me back to NUnit when I'm using MS Test.  I current try to code in a BDD way with the following test:

public abstract class TestBase
{
        [TestFixtureSetUp]
        public void Setup()
        {
            this.Given();

            try
            {
                this.When();
            }
            finally
            {
                this.Finally();
            }
        }
  
  protected virtual void Given()
  {
  }
  protected virtual void Finally()
  {
  }

  protected abstract void When()
}

public class DoSomethingTests : TestBase
{
     Foo foo;

     protected override void Given()
     {
          foo = new Foo();
    }

    protected override void When()
    {
        foo.DoSomething();
    }

    [Test]
    public void FooHasNewName()
    {
         Assert.AreEqual( "newName", foo.name );
    }

   [Test]
   public void FooHasAnotherNewValue()
   {
         ...
   }

}

This format isn't that big of a deal when i'm unit testing small to medium classes, but helps greatly for acceptance, integration, or other expensive tests without relying on static properties, static helper classes and various null checks to prevent reinitializing data.
--
thanks

cliff

Fabian Schmied

unread,
Aug 30, 2012, 3:39:13 AM8/30/12
to nunit-...@googlegroups.com
Hi Charlie,

[...]

> Yes... with the addition that the constructor is actually guaranteed _not_ to
> run once per test! NUnit historically chose the pattern of use where
> one instance of the fixture is used for all tests.

Okay, so the constructor currently has the same semantics as a
TestFixtureSetUp method?

[...]

>> - You can't use simple field initializer expressions.
>
> Except as below...
>
>> - You can't make fields readonly to express that tests are not
>> supposed to change the values of those fields.
>
> Actually, there's no harm in using simple readonly fields initialized in the
> constructor. I do that all the time. You want to avoid readonly instances
> (e.g. collections) where a test could change the contents.

You're right of course. I've made avoiding field initializers a
general rule for my own tests, as it's simply more robust and avoids
mistakes, but it's of course possible to make a case-by-case decision.

>> Is this something that may be changed for NUnit 3.0 (or maybe even before)?
>
> Maybe for 3.0. Such a major change would justify calling the release (at least)
> 2.7, and we don't plan for it. BTW, I say "major" with respect to the
> implications
> for users, not because it's a lot of work. Actually, it should be easy.

Yep, I thought so. One thing to consider would be a replacement for
TestFixtureSetUp and TestFixtureTearDown - now often used as a
performance improvement for lengthy initialization code. Maybe static
TestFixtureSetUp and TestFixtureTearDown methods could be the way to
go then.

> One possibility for NUnit 3.0 is to allow either model - single instance or one
> per test - with one of them selected as the default. What do folks think of
> this idea?

I think the main reason why one would choose the "classic model"
(assuming the two models are equivalent in functionality) is backwards
compatibility. There are two compatibility concerns:
- tests that have TestFixtureSetUp and TestFixtureTearDown instance
methods, as those wouldn't (couldn't) work as expected any more, and
- existing tests that rely on the guarantee that a test fixture is
constructed only once, using the fixture's ctor as a replacement for
TestFixtureSetUp (and IDisposable for TestFixtureTearDown).

Yes, for such tests (of which we ourselves have quite many), a
compatibility mode would be required, I think.

>> If so, I'd suggest also allowing to use IDisposable instead of or in
>> addition to TearDown methods, as the Dispose method is the "natural"
>> way of disposing the members initialized from within the constructor.
>
> IDisposable is already supported. If you implement it on your fixture class, it
> will be called right after TestFixtureTearDown.

Great, I didn't know that. If the ctor behaves like TestFixtureSetUp,
it's logical to have IDisposable behave like TestFixtureTearDown.

In the new model, those semantics would be changed to SetUp and
TearDown semantics.

Fabian

Charlie Poole

unread,
Aug 30, 2012, 10:48:58 AM8/30/12
to nunit-...@googlegroups.com
Hi Fabian,

> [...]
>
>> Yes... with the addition that the constructor is actually guaranteed _not_ to
>> run once per test! NUnit historically chose the pattern of use where
>> one instance of the fixture is used for all tests.
>
> Okay, so the constructor currently has the same semantics as a
> TestFixtureSetUp method?

With the distinction that an instance may be constructed at load time if needed.
For example, if you have a TestCaseSourceAttribute that specifies an instance
field, method or property.

In principle, this should cause no problems, but sometimes people make the
assumption that test case construction and execution both use the same instance.
That assumption can lead to errors.

> [...]
>
> Yep, I thought so. One thing to consider would be a replacement for
> TestFixtureSetUp and TestFixtureTearDown - now often used as a
> performance improvement for lengthy initialization code. Maybe static
> TestFixtureSetUp and TestFixtureTearDown methods could be the way to
> go then.

We currently allow static methods as equivalent to instance methods for tests,
setup, teardown, fixture setup and fixture teardown. This was motivated by
the fact that code analysis will sometimes flag instance methods that could
be made static. We'll probably want to keep that and do some thing with
the names of the attributes. I'm thinking of phasing in OneTimeSetUp and
OneTimeTearDown as a replacement, since it makes things clearer.

>> One possibility for NUnit 3.0 is to allow either model - single instance or one
>> per test - with one of them selected as the default. What do folks think of
>> this idea?
>
> I think the main reason why one would choose the "classic model"
> (assuming the two models are equivalent in functionality) is backwards
> compatibility. There are two compatibility concerns:
> - tests that have TestFixtureSetUp and TestFixtureTearDown instance
> methods, as those wouldn't (couldn't) work as expected any more, and
> - existing tests that rely on the guarantee that a test fixture is
> constructed only once, using the fixture's ctor as a replacement for
> TestFixtureSetUp (and IDisposable for TestFixtureTearDown).
>
> Yes, for such tests (of which we ourselves have quite many), a
> compatibility mode would be required, I think.

Where folks have (mis)used TestFixtureSetUp to save information in
instance fields, this will be a tricky transition. That's the main reason
to support the old approach.

If you (or anyone) is interested in working on this issue for 3.0, please
let me know. The first thing we need is to have the spec for this feature,
at http://www.nunit.org/wiki/doku.php?id=dev:specs:setup_and_teardown
revised and reviewed.

Charlie

Bryan Cook

unread,
Aug 30, 2012, 8:49:57 PM8/30/12
to nunit-...@googlegroups.com
I'm pretty sure MSTest instantiates a fixture per test and uses static fixture setup and teardown strategies. Personally, I'm not a big fan of this. I want to write BDD style tests where I use inheritance and nested contexts but static initialization has some limitations. I use MSTest at work.
 
First issue is what you've already pointed out, fixtures are recreated for every test.  In BDD, there's often one action per fixture (setup) and each test is a simple specification (assert). My tests in MSTest are unnecessarily slow for this reason. I would love to use NUnit's approach but I can't.
 
Second issue is that I want to use nested subclasses, where the context is specialized per subclass. Because setup is static I cannot override my initialization per fixture.
 
Third, static initialization requires static variables which is perfectly fine until you want to run your tests in parallel.
 
 
 
Reply all
Reply to author
Forward
0 new messages