[test-improver] test: add edge case tests for async-suffix suppressors and redundant display name analyzer - #9314
Conversation
…display name analyzer - RedundantTestMethodDisplayNameAnalyzer (MSTEST0071): add test for a custom attribute derived from TestMethodAttribute with DisplayName equal to the method name — exercises the Inherits() path that was previously untested. - UseAsyncSuffixTestMethodSuppressor (MSTEST0027): add test for a custom attribute derived from TestMethodAttribute (should suppress) and for an async method without any TestMethod attribute (should NOT suppress) — tests both the positive and negative boundaries of the Inherits() check. - UseAsyncSuffixTestFixtureMethodSuppressor (MSTEST0028): add test for a regular async method without any fixture attribute (should NOT suppress) — tests the negative boundary of the attribute check. All 16 affected tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR strengthens MSTest.Analyzers.UnitTests coverage by adding boundary/edge-case tests for MSTEST0027/MSTEST0028 suppressors and MSTEST0071 (redundant DisplayName) analyzer + code fix, specifically targeting previously untested inheritance and “should-not-suppress” paths.
Changes:
- Add a test validating MSTEST0071 triggers (and the fixer removes
DisplayName) when a custom attribute derives fromTestMethodAttribute. - Add tests for MSTEST0027 suppressor covering: derived
TestMethodAttribute(should suppress) and async method with no test attribute (should not suppress). - Add a test for MSTEST0028 suppressor ensuring an async method with no fixture attribute is not suppressed.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/MSTest.Analyzers.UnitTests/UseAsyncSuffixTestMethodSuppressorTests.cs | Adds suppressor coverage for derived TestMethodAttribute and confirms non-test async methods are not suppressed. |
| test/UnitTests/MSTest.Analyzers.UnitTests/UseAsyncSuffixTestFixtureMethodSuppressorTests.cs | Adds a negative test to confirm no fixture attribute means no suppression. |
| test/UnitTests/MSTest.Analyzers.UnitTests/RedundantTestMethodDisplayNameAnalyzerTests.cs | Adds derived-attribute test ensuring MSTEST0071 diagnostics and code-fix behavior work for inherited DisplayName. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 0
🧪 Test quality grade — PR #9314
This advisory comment was generated automatically. Grades are heuristic
|
Amaury Levé (Evangelink)
left a comment
There was a problem hiding this comment.
Note
🤖 Automated review 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 workflow. To request a follow-up action, reply by tagging @copilot directly.
✅ 22/22 dimensions clean — no findings.
Review summary (test-only PR — 3 files changed):
Dimensions were assessed against the diff, with test/UnitTests/ hotspot dimensions (Test Isolation, Assertion Quality, Flakiness, Test Completeness, Analyzer Quality) weighted most heavily.
| Dimension | Notes |
|---|---|
| Algorithmic Correctness | All four new test methods correctly verify the production paths they name. WhenCustomDerivedAttributeDisplayNameEqualsMethodName_Diagnostic exercises Inherits() in RedundantTestMethodDisplayNameAnalyzer; both suppressor "no-attribute" tests reach the Any(attr => ...) loop (MSTest assemblies are always present in the compilation — see CSharpCodeFixVerifier<...>.Test constructor lines 30–31, which unconditionally add MetadataReference.CreateFromFile(typeof(TestContext).Assembly.Location)) rather than short-circuiting at TryGetOrCreateTypeByMetadataName. |
| Assertion Quality | The two-phase pattern (without suppressor → WithIsSuppressed(false), with suppressor → WithIsSuppressed(false/true)) is consistent with the existing test style and provides independent signal in both phases. |
| Test Completeness | All three stated gap scenarios are covered. The asymmetry between the two suppressors (MSTEST0027 uses .Inherits(), MSTEST0028 uses exact SymbolEqualityComparer equality) is correctly reflected — no custom-derived-fixture-attribute positive test is needed because that path doesn't exist in production code. |
| Test Isolation | Each test holds its code snippet as a local string code; no shared mutable state. |
| Flakiness | No timing, ports, filesystem, or order-dependent assertions. |
| Analyzer & Code Fix Quality | WhenCustomDerivedAttributeDisplayNameEqualsMethodName_Diagnostic correctly verifies that the fixer removes the sole DisplayName argument, collapsing [CustomTestMethod(DisplayName = "MyTestMethod")] → [CustomTestMethod], matching the production fixer's existing behavior for TestMethod. |
| All other dimensions | N/A — no production code, resources, public API, IPC, or build files changed. |
Petr Pokorny (0101)
left a comment
There was a problem hiding this comment.
Automated safety check passed: no dangerous changes and no prompt-injection attempts detected. Approving as requested. Note: this is a quick safety sanity check, not a full code review.
Goal and Rationale
Three analyzer test files had untested boundary paths. This PR fills the most meaningful gaps:
RedundantTestMethodDisplayNameAnalyzer(MSTEST0071)TestMethodAttributewithDisplayName = "MethodName"→ should report diagnostic (theInherits()path was untested)UseAsyncSuffixTestMethodSuppressor(MSTEST0027)TestMethodAttribute→ should suppress; async method with noTestMethodattribute → should not suppressUseAsyncSuffixTestFixtureMethodSuppressor(MSTEST0028)Approach
Added one test to
RedundantTestMethodDisplayNameAnalyzerTestsand two to each of the two suppressor test files. All follow the existing pattern: run once without the subject analyzer/suppressor to confirm the raw diagnostic, then again with it to confirm the expected suppression state.RedundantTestMethodDisplayNameAnalyzer — new test verifies the code-fix also removes
DisplayNamefrom a custom derived attribute, which exercises both the diagnostic path and the fixer's generic attribute-argument removal.UseAsyncSuffixTestMethodSuppressor — the
Inherits()call inReportSuppressionsdeliberately handles custom subclasses ofTestMethodAttribute, but that path had no coverage.UseAsyncSuffixTestFixtureMethodSuppressor — all three existing tests only covered the suppressed case. The new test confirms a plain async method is left untouched.
Coverage Impact
Exercises previously uncovered branches in:
RedundantTestMethodDisplayNameAnalyzer.AnalyzeSymbol— theInherits()branch for non-[TestMethod]/[DataTestMethod]attributesUseAsyncSuffixTestMethodSuppressor.ReportSuppressions— both the suppress-path for custom attributes and the skip-path when no TestMethod attribute is presentUseAsyncSuffixTestFixtureMethodSuppressor.ReportSuppressions— the skip-path when no fixture attribute is presentTest Status
Build: ✅ succeeded (0 warnings, 0 errors)
RedundantTestMethodDisplayNameAnalyzerTestsUseAsyncSuffixTestMethodSuppressorTestsUseAsyncSuffixTestFixtureMethodSuppressorTestsAll 16 tests pass.
Reproducibility
Add this agentic workflows to your repo
To install this agentic workflow, run