Skip to content

test: MSTEST0029 — add edge case tests for virtual/override and make-private fixer - #9785

Merged
Evangelink merged 2 commits into
mainfrom
dev/amauryleve/mstest0029-edge-cases
Jul 9, 2026
Merged

test: MSTEST0029 — add edge case tests for virtual/override and make-private fixer#9785
Evangelink merged 2 commits into
mainfrom
dev/amauryleve/mstest0029-edge-cases

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes #9758.

Improves test coverage for PublicMethodShouldBeTestMethodAnalyzer (MSTEST0029) by closing gaps in the existing test suite.

Changes

test/UnitTests/MSTest.Analyzers.UnitTests/PublicMethodShouldBeTestMethodAnalyzerTests.cs

  • Add WhenPublicVirtualMethodInTestClass_NoDiagnostic — exercises the IsVirtual early-return path: a public virtual method in a [TestClass] that does not implement any interface should produce no diagnostic.
  • Add WhenPublicOverrideMethodInTestClass_NoDiagnostic — exercises the IsOverride early-return path: a public override of a non-interface base-class method in a [TestClass] should produce no diagnostic.
  • Add WhenMethodIsPublicAndNotMarkedAsTestMethod_ChangeToPrivateFix — exercises CodeActionIndex = 1, the "Change accessibility to private" code fix offered by PublicMethodShouldBeTestMethodFixer, which was previously untested.
  • Fix WhenMethodIsPublicAndMarkedAsTestCleanup_NoDiagnostic — the test body used [TestInitialize] instead of [TestCleanup], making it a duplicate of the TestInitialize test and leaving the [TestCleanup] suppression path untested.

Test status

Pure test additions — no production code changed. All 19 PublicMethodShouldBeTestMethodAnalyzerTests pass:

Test run summary: Passed!
  total: 19, failed: 0, succeeded: 19, skipped: 0

… fixer

Add three new tests for PublicMethodShouldBeTestMethodAnalyzer (MSTEST0029):

1. WhenPublicVirtualMethodInTestClass_NoDiagnostic - exercises the IsVirtual
   early-return path. A public virtual method in a [TestClass] that does not
   implement any interface should produce no diagnostic.
2. WhenPublicOverrideMethodInTestClass_NoDiagnostic - exercises the IsOverride
   early-return path. A public override of a non-interface base-class method in
   a [TestClass] should produce no diagnostic.
3. WhenMethodIsPublicAndNotMarkedAsTestMethod_ChangeToPrivateFix - exercises
   CodeActionIndex=1, the 'Change accessibility to private' code fix.

Also fix WhenMethodIsPublicAndMarkedAsTestCleanup_NoDiagnostic: the test body was
using [TestInitialize] instead of [TestCleanup].

Fixes #9758

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 9, 2026 12:58

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

This PR improves unit-test coverage for PublicMethodShouldBeTestMethodAnalyzer (MSTEST0029) by closing three gaps in PublicMethodShouldBeTestMethodAnalyzerTests.cs and correcting one misleading existing test. It is a test-only change with no production code touched. The additions correctly target the analyzer's IsVirtual/IsOverride early-return branches and the fixer's previously-untested "change accessibility to private" code action.

Changes:

  • Adds WhenPublicVirtualMethodInTestClass_NoDiagnostic and WhenPublicOverrideMethodInTestClass_NoDiagnostic to cover the analyzer's virtual/override suppression paths.
  • Adds WhenMethodIsPublicAndNotMarkedAsTestMethod_ChangeToPrivateFix to cover the fixer's second code action (CodeActionIndex = 1).
  • Fixes WhenMethodIsPublicAndMarkedAsTestCleanup_NoDiagnostic to actually use [TestCleanup] instead of [TestInitialize], so the cleanup suppression path is genuinely exercised.
Show a summary per file
File Description
test/UnitTests/MSTest.Analyzers.UnitTests/PublicMethodShouldBeTestMethodAnalyzerTests.cs Adds three new tests (virtual, override, change-to-private fixer) and corrects the [TestCleanup] test body; all consistent with existing verifier patterns and diagnostic-location conventions.

Review details

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

@github-actions

This comment has been minimized.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🔍 Build Failure Analysis

Summary — The build fails because two new protected static methods in BaseSerializer.cs (ReadFields and WriteListPayload<T>) are not declared in the internal API tracking file. This is a pre-existing issue on main — it is unrelated to the test-only changes in this PR.

Root cause: Undeclared internal API members (RS0051)

The Microsoft.Testing.Platform project tracks its internal API surface via InternalAPI/InternalAPI.Shipped.txt and InternalAPI/InternalAPI.Unshipped.txt files. Two recently-added methods in BaseSerializer.cs were not registered in InternalAPI.Unshipped.txt, causing the Roslyn public API analyzer (RS0051) to fire on all three target frameworks (net8.0, net9.0, netstandard2.0).

The failure in Microsoft.Testing.Platform.csproj cascades to all downstream projects (Microsoft.Testing.Extensions.AzureDevOpsReport, Microsoft.Testing.Extensions.CrashDump, etc.) since they depend on it.

Affected symbols

Method File Line
BaseSerializer.ReadFields(Stream, Func<ushort, int, bool>) BaseSerializer.cs:359 359
BaseSerializer.WriteListPayload<T>(Stream, ushort, T[]?, Action<Stream, T>) BaseSerializer.cs:380 380

Proposed fix

Add the following two lines to src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt:

 #nullable enable
+static Microsoft.Testing.Platform.IPC.Serializers.BaseSerializer.ReadFields(System.IO.Stream! stream, System.Func<ushort, int, bool>! tryReadField) -> void
+static Microsoft.Testing.Platform.IPC.Serializers.BaseSerializer.WriteListPayload<T>(System.IO.Stream! stream, ushort fieldId, T[]? list, System.Action<System.IO.Stream!, T>! writeItem) -> void

Note: Since BaseSerializer is an internal abstract class and its other members are already tracked in the common InternalAPI/InternalAPI.Shipped.txt file (not the net/-specific one), the fix belongs in the common InternalAPI/InternalAPI.Unshipped.txt.

Impact on this PR: This PR only adds test cases to PublicMethodShouldBeTestMethodAnalyzerTests.cs — no production code was changed. The build failure is inherited from main and should be fixed there (or in this branch as a drive-by fix to unblock CI).


Build overview
Property Value
Status FAILED
Duration 154.3 s
MSBuild 18.8.0-preview-26302-115+f7b4c5716
Projects 49
Errors 7 (6 × RS0051 + 1 "Build failed")
Failed project Microsoft.Testing.Platform.csproj (net8.0, net9.0, netstandard2.0)
Cascading failures NonWindowsTests.slnf, AzureDevOpsReport.csproj, CrashDump.csproj
All MSBuild errors (7)
Code TFM File:Line Message
RS0051 net8.0 BaseSerializer.cs:359 ReadFields(...) is not part of the declared API
RS0051 net9.0 BaseSerializer.cs:359 ReadFields(...) is not part of the declared API
RS0051 netstandard2.0 BaseSerializer.cs:359 ReadFields(...) is not part of the declared API
RS0051 net8.0 BaseSerializer.cs:380 WriteListPayload<T>(...) is not part of the declared API
RS0051 net9.0 BaseSerializer.cs:380 WriteListPayload<T>(...) is not part of the declared API
RS0051 netstandard2.0 BaseSerializer.cs:380 WriteListPayload<T>(...) is not part of the declared API
Build failed.

🤖 Generated by the Build Failure Analysis workflow using (a href="(dev.azure.com/redacted) · commit 2839244

🤖 Automated content by GitHub Copilot. Generated by the Build Failure Analysis workflow. · 101.8 AIC · ⌖ 8.03 AIC · ⊞ 7.3K · [◷]( · )

@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.

✅ 22/22 dimensions clean — no findings.

Applicable dimensions verified (all clean): Algorithmic Correctness, Test Isolation, Assertion Quality, Test Completeness, Code Structure, Naming & Conventions, Analyzer & Code Fix Quality, Scope & PR Discipline.

N/A dimensions (test-only PR, no production code): Threading, Security, Public API, Performance, Cross-TFM, Resource Management, Defensive Coding, Localization, Flakiness, Data-Driven, Documentation, IPC Wire, Build Infrastructure, PowerShell.

Notes:

  • The [TestInitialize][TestCleanup] fix correctly eliminates a duplicate of the TestInitialize test and restores coverage of the testCleanupAttributeSymbol suppression path.
  • The new virtual/override tests appropriately use VerifyCS.VerifyAnalyzerAsync (analyzer-only verification) since these paths produce no diagnostic and no code-fix action is relevant.
  • The CodeActionIndex = 1 test correctly exercises the second registered code action (ChangeMethodAccessibilityToPrivateFix) from the fixer.

Copilot AI review requested due to automatic review settings July 9, 2026 16:35

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

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🧪 Test quality grade — PR #9785

GradeTestNotes
A (90–100) mod PublicMethodShouldBeTestMethodAnalyzerTests.
WhenMethodIsPublicAndMarkedAsTestCleanup_
NoDiagnostic
No issues found.
A (90–100) new PublicMethodShouldBeTestMethodAnalyzerTests.
WhenMethodIsPublicAndNotMarkedAsTestMethod_
ChangeToPrivateFix
Verifies both diagnostic detection and the "make private" code fix; CodeActionIndex = 1 is clear from the method name and fixedCode, though a brief inline comment (e.g. // change-to-private fix) would help future readers.
A (90–100) new PublicMethodShouldBeTestMethodAnalyzerTests.
WhenPublicOverrideMethodInTestClass_
NoDiagnostic
No issues found.
A (90–100) new PublicMethodShouldBeTestMethodAnalyzerTests.
WhenPublicVirtualMethodInTestClass_
NoDiagnostic
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. Generated by the Grade Tests on PR (on open / sync) workflow. · 56.6 AIC · ⌖ 6.16 AIC · ⊞ 9.5K · [◷]( · )

@Evangelink Evangelink added the state/needs-review Awaiting review from the team. label Jul 9, 2026
@Evangelink
Evangelink enabled auto-merge (squash) July 9, 2026 17:07
@Evangelink
Evangelink merged commit 88c5da4 into main Jul 9, 2026
39 checks passed
@Evangelink
Evangelink deleted the dev/amauryleve/mstest0029-edge-cases branch July 9, 2026 18:03
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: MSTEST0029 — add edge case tests for virtual/override and make-private fixer

3 participants