MSTEST0012 and MSTEST0016 contradict each other. One wants the containing class to be static, the other wants a non-static class.
Describe the bug
MSTEST0012 and MSTEST0016 contradict each other. One wants the containing class to be static, the other wants a non-static class.
MSTEST0012
The class shouldn't be static.
MSTEST0016
A test class should have at least one test method or be static and have methods that are attributed with [AssemblyInitialization] or [AssemblyCleanup].
Can I satisfy both rules or is this a bug?
Steps To Reproduce
- Copy this basic example into a new cs-file
The methods AssemblyInitialize and AssemblyCleanup are within a base class which other normal test classes derive from. Naturally, there are no tests within that base class. I don't know why MSTEST0012 wants me to mark a base class with [TestClass], but I'm okay with that.
using Microsoft.VisualStudio.TestTools.UnitTesting;
[TestClass]
public class BaseClass
{
public TestContext TestContext { get; set; } = null!;
[AssemblyInitialize]
public static void AssemblyInitialize(TestContext testContext)
{
}
[AssemblyCleanup]
public static void AssemblyCleanup()
{
}
[TestInitialize]
public void TestInitialize()
{
}
[TestCleanup]
public void TestCleanup()
{
}
}
Expected behavior
No analyzer suggestions or warnings
Actual behavior
Code Analyzer suggestion MSTEST0016 is displayed
Additional context
This also affects MSTEST0013 (AssemblyCleanup).
Hi @baumheld,
Thanks for ticket! There are indeed multiple things that are wrong on our side.
The documentation for MSTEST0012 and MSTEST0013 are incorrect (it's valid to have static class for [AssemblyInitialize] and [AssemblyCleanup] methods). The analyzers themselves are fine.
The documentation for MSTEST0016 should be improved to say that the class should be either abstract, have some test or be static (if it contains only [AssemblyInitialize] and/or [AssemblyCleanup] methods). We will also need to update the analyzer description.
There is nothing mandatory but I would probably recommend to have a different class for the AssemblyInitialize/AssemblyCleanup methods as the code is only executed once which could be confusing for some people when they see the base class.
Assuming this base class is really meant to have no tests, I would recommend to mark the class has abstract as it gives a clearer intent. You should not mark the class as static otherwise TestContext, TestInitialize and TestCleanup would not work (@engyebrahim could you double check if we would be reporting on these cases?).
Indeed, adding abstract and removing [TestClass] would be the cleanest way.
But currently with MSTest 2.6.0 this leads to MSTEST0012 and MSTEST0013.
public abstract class BaseClass
{
public TestContext TestContext { get; set; } = null!;
[AssemblyInitialize]
public static void AssemblyInitialize(TestContext testContext) // => MSTEST0012 Warning
{
}
[AssemblyCleanup]
public static void AssemblyCleanup() // => MSTEST0013 Warning
{
}
}
These warnings are actually correct. You should keep the [TestClass] attribute otherwise AssemblyInitialize and AssemblyCleanup will not be triggered.
Either you should have
[TestClass]
public abstract class BaseClass
{
public TestContext TestContext { get; set; } = null!;
[AssemblyInitialize]
public static void AssemblyInitialize(TestContext testContext)
{
}
[AssemblyCleanup]
public static void AssemblyCleanup()
{
}
[TestInitialize]
public void TestInitialize()
{
}
[TestCleanup]
public void TestCleanup()
{
}
}
or something like
[TestClass]
public static class GlobalHooks
{
[AssemblyInitialize]
public static void AssemblyInitialize(TestContext testContext)
{
}
[AssemblyCleanup]
public static void AssemblyCleanup()
{
}
}
public abstract class BaseClass
{
public TestContext TestContext { get; set; } = null!;
[TestInitialize]
public void TestInitialize()
{
}
[TestCleanup]
public void TestCleanup()
{
}
}