views:

88

answers:

4

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

+1  A: 

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.
        }
Jesse C. Slicer
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.
Kirk
@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.
Jesse C. Slicer
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?
sbenderli
@sbenderli, the `using` statement is a shorthand for disposing of an object when the scope is exited.
Jesse C. Slicer
ahh yes, dumb question. Thanks!
sbenderli
+2  A: 

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.

Neal
+2  A: 

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.

Jeff Meatball Yang
+2  A: 

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.

Igor Zevaka