Skip to content

test: add edge-case tests for TestClassShouldBeValidAnalyzer static-class guard - #9965

Closed
Evangelink wants to merge 2 commits into
mainfrom
dev/amauryleve/testclass-valid-edge-cases
Closed

test: add edge-case tests for TestClassShouldBeValidAnalyzer static-class guard#9965
Evangelink wants to merge 2 commits into
mainfrom
dev/amauryleve/testclass-valid-edge-cases

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Goal

Fixes #9951.

The TestClassShouldBeValidAnalyzer (MSTEST0002) static-class validation branch had two untested paths. This PR adds two focused tests to TestClassShouldBeValidAnalyzerTests.

Changes

  • WhenStaticTestClassContainsDerivedTestMethodAttribute_Diagnostic — a static [TestClass] with [DataTestMethod] (a TestMethodAttribute subclass) fires MSTEST0002, exercising the attribute.AttributeClass.Inherits(testMethodAttributeSymbol) branch of the static-class guard.
  • WhenStaticTestClassContainsGlobalTestInitialize_NoDiagnostic — a static [TestClass] with only [GlobalTestInitialize] does NOT fire, confirming the analyzer intentionally omits that attribute from the static-class check (it only checks TestInitialize, TestCleanup, ClassInitialize, ClassCleanup, and TestMethod-derived attributes).

Verification

Built and ran MSTest.Analyzers.UnitTests on net472 and net8.0 — all tests pass.

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

…lass guard

Add two tests exercising the static-class validation branch:
- WhenStaticTestClassContainsDerivedTestMethodAttribute_Diagnostic: verifies
  that a static [TestClass] with [DataTestMethod] (a TestMethodAttribute
  subclass) fires MSTEST0002, covering the Inherits() path in the guard.
- WhenStaticTestClassContainsGlobalTestInitialize_NoDiagnostic: verifies
  that a static [TestClass] containing only [GlobalTestInitialize] is NOT
  flagged, because the guard does not check GlobalTestInitialize.

Fixes #9951

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2b0a5a99-adf4-4127-8801-229c5786331d
Copilot AI review requested due to automatic review settings July 15, 2026 08:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds edge-case coverage for MSTEST0002’s static-class guard.

Changes:

  • Tests derived TestMethodAttribute detection.
  • Tests omission of GlobalTestInitialize.
Show a summary per file
File Description
TestClassShouldBeValidAnalyzerTests.cs Adds two focused static-class analyzer tests.

Review details

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread test/UnitTests/MSTest.Analyzers.UnitTests/TestClassShouldBeValidAnalyzerTests.cs Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review Summary

Two new analyzer unit tests for TestClassShouldBeValidAnalyzer — one verifying that derived [TestMethod] attributes (e.g. [DataTestMethod]) trigger the static-class diagnostic, and one verifying that [GlobalTestInitialize] does not. Both are well-structured and follow existing conventions.

# Dimension Verdict
1 Algorithmic Correctness ✅ N/A — test-only, logic is straightforward
2 Threading & Concurrency ✅ N/A
3 Security & IPC Contract Safety ✅ N/A
4 Public API & Binary Compatibility ✅ N/A — no public API changes
5 Performance & Allocations ✅ N/A — test code only
6 Cross-TFM Compatibility ✅ N/A
7 Resource Management ✅ N/A
8 Error Handling ✅ N/A
9 Naming & Conventions ✅ Clean — method names follow When..._Diagnostic/_NoDiagnostic pattern
10 Documentation & Comments ✅ Clean — inline comments explain why each test exists
11 Test Quality ✅ Clean — tests are focused, comments explain the analyzer behavior being validated
12 Localization ✅ N/A
13 Telemetry ✅ N/A
14 Logging ✅ N/A
15 Configuration ✅ N/A
16 Serialization ✅ N/A
17 Build & Packaging ✅ N/A
18 Backward Compatibility ✅ N/A
19 Code Duplication ✅ Clean
20 Scope Discipline ✅ Clean — single concern
21 TODO Hygiene ✅ N/A — no TODOs
22 PowerShell & Script Hygiene ✅ N/A

Verdict: No issues found. Clean addition of two well-motivated test cases.

Address review feedback: GlobalTestInitialize requires exactly one
TestContext parameter, so the no-diagnostic sample now uses a valid
signature to isolate the static-class guard.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2b0a5a99-adf4-4127-8801-229c5786331d
Copilot AI review requested due to automatic review settings July 15, 2026 08:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test quality grade — PR #9965

GradeTestNotes
A (90–100) new TestClassShouldBeValidAnalyzerTests.
WhenStaticTestClassContainsDerivedTestMethodAttribute_
Diagnostic
Clear AAA; VerifyAnalyzerAsync with explicit location and arguments verifies the correct diagnostic fires. No issues found.
A (90–100) new TestClassShouldBeValidAnalyzerTests.
WhenStaticTestClassContainsGlobalTestInitialize_
NoDiagnostic
Concise negative test; comment clearly explains why [GlobalTestInitialize] must not trigger the diagnostic. No issues found.

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

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

🤖 Automated content by GitHub Copilot. Generated by the Grade Tests on PR (on open / sync) workflow. · 29.3 AIC · ⌖ 5.62 AIC · ⊞ 8.9K · [◷]( · )

@Evangelink Evangelink added the state/needs-review Awaiting review from the team. label Jul 15, 2026
@Evangelink

Copy link
Copy Markdown
Member Author

Closing in favor of #9967, which covers the same static-class guard gap and additionally verifies the code-fix output (converting \static class\ -> \class). Consolidating on the stronger of the two duplicate PRs.

@Evangelink Evangelink closed this Jul 15, 2026
@Evangelink
Evangelink deleted the dev/amauryleve/testclass-valid-edge-cases branch July 15, 2026 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-review Awaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[test-improver] test: add edge-case tests for TestClassShouldBeValidAnalyzer static-class guard

3 participants