microsoft / testfx

MSTest framework and adapter
MIT License
697 stars 250 forks source link

Analyzer that flags `[ClassCleanup]` where the cleanup behavior is not set #3227

Closed Evangelink closed 1 month ago

Evangelink commented 1 month ago

Summary

Analyzer that flags [ClassCleanup] where the cleanup behavior is not set.

Background and Motivation

As of today, [ClassCleanup] will by default be run at the end of the assembly and not at the end of the class. Until we move to v4 and can break this behavior, it'd be interesting to introduce an analyzer that would flag when the behavior is not explicitely set so we can ensure the user really wanted to run at the end of the assembly.

Proposed Feature

Flag:

[TestClass]
public class C
{
    [ClassCleanup]
    public static Cleanup() {}
}

and the other overloads where the cleanup behavior is not specified.

Rule info:

Alternative Designs

We could have edited the rule MSTEST0011 but this rule should not be warning by default on a minor release as we don't want to break build for something that doesn't result in a guaranteed runtime error.

Evangelink commented 1 month ago

@engyebrahim I am wondering if we should go one step further and not only report when it's not explicitely set but always report if that's not set to EndOfClass. I don't see much sense in using this attribute for EndOfAssembly.

@cremor I see you upvoted this rule, would you be ok with that change of scope?

cremor commented 1 month ago

Yes, that would be ok for me. I upvoted this issue because I was surprised that there is even a configuration option, and even more by the default value.

Evangelink commented 1 month ago

I was surprised that there is even a configuration option, and even more by the default value.

Yeah we are doing many fun discoveries :) We can't wait for v4 for the big bang breaking changes of default and simplification of options.