Private static field causes false positive for analyzer rule MSTEST0005

[abatishchev]

Describe the bug

A private static field of type TestContext causes a false positive for rule MSTEST0005:

Property 'TestContext' should be valid

Steps To Reproduce

The following code:

[TestClass]
public class MiseCorrelationIdProviderTests
{
    private static TestContext _context;

    [ClassInitialize]
    public static void ClassInitialize(TestContext context) =>
        _context = context;
}    

causes the following warning:

Expected behavior

A field does not trigger a rule for a property.

Actual behavior

It does.

Additional context

<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.12.0" />
<PackageVersion Include="MSTest.TestAdapter" Version="3.7.0" />
<PackageVersion Include="MSTest.TestFramework" Version="3.7.0" />
<GlobalPackageReference Include="MSTest.Analyzers" Version="3.7.0">
  <PrivateAssets>all</PrivateAssets>
  <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
</GlobalPackageReference>

[gao-artur]

This is actually a correct behavior. Most likely you are using the TestContext incorrectly. See this comment where I asked for this analyzer.

[abatishchev]

Let disagree that it's ok that a rule for a property is triggered by a field. Why?

For an incorrectly stored TextContext, there is already a different rule: MSTEST0024.

Overall, I found that TestContext is a messed up API:

• some members are fine to be accessed from a static methods: properties which come from run settings. They're static by design.
• some aren't: cancellation token, test name. They're per-test by design.

[Evangelink]

I agree that's the TestContext API is fully broken and that's why I want to change it but that implies a massive breaking change and so a massive pushback from users. We have been thinking about migration solution with @Youssef1313 and I think we have a solid design for V4.

[Youssef1313]

Analyzer wids, there is one concern for me here. We already have a separate analyzer specifically about static TestContext which is not enabled by default. But then MSTEST0005 is duplicate part of that logic which is enabled by default as warning. Do we want this case to be warning by default? Or should we let it be handled by MSTEST0024 which is not warning by default.

API wise, yeah we need to really change the design around TestContext.

[abatishchev]

Here are my scenarios of using TestContext (in an order of importance):

1. Reading parameters from run settings (in a static method which is a data source for [DynamicData])
2. TestContext.CancellationTokenSource.Token (I can create my own, but it would be nice to use the built-in)
3. TestContext.TestName (to start an Activity for OTel)

I was looking for a way to perform no. 1 without using TestContext, but apparently it's the only way. Unlike all others, which allow a workaround. Please suggest a path forward, if any.

Meanwhile, I submitted a small change to the doc, please take a look too: MicrosoftDocs/visualstudio-docs#10568