Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSTEST0012 and MSTEST0016 contradict each other. One wants the containing class to be static, the other wants a non-static class. #3818

Open
baumheld opened this issue Sep 12, 2024 · 3 comments

Comments

@baumheld
Copy link

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

image

Additional context

This also affects MSTEST0013 (AssemblyCleanup).

@Evangelink
Copy link
Member

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?).

@baumheld
Copy link
Author

baumheld commented Sep 12, 2024

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
    {
    }
}

@Evangelink
Copy link
Member

Evangelink commented Sep 12, 2024

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()
    {
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants