Should Dispose methods be unit tested?

Question

I am using C#. Is it advised to unit test dispose methods? If so why, and how should one test these methods?

Kissie

Dispose methods as in C# or do you mean C++ style destructors? – Baler

C# style Dispose methods – Kissie

Answer 1

Yes, but it might be hard. There are two things that can generally happen in Dispose implementation:

Unmanaged resources are released.

In this case it's pretty hard to verify that the code called, for example, Marshal.Release. A possible solution is to inject an object that can do the disposing and pass a mock to it during testing. Something to this effect:

interface ComObjectReleaser {
    public virtual Release (IntPtr obj) {
       Marshal.Release(obj);
    }
}

class ClassWithComObject : IDisposable {

    public ClassWithComObject (ComObjectReleaser releaser) {
       m_releaser = releaser;
    }

    // Create an int object
    ComObjectReleaser m_releaser;
    int obj = 1;
    IntPtr m_pointer = Marshal.GetIUnknownForObject(obj);

    public void Dispose() {
      m_releaser.Release(m_pointer);
    }
}

//Using MOQ - the best mocking framework :)))
class ClassWithComObjectTest {

    public DisposeShouldReleaseComObject() {
       var releaserMock = new Mock<ComObjectReleaser>();
       var target = new ClassWithComObject(releaserMock);
       target.Dispose();
       releaserMock.Verify(r=>r.Dispose());
    }
}

Other classes' Dispose method is called

The solution to this might not be as simple as above. In most cases, implementation of Dispose is not virtual, so mocking it is hard.

One way is to wrap up those other objects in a mockable wrapper, similar to what System.Web.Abstractions namespace does for HttpContext class - i.e. defines HttpContextBase class with all virtual methods that simply delegates method calls to the real HttpContext class.

For more ideas on how to do something like that have a look at System.IO.Abstractions project.

Ashlan

One should ideally also test the re-entrancy and concurrent calls to Dispose (in a multi-threaded application). – Unconformity

Answer 2

Certainly can't hurt. Client code may try to use an object of your class after it has disposed of it. If your class is composed of other IDisposable objects, you should always be throwing the ObjectDisposedException exception if it is in a state which it is no longer usable.

Of course, you should only be testing the external state of your object. In the example below, I've made the property Disposed external to give me the state.

Consider:

internal class CanBeDisposed : IDisposable
{
    private bool disposed;
    public bool Disposed
    {
        get
        {
            if (!this.disposed)
                return this.disposed;
            throw new ObjectDisposedException("CanBeDisposed");
        }
    }

    public void Dispose()
    {
        this.Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (!this.disposed)
        {
            if (disposing)
            {
                //// Dispose of managed resources.
            }
            //// Dispose of unmanaged resources.
            this.disposed = true;
        }
    }
}

So how I would test this is thus:

CanBeDisposed cbd;

using (cbd = new CanBeDisposed())
{
    Debug.Assert(!cbd.Disposed); // Best not be disposed yet.
}

try
{
    Debug.Assert(cbd.Disposed); // Expecting an exception.
}
catch (Exception ex)
{
    Debug.Assert(ex is ObjectDisposedException); // Better be the right one.
}

Burr

I don't see that you have proven anything useful happened. To be worthwhile you would need to check that the resources were actually freed up. If you were accessing a file, then you should check that you can now open it for modification. I am not sure how you would test that a database connection was actually closed. – Retaliation

@Kirk, like I said, internal state is usually not part of a unit test. Only the pieces that are viewable from outside the class are what one writes unit tests for. – Burr

Just a quick question on your test code, did you mean to call the dispose method for the object before you test, or is there something going on that i don't see that disposes of the object? – Kissie

@sbenderli, the using statement is a shorthand for disposing of an object when the scope is exited. – Burr

ahh yes, dumb question. Thanks! – Kissie

This has worked fine for me too. After several tests, I can close the selenium driver started in the TestInitialize. – Ogdon

Answer 3

If your class creates and works with unmanaged resources, then you should definitely ensure that Dispose works as you expect it to - although it could be argued it is more of an integration test due to the type of hoops you're going to have to jump through.

If your class only creates / uses managed resources ( i.e. they implement IDisposable ) then all you really need to ensure is that the Dispose method on these resources is invoked at the correct time - if you are using some form of DI then you can inject a mock and assert that Dispose was called.

Look at the complexity of your dispose methods - if they are only a couple of lines long with maybe 1 condition, ask yourself if there really is a benefit in unit testing them.

Flew

Answer 4

Big yes - if your situation requires you to implement a Dispose function - you better make sure it does what you think!

For example, we have classes that coordinate database tasks (think SSIS packages, but with SqlConnection and SqlCommand and SqlBulkCopy etc.).

If I don't properly implement my Dispose, I could have an uncommitted SqlTransaction, or dangling SqlConnection. This would be VERY bad if I were running multiple instances of these database tasks in series.

Ungracious

Answer 5

As a practical tip (because yes, you should test Dispose()) my experience has been that there are two ways to do so without too much hassle.

IDisposer

The first follows Igor's accepted answer - inject something like an IDisposer, so that you can call

public void Dispose()
{
    _disposer.Release(_disposable);
}

where

public interface IDisposer
{
    void Release(IDisposable disposable);
}

Then all you need to do is mock the IDisposer and assert that it's called once and you're golden.

Factory

The second, and my personal favourite, is to have a factory that creates the thing you need to test disposal of. This only works when the factory produces a mockable type (interface, abstract class), but hey, that's almost always the case, especially for something that's to be disposed. For testing purposes, mock the factory but have it produce a mock implementation of the thing you want to test disposal of. Then you can assert calls to Dispose directly on your mock. Something along the lines of

public interface IFooFactory
{
    IFoo Create(); // where IFoo : IDisposable
}

public class MockFoo : IFoo
{
    // ugly, use something like Moq instead of this class
    public int DisposalCount { get; privat set; }

    public void Dispose()
    {
        DisposalCount++;
    }
}

public class MockFooFactory
{
    public MockFoo LatestFoo { get; private set; }

    public IFoo Create()
    { 
        LatestFoo = new MockFoo();
        return LatestFoo;
    }
}

Now you can always ask the factory (which will be available in your test) to give you the latest MockFoo, then you dispose of the outer thing and check that DisposalCount == 1 (although you should use a test framwork instead, e.g. Moq).

Microsporangium