I am using C#. Is it advised to unit test dispose methods? If so why, and how should one test these methods?
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.
}
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.
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.
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.