[test-improver] test: add DataTestMethod edge cases for TypeContainingTestMethodShouldBeATestClassAnalyzer (MSTEST0030)

[Evangelink]

Goal and Rationale

TypeContainingTestMethodShouldBeATestClassAnalyzer (MSTEST0030) flags non-test classes that contain test methods (direct or inherited). The analyzer detects test methods via attribute.AttributeClass.Inherits(testMethodAttributeSymbol), which should recognize DataTestMethodAttribute since it inherits from TestMethodAttribute. Two concrete scenarios were not explicitly tested:

1. DataTestMethod directly on a method in a non-test class — the Inherits() check should recognize DataTestMethodAttribute as a test method, triggering the diagnostic.

2. DataTestMethod inherited from an abstract base class — the inheritance walk (while (currentType is not null)) should traverse up to the abstract base and detect the DataTestMethod. The abstract base is exempt (per the IsAbstract guard), but the concrete derived class should fire the diagnostic.

Approach

Added two new [TestMethod] test cases to TypeContainingTestMethodShouldBeATestClassAnalyzerTests.cs:

• WhenNonTestClassHasDataTestMethod_Diagnostic — confirms [DataTestMethod] (first-party TestMethodAttribute subclass) triggers the diagnostic and the code fix correctly adds [TestClass]
• WhenNonTestClassInheritsDataTestMethodFromAbstractBase_Diagnostic — confirms the inheritance walk detects [DataTestMethod] from an abstract base, firing only on the concrete derived class (abstract base is exempt)

Test Status

Test run summary: Passed!
  total: 16
  failed: 0
  succeeded: 16
  skipped: 0
  duration: 7s 373ms

✅ All 16 tests pass (MSTest.Analyzers.UnitTests, net8.0, Debug — 14 original + 2 new).

Reproducibility

./build.sh --restore   # first run only
.dotnet/dotnet test test/UnitTests/MSTest.Analyzers.UnitTests/MSTest.Analyzers.UnitTests.csproj \
  -f net8.0 --no-build -c Debug \
  --filter "ClassName=MSTest.Analyzers.Test.TypeContainingTestMethodShouldBeATestClassAnalyzerTests"

Trade-offs

• Tests exercise existing code paths only — no production changes, no maintenance burden beyond the tests.
• Both tests document how the Inherits() check handles DataTestMethodAttribute (the built-in TestMethodAttribute subclass), complementing the existing test for custom subclasses.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Test Improver workflow. · 1K AIC · ⌖ 23 AIC · [◷]( · ◷)

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/test-improver.md@main

[Copilot]

Pull request overview

Adds regression/edge-case coverage to the MSTest analyzer test suite for MSTEST0030 (TypeContainingTestMethodShouldBeATestClassAnalyzer), specifically ensuring that [DataTestMethod] is treated as a test method (via inheritance from TestMethodAttribute) both when applied directly and when inherited from an abstract base type.

Changes:

• Add a new code-fix test verifying a diagnostic is produced when a non-test class contains a [DataTestMethod].
• Add a new code-fix test verifying the analyzer’s base-type walk detects a [DataTestMethod] declared on an abstract base type and reports only on the concrete derived non-test class.

Show a summary per file

File	Description
test/UnitTests/MSTest.Analyzers.UnitTests/TypeContainingTestMethodShouldBeATestClassAnalyzerTests.cs	Adds two new analyzer/code-fix test cases covering [DataTestMethod] direct usage and inherited-from-abstract-base scenarios for MSTEST0030.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 0

[Evangelink]

PR #9092 — Six-Dimension Test Review

---

D13 · Test Completeness & Coverage — ISSUE

SEVERITY: MODERATE
FILE: test/UnitTests/MSTest.Analyzers.UnitTests/TypeContainingTestMethodShouldBeATestClassAnalyzerTests.cs
LINES: 466–532

Positive paths covered: Both stated scenarios are correctly exercised — direct [DataTestMethod] triggers the diagnostic, and [DataTestMethod] inherited from an abstract base also triggers the diagnostic on the derived concrete class.

Missing negative test — [TestClass] + [DataTestMethod]:
There is no explicit test asserting that a class already decorated with [TestClass] and containing [DataTestMethod] produces no diagnostic. The existing WhenTestClassHasTestMethod_NoDiagnostic covers [TestMethod], and the analyzer's isTestClass early-return path is attribute-agnostic, so the behavior is implicitly correct — but the symmetry is incomplete. The pattern across the file pairs every diagnostic test with a corresponding no-diagnostic test (e.g., WhenAbstractClassWithoutTestAttribute_HaveTestMethod_NoDiagnostic mirrors the abstract-base scenario added here). A matching WhenTestClassHasDataTestMethod_NoDiagnostic would close this gap.

Abstract-base negative path:
This is adequately covered: AbstractBase in the second test has no [|...|] marker, confirming it produces no diagnostic, and WhenAbstractClassWithoutTestAttribute_HaveTestMethod_NoDiagnostic establishes the general rule. No gap here.

[DataTestMethod] without [DataRow]:
The [DataRow(1)] in both snippets is irrelevant to the analyzer (it only checks attribute presence), so no additional test is required on this axis.

RECOMMENDATION: Add one negative test:

[TestMethod]
public async Task WhenClassWithoutTestAttribute_HaveDataTestMethod_NoDiagnostic_WhenTestClassPresent()
{
    string code = """
        using Microsoft.VisualStudio.TestTools.UnitTesting;

        [TestClass]
        public class MyTestClass
        {
            [DataTestMethod]
            [DataRow(1)]
            public void TestMethod1(int value) {}
        }
        """;

    await VerifyCS.VerifyAnalyzerAsync(code);
}

---

D14 · Data-Driven Test Coverage — LGTM

The [DataRow(1)] value and int value parameter type in the test snippets are irrelevant to the analyzer — it checks only for the presence of [DataTestMethod] (which inherits from [TestMethod]), never the row data. The snippets are syntactically valid and representative. The test methods themselves are correctly non-parameterized async tasks. No gaps.

---

D15 · Code Structure & Simplification — LGTM

Both new tests follow the exact same structure as every other test in the file: a raw string code, an optional raw string fixedCode, and a single await VerifyCS.VerifyCodeFixAsync(code, fixedCode) call. No unnecessary complexity, no dead code, no deviation from the established style.

---

D16 · Naming & Conventions — ISSUE

SEVERITY: NIT
FILE: test/UnitTests/MSTest.Analyzers.UnitTests/TypeContainingTestMethodShouldBeATestClassAnalyzerTests.cs
LINES: 467, 496

The established naming convention throughout the file uses the prefix WhenClassWithoutTestAttribute_ (e.g., WhenClassWithoutTestAttribute_HaveTestMethod_Diagnostic, WhenClassWithoutTestAttribute_HasInheritedTestMethodAttribute_Diagnostic). The two new methods use WhenNonTestClass... instead, which diverges from that pattern.

Additionally, the first new test uses _HaveDataTestMethod_ (verb form matching the existing _HaveTestMethod_ tests) would be more consistent than _HasDataTestMethod_.

RECOMMENDATION:

• WhenNonTestClassHasDataTestMethod_Diagnostic → WhenClassWithoutTestAttribute_HaveDataTestMethod_Diagnostic
• WhenNonTestClassInheritsDataTestMethodFromAbstractBase_Diagnostic → WhenClassWithoutTestAttribute_InheritsDataTestMethodFromAbstractBase_Diagnostic

---

D17 · Documentation Accuracy — LGTM

The comment in WhenNonTestClassInheritsDataTestMethodFromAbstractBase_Diagnostic:

// Abstract base is exempt from the diagnostic; derived non-test class
// that inherits [DataTestMethod] via the inheritance walk should fire.

This is accurate. The abstract-class exemption comes from the namedTypeSymbol.IsAbstract guard (explicit early return). The "inheritance walk" correctly describes the while (currentType is not null && !hasTestMethod) loop that traverses base types. The comment adds modest value by explaining why the abstract base is exempt, which is not fully captured by the method name alone.

---

D21 · Scope & PR Discipline — LGTM

The PR is precisely scoped: two test methods added to a single test file, no production code changes, no unrelated modifications. The stated goal (add [DataTestMethod] coverage) matches the diff exactly.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review (on PR ready) workflow. · 571.5 AIC · ⌖ 13.2 AIC · ◷

[Evangelink]

🧪 Test quality grade — PR #9092

Δ	Test	Grade	Band	Notes
new	TypeContainingTestMethodShouldBeATestClassAnalyzerTests. WhenClassWithoutTestAttribute_ InheritsDataTestMethodFromAbstractBase_ Diagnostic	B	80–89	Body is 38 lines, slightly over the ~30-line threshold; extra length comes from the AbstractBase + derived-class inheritance scenario in the embedded snippets.
new	TypeContainingTestMethodShouldBeATestClassAnalyzerTests. WhenClassWithoutTestAttribute_ HasDataTestMethod_ Diagnostic	A	90–100	No issues found.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 195.6 AIC · ⌖ 13.1 AIC · [◷]( · ◷)