Support class-level [Retry] attribute (#7556)#8566
Merged
Conversation
Resolves #7556. - Extend RetryAttribute / RetryBaseAttribute targets to include classes (Inherited = false, AllowMultiple = false). - TestMethodInfo now resolves retry from method first, then declaring class. Method-level retry fully overrides class-level. Multiple retry attributes at either level still throw TypeInspectionException. - New resource UTA_MultipleAttributesOnTestClass with localized xlf entries. - Update MSTEST0035 analyzer to also flag [Retry] on non-test classes; updated diagnostic text to mention test classes. - New unit tests in TestMethodInfoTests, UseRetryWithTestMethodAnalyzerTests and a new acceptance test ClassLevelRetryTests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds MSTest support for applying [Retry] (or any RetryBaseAttribute-derived attribute) at the test-class level, making it the default for all test methods in that class while allowing method-level [Retry] to fully override it. This spans the MSTest framework attribute definitions, adapter execution-time resolution/validation, analyzer guidance, and unit + acceptance test coverage.
Changes:
- Extend
RetryBaseAttribute/RetryAttributeto allowAttributeTargets.Classand document class-vs-method override semantics. - Update adapter execution (
TestMethodInfo) to resolve retry from method first, then class, and add a dedicated error for multiple class-level retry attributes. - Update MSTEST0035 analyzer/resources to allow
[Retry]on test classes (and keep flagging invalid usages), with new unit and acceptance tests.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TestMethodInfoTests.cs | Adds unit tests covering class-level retry resolution, method override semantics, non-inheritance, and duplicate class-level validation. |
| test/UnitTests/MSTest.Analyzers.UnitTests/UseRetryWithTestMethodAnalyzerTests.cs | Adds analyzer tests for [Retry] on [TestClass], non-test classes, and custom TestClassAttribute derivations. |
| test/IntegrationTests/MSTest.Acceptance.IntegrationTests/RetryTests.cs | Adds end-to-end acceptance scenario verifying class-level retry applies and method-level retry overrides. |
| src/TestFramework/TestFramework/Attributes/TestMethod/RetryBaseAttribute.cs | Allows class-level usage and documents precedence/non-inheritance behavior. |
| src/TestFramework/TestFramework/Attributes/TestMethod/RetryAttribute.cs | Allows class-level usage and documents full override semantics and non-inheritance. |
| src/Analyzers/MSTest.Analyzers/UseRetryWithTestMethodAnalyzer.cs | Extends MSTEST0035 to also analyze types, permitting retry on test classes while flagging non-test types/methods. |
| src/Analyzers/MSTest.Analyzers/Resources.resx | Updates analyzer title/message to include “test method or test class”. |
| src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hant.xlf | Updates localized resource entry metadata for changed analyzer strings. |
| src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hans.xlf | Updates localized resource entry metadata for changed analyzer strings. |
| src/Analyzers/MSTest.Analyzers/xlf/Resources.tr.xlf | Updates localized resource entry metadata for changed analyzer strings. |
| src/Analyzers/MSTest.Analyzers/xlf/Resources.ru.xlf | Updates localized resource entry metadata for changed analyzer strings. |
| src/Analyzers/MSTest.Analyzers/xlf/Resources.pt-BR.xlf | Updates localized resource entry metadata for changed analyzer strings. |
| src/Analyzers/MSTest.Analyzers/xlf/Resources.pl.xlf | Updates localized resource entry metadata for changed analyzer strings. |
| src/Analyzers/MSTest.Analyzers/xlf/Resources.ko.xlf | Updates localized resource entry metadata for changed analyzer strings. |
| src/Analyzers/MSTest.Analyzers/xlf/Resources.ja.xlf | Updates localized resource entry metadata for changed analyzer strings. |
| src/Analyzers/MSTest.Analyzers/xlf/Resources.it.xlf | Updates localized resource entry metadata for changed analyzer strings. |
| src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf | Updates localized resource entry metadata for changed analyzer strings. |
| src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf | Updates localized resource entry metadata for changed analyzer strings. |
| src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf | Updates localized resource entry metadata for changed analyzer strings. |
| src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf | Updates localized resource entry metadata for changed analyzer strings. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/Resource.resx | Adds new adapter resource string for multiple class-level retry attributes. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.zh-Hant.xlf | Adds new localized resource entry for class-level duplicate retry attribute error. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.zh-Hans.xlf | Adds new localized resource entry for class-level duplicate retry attribute error. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.tr.xlf | Adds new localized resource entry for class-level duplicate retry attribute error. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.ru.xlf | Adds new localized resource entry for class-level duplicate retry attribute error. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.pt-BR.xlf | Adds new localized resource entry for class-level duplicate retry attribute error. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.pl.xlf | Adds new localized resource entry for class-level duplicate retry attribute error. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.ko.xlf | Adds new localized resource entry for class-level duplicate retry attribute error. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.ja.xlf | Adds new localized resource entry for class-level duplicate retry attribute error. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.it.xlf | Adds new localized resource entry for class-level duplicate retry attribute error. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.fr.xlf | Adds new localized resource entry for class-level duplicate retry attribute error. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.es.xlf | Adds new localized resource entry for class-level duplicate retry attribute error. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.de.xlf | Adds new localized resource entry for class-level duplicate retry attribute error. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.cs.xlf | Adds new localized resource entry for class-level duplicate retry attribute error. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.Execution.cs | Adds a dedicated exception helper for multiple class-level retry attributes. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.cs | Updates retry attribute resolution to consider method then class, validating duplicates at both levels. |
Copilot's findings
- Files reviewed: 36/36 changed files
- Comments generated: 0
Evangelink
commented
May 25, 2026
Member
Author
Evangelink
left a comment
Evangelink
left a comment
There was a problem hiding this comment.
Review Summary
✅ 19/21 dimensions clean — 2 dimensions with findings (all NITs).
| # | Dimension | Verdict |
|---|---|---|
| 17 | Documentation Accuracy | 🟢 2 NIT |
| 15 | Code Structure & Simplification | 🟢 1 NIT |
Clean Dimensions (19/21)
All other dimensions passed review:
- ✅ Algorithmic Correctness: Logic correctly implements method-overrides-class semantics with proper validation of both levels
- ✅ Threading & Concurrency: No shared mutable state; uses cached reflection appropriately
- ✅ Public API & Binary Compatibility: Extends
AttributeUsageon existing attributes (additive change); no breaking changes; XML docs added - ✅ Performance: Uses
GetCustomAttributesCachedconsistently; no hot-path regressions - ✅ Cross-TFM Compatibility: No TFM-specific code; uses standard reflection APIs
- ✅ Resource Management: No new disposable resources introduced
- ✅ Defensive Coding: Validates duplicate class-level attributes even when method-level override exists (prevents silent misuse)
- ✅ Localization: New resource string
UTA_MultipleAttributesOnTestClassproperly added to.resx;.xlffiles regenerated correctly - ✅ Test Isolation: Tests use independent test classes with no shared static state
- ✅ Assertion Quality: Tests verify exact behavior (4 runs for class-level retry, 2 runs for method override)
- ✅ Test Completeness: 6 new unit tests cover class-level pickup, override (larger/smaller), non-inheritance, duplicate validation (with/without method override); 1 new acceptance test verifies end-to-end scenario
- ✅ Analyzer Quality:
UseRetryWithTestMethodAnalyzercorrectly extended to validate[Retry]on test classes and flag non-test classes; 3 new analyzer tests cover the new scenarios - ✅ Security: No security-relevant changes
- ✅ Flakiness Patterns: No timing dependencies or shared state
- ✅ Data-Driven Test Coverage: N/A
- ✅ Naming & Conventions: Consistent with codebase conventions
- ✅ IPC Wire Compatibility: N/A
- ✅ Build Infrastructure: N/A
- ✅ Scope & PR Discipline: Single concern (class-level retry); well-scoped
Findings Detail
Documentation Accuracy (2 NITs)
- Inline comment would clarify
??operator precedence semantics (line 199) - XML doc on
GetSingleRetryAttributecould be clearer about error-throwing purpose
Code Structure (1 NIT)
-
isClassLevelparameter could be renamed or replaced with enum for clarity (line 201)
Verdict
All findings are NITs — no blocking or major issues. The implementation is correct, well-tested, and follows MSTest conventions. The PR is ready to merge after addressing the nit-level documentation/readability suggestions if desired.
Great work on the comprehensive test coverage and thoughtful handling of the edge cases (duplicate validation even with method override, non-inheritance verification)! 🎉
Generated by Expert Code Review (on open) for issue #8566 · ● 1.6M
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #7556.
Summary
Adds support for decorating
[TestClass]types with[Retry(N)](or anyRetryBaseAttribute-derived attribute). When a class-level retry is present, every test method in the class retries on failure with the specified count. A method-level retry attribute, when present, fully overrides the class-level one (smaller, equal or larger).Assembly-level retry is intentionally out of scope here (the user only asked for classes, and assembly-level retry is already supported via
Microsoft.Testing.Extensions.Retryfor MTP users).Behavior
[Retry(N)][Retry(N)](existing)[Retry(N)][Retry(N)](new)[Retry(N)][Retry(M)][Retry(M)](method wins)TypeInspectionException(new messageUTA_MultipleAttributesOnTestClass)TypeInspectionException(existing message)Class-level duplicates are always validated, even when a method-level override is present, so misuse on the test class is never silenced.
RetryAttribute/RetryBaseAttributekeepInherited = false— base-class retry does not flow to a derived test class. There is a regression test for this.Changes
RetryBaseAttribute/RetryAttribute:AttributeUsageextended toMethod | Class; added remarks doc explaining override semantics.TestMethodInfo.GetRetryAttribute: rewritten to walk method then class attributes via a sharedGetSingleRetryAttributehelper.Resource.resx: newUTA_MultipleAttributesOnTestClassentry (xlf regenerated).UseRetryWithTestMethodAnalyzer(MSTEST0035): now also registersSymbolKind.NamedTypeand flags[Retry]on non-test classes; diagnostic text updated to mention test classes (xlf regenerated).Tests
TestMethodInfoTests: 6 new unit tests covering class-level pickup, method-overrides-class (both larger and smaller), non-inheritance from a base class, and class-level duplicate validation (with and without a method-level override).UseRetryWithTestMethodAnalyzerTests: 3 new tests covering[TestClass][Retry],[Retry]on non-test class, and custom-derivedTestClassAttribute.RetryTests(acceptance): newClassLevelRetryTestsend-to-end scenario exercising class-level retry plus per-method override.