3

I want to write unit tests for a class that implements IDisposable. The class has numerous private fields that also implement IDisposable. In my test, I want to verify that when I call Dispose(), it correctly calls Dispose() on all of its IDisposable fields. Essentially, I want my unit test to look like this:

var o = new ObjectUnderTest();
o.Dispose();
Assert.IsFalse(ObjectHasUndisposedDisposables(o));

I was thinking of using reflection to achieve this. It seems like this would be a fairly common requirement, but I can't find any examples of it.

Anyone tried this?

EDIT -- I do NOT want to have to inject the Disposables into the class under test.

Old Fox
  • 8,629
  • 4
  • 34
  • 52
user6118784
  • 33
  • 1
  • 4
  • ``o.IsDisposed == true`` ? in your case probably : ``Assert.IsTrue(o.IsDispoed);`` – Ehsan Sajjad May 16 '16 at 18:18
  • 1
    I recommend using a dependency injection container. Managing disposable dependencies is only one benefit. If your class is creating more dependencies that it needs to track and dispose then it's not depending on abstractions. With DI your class just depends on `IWhatever`. It doesn't know or care if that class is disposable. – Scott Hannen May 16 '16 at 18:52

4 Answers4

6

There is no generic way to handle this. the IDisposeable interface does not require you to track if dispose has been called or not.

The only way I could think of that you could handle this is if all of those disposable classes where injected using dependency injection. If you did that you could mock the injected classes and track if dispose was called.

    [TestMethod]
    public void TestAllDisposeablesAreDisposed()
    {
        var fooMock = new Mock<IFoo>();
        fooMock.Setup(x=>x.Dispose())
            .Verifiable("Dispose never called on Foo");

        var barMock = new Mock<IBar>();
        barMock.Setup(x => x.Dispose())
            .Verifiable("Dispose never called on Bar");

        using (var myClass = new MyClass(fooMock.Object, barMock.Object))
        {
        }

        fooMock.Verify();
        barMock.Verify();
    }
Scott Chamberlain
  • 124,994
  • 33
  • 282
  • 431
  • This is a great answer. If you are handling the disposal of your dependencies inside the class, write tests to verify this. They will break if you move the disposal logic outside of the class, which is a good thing. – xofz May 16 '16 at 18:25
  • Also, I don't mind wrapping all the Disposables I'm using in another interface (say ITrackedDisposable) which does track when they've been disposed. – user6118784 May 16 '16 at 18:26
6

The only way to verify the behavior you are looking for without any refactoring in your code is to use code weaving tools Eg; Typemock Isolator, MsFakes and etc...

The following snippet show the way to verify the behavior using MsFakes:

[TestMethod]
public void TestMethod1()
{
    var wasCalled = false;
    using (ShimsContext.Create())
    {
        ForMsFakes.Fakes.ShimDependency.AllInstances.Dispose = dependency =>
        {
            wasCalled = true;
        };

        var o = new ObjectUnderTest();

        o.Dispose();
    }

    Assert.IsTrue(wasCalled);
}

public class Dependency : IDisposable
{
    public void Dispose() {}
}

public class ObjectUnderTest: IDisposable
{
    private readonly Dependency _d = new Dependency();

    public void Dispose()
    {
        _d.Dispose();
    }
}
Old Fox
  • 8,629
  • 4
  • 34
  • 52
  • One drawback with this is it is not generic. To the best of my knowledge there is no automatic way to generate a shim during run time so you would need to manually generate a `ShimXxxxxx` for every disposable type in the class. And if the class is updated at a later date with a new disposable type the unit test will not check for it. This takes away half of the value of the unit test, because it does not protect you when new types are added. However there is a work around... – Scott Chamberlain May 17 '16 at 20:29
  • Do `var fieldCount = typeof(ClassUnderTest).GetFields(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic).Where(x=>typeof(IDisposable).IsAssignableFrom(x.FieldType)).Count(); Assert.AreEqual(KnownFieldCount, fieldCount, "The number of known fields did not match the class. Update the unit test with new dispose classes as needed");` as one of your tests. This will cause the test to fail if someone updates the class with a different number of disposables, however it does not flag if you both remove one and add one. – Scott Chamberlain May 17 '16 at 20:33
2

Are you trying to test whether your IDisposable behavior works within a given class, or do you want to test whether or not the whatever is managing your disposable class actually calls IDisposable? That's valid, if you're not sure that it's getting called and you want to be sure. (I've written tests for that.)

For that you can create a simple class like this:

public class DisposeMe : IDisposable
{
    public bool Disposed {get;set;}
    public void Dispose()
    {
        Disposed = true;
    }
}

Then you can just assert that Disposed is true when you expect it to have been disposed.

For example, I've used that when I'm creating classes from a DI container and I want to confirm that when a class created by the container is released that its disposable dependencies get disposed. I can't track that within the actual concrete classes, but I can test that the DI container is doing what I think it's supposed to be doing. (That's not a test I'd write over and over. Once I've confirmed the expected behavior of Windsor, for example, I'm not going to keep writing unit tests for it.)

Scott Hannen
  • 27,588
  • 3
  • 45
  • 62
  • I want to test that when I call ObjectUnderTest.Dispose(), it correctly calls Dispose() on all of its composed Disposables. I realize I can do this by injecting the disposables, but I think that's a bad design because then my interface has to change due to a change in implementation. It seems that reflection would be a much cleaner way to achieve this, and could be reused again and again on any IDisposable. – user6118784 May 16 '16 at 18:42
  • @user6118784 reflection of what? Any time you want to use reflection replace it with "make all fields public", So your sentence becomes "It seems that *making all fields public* would be a much cleaner way to achieve this, and could be reused again and again on any IDisposable". How would making the fields public solve your problem of knowing when someone called .Dispose() on one of those fields? You must intercept the call to be able to detect it, and without going out in to really exotic stuff like IL rewriting you have to use injection to do it. – Scott Chamberlain May 16 '16 at 18:48
  • To answer your questions: 1) Reflection of the private fields of ObjectUnderTest. 2) If I made the disposable fields public my unit test could easily test to see if they had been disposed - I would wrap all my disposables in something like an ITrackedDisposable (which keeps track of when Dispose() is invoked on them) to achieve this. – user6118784 May 16 '16 at 19:54
0

If you are unwilling to use dependency injection in the constructor because this would force you to change your interface then you could inject your dependencies via properties. Give your property a default value if nothing gets injected so that when you are using the class you do not have to remember to pass in the real implementation

E.g.

public class ObjectUnderTest : IDisposable
{
    private IObjectNeedingDisposal _foo;

    public IObjectNeedingDisposal Foo
    {
        get { return _foo ?? (_foo = new ObjectNeedingDisposal()); }
        set { _foo = value; }
    }

    public void MethodUsingDisposable()
    {
        Foo.DoStuff();
    }

    public void Dispose()
    {
        Foo.Dispose();
    }
}

In your tests you could then create an instance of your object and pass in the mock version of your class but in the real implementation you could leave it unset and it would default to an instance of the type you wanted

mark_h
  • 5,233
  • 4
  • 36
  • 52
  • Yeah, I had considered that approach too, but really I don't like adding public stuff to classes just for the purpose of unit testing. It seems clunky, and it's also code that cannot be reused. I know what I want to do can be done with reflection - I had hoped someone had already written it in a nice reusable module to save me the effort :-). – user6118784 May 16 '16 at 20:00
  • I agree that it's not perfect using properties in this way, I prefer passing them in constructor. I know you can use reflection to get private fields - http://stackoverflow.com/questions/95910/find-a-private-field-with-reflection but even then you would have missed the event and would not be able to tell after the fact that the Disposed method had been called. – mark_h May 16 '16 at 20:13