It all started when I began my work on fixing some CA1001 warnings that have been lingering for a while. In case you are not familiar with it, it basically says that if your class has fields that are disposable, your class should be as well. And my mistake happened while fixing warnings in test classes, after my brilliant idea of setting my TearDown methods as the Dispose method.

Rise of the genius

The idea came to me when I was halfway through my test classes, after I found one which had a TearDown method. It took me a while to find one since it’s usually better to avoid this kind of method. I then took a moment to think about how could I best address this issue, and then it came to me: mix them all! I mean, they basically do the same thing, right?

My class would then look like this, for example:

class Tests : IDisposable
{
    private UseMeDisposeMe someClass;

    [SetUp]
    public void Setup()
        => someClass = new UseMeDisposeMe();

    [TearDown]
    public void Dispose()
        => someClass.Dispose();

    [Test]
    public void DoSomeTests()
    {
        //Test logic
    }
}

Downfall of the fool

The thing is, I didn’t realize that the TearDown/Dispose method would be called 1 extra time each time I ran my unit tests. Since TearDown runs after each unit test, and dispose is called after everything is done, that would result in us calling someClass after it has already been disposed.

The first problem with this is regarding performance. If we have 1000 test classes with the same pattern, that’s 1000 method calls that aren’t needed. And if our dispose logic is a bit crazy, we can get a big performance hit. Although one might argue that this isn’t an issue, since it won’t happen in a production environment, imagine hundred of developers triggering hundreds of builds. The slower it takes, the bigger the queue. And as average queue time increases, more infrastructure can be demanded. Which means more money spent that could be avoided.

Another issue is that some classes might not accept being disposed twice, causing some errors such as an ObjectDisposedException. Even though we try our best to mock everything that we are testing besides the system under test, sometimes it just can’t be helped due to the complexity.

Fixing the situation

If you find yourself in need of disposing fields in your test class, identify when they need to be disposed. For example, in our code above, we are initializing our someClass in the setup, so we could remove the TearDown method, and then only have the Dispose method. Another option is to suppress the error on the project, just be careful not to do it on the whole solution, these analyzers are there to help us. Usually.

---