Static analysis dispose warning in test class when object is disposed in test cleanup

Question

I have a lot of test classes like this.

[TestClass]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1001:TypesThatOwnDisposableFieldsShouldBeDisposable")]
public class TestClass
{
    private IDisposable _disposable;

    [TestInitialize]
    public void TestInitialize()
    {
        _disposable = //new disposable object...;
    }

    [TestCleanup]
    public void TestCleanup()
    {
        _disposable.Dispose();
    }

    [TestMethod]
    public void Test1()
    {
        //Uses _disposable
    }

    [TestMethod]
    public void Test2()
    {
        //Uses _disposable
    }

    [TestMethod]
    public void TestN()
    {
        //Uses _disposable
    }
}

Static analysis with FxCop results in the following warning because I do not implement the dispose pattern on my test class.

"CA1001: Types that own disposable fields should be disposable"

Right now I just suppress the message in the source, but I feel like there has to be a better way than cluttering all my tests with SuppressMessageAttribute. This seems like it would be a common pattern in testing - create object for the test and then dispose it after the test. I cannot just implement IDisposable on the test class because only one test object is created for all test methods. I want to dispose this object between every test method.

I know I could create the object in each test and dispose it in the test, but I'd rather continue to use SuppressMessageAttribute over copying and pasting the same code into each test method. It seems like the lesser of the two evils. Is there a better way to create a disposable object before each test and dispose it after each test that doesn't result in warning CA1001?

Thanks for the help in advance.

*"I cannot just implement IDisposable on the test class because only one test object is created for all test methods".* Not with the code you posted, `[TestInitialize]` runs for **every test** not just once. If you want to run it once, you need to use `[ClassInitialize]`. Try adding trace calls to the initialize function, you'll see its called each method. — Ron Beyer, Apr 29 '15 at 17:50
Actually with VS 2013 (and 2012 I suspect), the testing framework will create an instance of the test class per test, so implementing IDisposable would work as you want it to. — Mike Zboray, Apr 29 '15 at 17:55
Also do you think that you get a lot out of running FxCop on test assemblies? I've never seen the need to do this. Just way too many false positives. — Mike Zboray, Apr 29 '15 at 17:56

score 7 · Accepted Answer · answered Apr 29 '15 at 17:50

7

The best way I've found is to implement IDisposable in the test class and mark the Dispose method with the TestCleanup attribute.

[TestClass]
public class TestClass : IDisposable
{
    private IDisposable _disposable;

    [TestInitialize]
    public void TestInitialize()
    {
        _disposable = //new disposable object...;
    }

    [TestCleanup]
    public void Dispose()
    {
        _disposable.Dispose();
    }

answered Apr 29 '15 at 17:50

Erik

5,355
25
39

2

Its not required to mark it as `TestCleanup`, the object is disposed each test anyway. – Ron Beyer Apr 29 '15 at 17:51
@RonBeyer Great call! I didn't know Dispose would be called between each test if the test class implemented IDisposable. I like this solution because it doesn't involve adding code just for the sake of getting rid of warnings. – r2_118 Apr 29 '15 at 18:19
@Erik can you please consider a small update to include what Ron mentioned there? It would make it easier to see for others (not everybody will read the comments). – julealgon Nov 06 '18 at 19:08
1

Also, I'd strongly suggest marking the class as `sealed` as that nets you a lot of extra guarantees when dealing with `IDisposable`. If you don't seal the class, FxCop will complain about your implementation probably as a possible inheritor could introduce a mess to the chain (it would ask you to make the method virtual and add a `IsDisposed` property etc). – julealgon Nov 06 '18 at 19:11

score 2 · Answer 2 · edited May 23 '17 at 12:09

2

The error is asking to implement IDisposable interface on your TestClass. (Here is a good discussion why it is necessary). Its implementation should be like:

public class TestClass :IDisposable
{

    public void Dispose()
    {
        if (!IsDisposed) //Check if _disposable is not disposed
            _disposable.Dispose();
    }

See: Implement IDisposable correctly

edited May 23 '17 at 12:09

Community

1
1

answered Apr 29 '15 at 17:47

Habib

219,104
29
407
436

Yes, there is nothing saying test classes can't be `IDisposable`, and the test framework disposes of them properly. – Ron Beyer Apr 29 '15 at 17:48
Yeah I could implement IDisposable. In that case I am just adding code to satisfy the FxCop warning without any real value, just like SuppressMessageAttribute. I was hoping that there was a pattern that played nicely with FxCop and did not involve adding code with the singular purpose of getting rid of warnings. – r2_118 Apr 29 '15 at 18:01
@r2_118, well if you are going to have a disposable field in your class, rules says, that, *that* particular class should be disposable as well. – Habib Apr 29 '15 at 18:02
@Habib I understand that this is the rule. However, I believe the purpose of static analysis is to support quality coding habits. If the static analysis is forcing you to create more code without a true purpose then it is doing the exact opposite of promoting quality code. It's cluttering your actual code with code that has no purpose, increasing the support load and effort to refactor when necessary. I don't want to settle for that and want to find a solution where all code has a real purpose and does not violate any FxCop rules. – r2_118 Apr 29 '15 at 18:14
@RonBeyer pointed out that Dispose is called between each test method, which I didn't know. I can work with that because I can implement IDisposable without having duplicate code in TestCleanup. – r2_118 Apr 29 '15 at 18:17
@r2_118, you can't follow all the FxCop rules,it is practically not possible, sometimes you have to consider them more like a *guidelines*, if you think that following one particular rule is creating some bad code for you, then you can disable that rule. Again, you don't have to follow them strictly, they are there to help you find code smells. – Habib Apr 29 '15 at 18:17
@Habib Agreed but I don't want to disable rule CA1001. It is very helpful in most cases. – r2_118 Apr 29 '15 at 18:21

Static analysis dispose warning in test class when object is disposed in test cleanup

2 Answers2