Add [MemberCondition] attribute for static-member-based test conditions (#9070)

[Evangelink]

Fixes #9070.

Summary

Adds a new public attribute Microsoft.VisualStudio.TestTools.UnitTesting.MemberConditionAttribute that derives from ConditionBaseAttribute and lets users gate a test class or test method on one or more static bool members (property, field, or parameterless method) referenced by Type + member name — without having to write a dedicated ConditionBaseAttribute subclass per condition.

This mirrors the well-known [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] pattern from Microsoft.DotNet.XUnitExtensions used heavily by dotnet/runtime, dotnet/sdk, and dotnet/aspnetcore.

The name MemberCondition was picked to align with the existing OSConditionAttribute / CIConditionAttribute family and to clearly convey that the condition is resolved against a member identified by name (rather than a hard-coded predicate).

Example

public static class TestConditions
{
    public static bool IsNotMacOs => !RuntimeInformation.IsOSPlatform(OSPlatform.OSX);
    public static bool HasNetworkAccess() => /* … */;
}

[TestClass]
public class MyTests
{
    // Skipped unless both members return true.
    [TestMethod]
    [MemberCondition(typeof(TestConditions),
                     nameof(TestConditions.IsNotMacOs),
                     nameof(TestConditions.HasNetworkAccess))]
    public void Test_That_Needs_Network_On_Non_Mac()
    {
    }

    // Stacking multiple [MemberCondition] attributes also ANDs them.
    [TestMethod]
    [MemberCondition(typeof(TestConditions), nameof(TestConditions.IsNotMacOs))]
    [MemberCondition(typeof(TestConditions), nameof(TestConditions.HasNetworkAccess))]
    public void Equivalent_To_Above()
    {
    }

    // Exclude mode flips the semantics: skip when the condition holds.
    [TestMethod]
    [MemberCondition(ConditionMode.Exclude, typeof(TestConditions), nameof(TestConditions.IsNotMacOs))]
    public void Mac_Only()
    {
    }
}

Behavior

• Multiple member names within a single attribute are AND-combined inside IsConditionMet.
• Each attribute instance gets a unique GroupName derived from the type and members, so stacking multiple [MemberCondition] attributes ANDs them across instances (this matches how the existing AttributeHelpers.IsIgnored groups conditions — OR within a group, AND across groups).
• ConditionMode.Exclude flips the semantics (skip when the condition holds), consistent with OSConditionAttribute / CIConditionAttribute.
• Member resolution looks up public static bool property, field, or parameterless method (in that order). Non-public members are intentionally excluded — keep your condition helpers public, like the xUnit [ConditionalFact] pattern.
• On any resolution failure (missing member, non-bool, instance-only, parameterized method, …), IsConditionMet throws InvalidOperationException so the test fails with an Error outcome rather than silently skipping — avoiding the classic "typo in member name silently disables the test" pitfall.

API surface

Four constructor overloads:

public MemberConditionAttribute(Type conditionType, string conditionMemberName);
public MemberConditionAttribute(Type conditionType, string conditionMemberName, params string[] additionalConditionMemberNames);
public MemberConditionAttribute(ConditionMode mode, Type conditionType, string conditionMemberName);
public MemberConditionAttribute(ConditionMode mode, Type conditionType, string conditionMemberName, params string[] additionalConditionMemberNames);

Uses DynamicallyAccessedMembers annotations on the Type parameter for trimming/AOT correctness (covers public properties, fields, and methods). Targets all existing TFMs (netstandard2.0, net462, net8.0, net9.0).

PublicAPI.Unshipped.txt updated accordingly.

Tests

Adds 23 unit tests in TestFramework.UnitTests/Attributes/MemberConditionAttributeTests.cs covering:

• Constructor argument validation (null/empty member names, null type)
• Member resolution variants (public property/field/method; non-public members are rejected with InvalidOperationException)
• AND-semantics across multiple member names within one attribute
• ConditionMode.Exclude
• GroupName uniqueness for distinct attribute instances
• Error messages and exception types on resolution failure

All 23 tests pass on net8.0 (1252/1252 in TestFramework.UnitTests). MSTestAdapter.PlatformServices still builds clean (no churn needed — the existing AttributeHelpers.IsIgnored already handles arbitrary ConditionBaseAttribute subclasses).

Follow-ups

• The error messages emitted by IsConditionMet on resolution failure are currently inline strings. A small follow-up could move them to FrameworkMessages.resx for localization; doing it here would have inflated the diff with the 14-language .xlf regeneration.
• A future analyzer to validate that the referenced member exists and is a static bool property/field/parameterless method (similar to MSTest's existing DataRow/DynamicData analyzers) would catch typos at build time — worth a separate issue.

[Copilot]

Pull request overview

Adds a new MSTest public attribute, Microsoft.VisualStudio.TestTools.UnitTesting.ConditionAttribute, enabling conditional execution of test methods/classes based on one or more referenced static bool members (property/field/parameterless method) specified by Type + member name, along with unit tests and public API declarations.

Changes:

• Introduces ConditionAttribute implementation (member resolution + AND semantics + grouping + include/exclude modes).
• Adds unit tests validating constructor validation, resolution behavior, grouping, and failure modes.
• Updates PublicAPI.Unshipped.txt to include the new public API surface.

Show a summary per file

File	Description
test/UnitTests/TestFramework.UnitTests/Attributes/ConditionAttributeTests.cs	Adds unit tests covering ConditionAttribute behavior and error cases.
src/TestFramework/TestFramework/PublicAPI/PublicAPI.Unshipped.txt	Declares the new public API entries for ConditionAttribute.
src/TestFramework/TestFramework/Attributes/TestMethod/ConditionAttribute.cs	Implements the new ConditionAttribute (reflection-based member evaluation, grouping, and messages).

Copilot's findings

• Files reviewed: 3/3 changed files
• Comments generated: 2

[Evangelink]

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.

Expert Review — PR #9071 [Condition] attribute

Overall the implementation is well-structured, correctly annotated for AOT/trimming ([DynamicallyAccessedMembers]), follows the existing ConditionBaseAttribute contract, and has solid positive-path test coverage. Three actionable issues below; one is a design correctness concern that should be fixed before this ships as public API.

---

Verdict Table

#	Dimension	Status	Severity
1	Algorithmic Correctness	⚠️ ISSUE	MODERATE
2	Threading & Concurrency	✅ LGTM	—
3	Security & IPC Contract Safety	✅ LGTM	—
4	Public API & Binary Compatibility	⚠️ ISSUE	MODERATE
5	Performance & Allocations	⚠️ ISSUE	MODERATE
6	Cross-TFM Compatibility	✅ LGTM	—
7	Resource & IDisposable Management	N/A	—
8	Defensive Coding at Boundaries	✅ LGTM	—
9	Localization & Resources	✅ LGTM	—
10	Test Isolation	✅ LGTM	—
11	Assertion Quality	✅ LGTM	—
12	Flakiness Patterns	✅ LGTM	—
13	Test Completeness & Coverage	⚠️ ISSUE	MODERATE
14	Data-Driven Test Coverage	✅ LGTM	—
15	Code Structure & Simplification	✅ LGTM	—
16	Naming & Conventions	✅ LGTM	—
17	Documentation Accuracy	✅ LGTM	—
18	Analyzer & Code Fix Quality	N/A	—
19	IPC Wire Compatibility	N/A	—
20	Build Infrastructure & Dependencies	✅ LGTM	—
21	Scope & PR Discipline	✅ LGTM	—
22	PowerShell Scripting Hygiene	N/A	—

18/22 dimensions clean (4 N/A).

---

Issues (inline comments carry the detail)

MODERATE — GroupName omits ConditionMode (line 208 of ConditionAttribute.cs)

GroupName is keyed on {type}:{members} only. Two attributes with the same type+member names but opposite modes (Include and Exclude) collide into the same group and are OR-combined in AttributeExtensions.IsIgnored. The net result is the test always runs regardless of the condition value — a silent no-op. Because GroupName is now public API, the fix must land before this ships.

Fix: append :{Mode} to the format string.

MODERATE — EvaluateMember performs fresh reflection on every IsConditionMet access (line 233 of ConditionAttribute.cs)

For a class-level [Condition], IsConditionMet is called once per test method; each call performs up to three Get{Property,Field,Method} lookups on the live Type. Caching the resolved MemberInfo (or a compiled Func<bool>) at construction time would both improve performance and surface misconfigurations eagerly.

MODERATE — method.ReturnType != typeof(bool) branch has no test (line 260 of ConditionAttribute.cs)

The Conditions fixture includes a non-bool property (NotABool) and a method with a parameter (WithParam), but no static parameterless method returning a non-bool. The branch method.ReturnType != typeof(bool) in EvaluateMember is therefore unreachable in the current test suite.

NIT — IgnoreMessage_Exclude_HasExpectedText doesn't assert the type name (line 103 of test file)

The Include variant asserts .Contain(typeof(Conditions).FullName!) but the Exclude variant does not, leaving that half of the message-formatting code under-asserted.

🤖 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. · 879.9 AIC · ⌖ 14.1 AIC · ◷

[Copilot]

Copilot's findings

• Files reviewed: 3/3 changed files
• Comments generated: 2

[Evangelink]

Per review feedback, pushed dd50f2b:

• Renamed ConditionAttribute → MemberConditionAttribute. The previous name was too generic and overloaded against the existing ConditionBaseAttribute / OSConditionAttribute / CIConditionAttribute family. MemberConditionAttribute better reflects that the attribute references a static member by name (matches the spirit of xUnit's [ConditionalFact]).
• Dropped the [CLSCompliant(false)] annotations on the params string[] overloads -- they aren't needed (see existing DataRowAttribute(params object?[]?) and DynamicDataAttribute(string, params object?[]) in the same CLS-compliant assembly). Build is clean with 0 warnings.

All 23 unit tests still pass on net8.0 (1252/1252 in TestFramework.UnitTests).

[Copilot]

Copilot's findings

• Files reviewed: 3/3 changed files
• Comments generated: 5

[Evangelink]

🧪 Test quality grade — PR #9071

26 tests graded from 1 new file (MemberConditionAttributeTests.cs). 21/26 earn A (90–100) and 5/26 earn B (80–89). The five B-grade tests each make a single focused assertion that fully covers their contract while a sibling test handles broader verification — an intentional and clean decomposition. Exception tests consistently pair a typed-throw check with a ParamName or WithMessage assertion. No tests fall below B; this is a strong submission.

Δ	Test	Grade	Band	Notes
new	MemberConditionAttributeTests. ConditionMemberNames_ CannotBeDowncastToMutableArray	B	80–89	Single type-assignability assertion adequately covers the immutability contract.
new	MemberConditionAttributeTests. Constructor_ ExplicitMode_ IsHonored	B	80–89	Single Mode assertion; intentionally focused — sibling covers the full constructor contract.
new	MemberConditionAttributeTests. GroupName_ DifferentMembers_ AreDifferentGroups	B	80–89	Single inequality assertion adequately verifies the distinctness contract.
new	MemberConditionAttributeTests. GroupName_ DifferentMode_ AreDifferentGroups	B	80–89	Single inequality assertion verifies that mode is encoded in the group name.
new	MemberConditionAttributeTests. GroupName_ SameTypeAndMembers_ AreSameGroup	B	80–89	Single equality assertion confirms group-name determinism.
new	MemberConditionAttributeTests. Constructor_ DefaultMode_ IsInclude	A	90–100	Three-property check covers Mode, ConditionType, and ConditionMemberNames in one focused test.
new	MemberConditionAttributeTests. Constructor_ EmptyAdditionalMemberNames_ Ok	A	90–100	No issues found.
new	MemberConditionAttributeTests. Constructor_ NullAdditionalMemberNames_ DoesNotThrow	A	90–100	No issues found.
new	MemberConditionAttributeTests. Constructor_ NullMemberName_ Throws	A	90–100	Exception plus ParamName check fully verifies the null-guard contract.
new	MemberConditionAttributeTests. Constructor_ NullType_ Throws	A	90–100	Exception plus ParamName check fully verifies the null-guard contract.
new	MemberConditionAttributeTests. Constructor_ WhitespaceAdditionalMemberName_ Throws	A	90–100	Exception plus ParamName check fully verifies the whitespace-guard contract.
new	MemberConditionAttributeTests. Constructor_ WhitespaceMemberName_ Throws	A	90–100	Exception plus ParamName check fully verifies the whitespace-guard contract.
new	MemberConditionAttributeTests. GroupName_ EncodesTypeAndMembers	A	90–100	Three Contains assertions verify all required components of the group-name format.
new	MemberConditionAttributeTests. IgnoreMessage_ Exclude_ HasExpectedText	A	90–100	No issues found.
new	MemberConditionAttributeTests. IgnoreMessage_ Include_ HasExpectedText	A	90–100	No issues found.
new	MemberConditionAttributeTests. IgnoreMessage_ MultipleMembers_ ListsAllWithAnd	A	90–100	No issues found.
new	MemberConditionAttributeTests. IsConditionMet_ InstanceProperty_ NotFoundForStaticLookup	A	90–100	No issues found.
new	MemberConditionAttributeTests. IsConditionMet_ MethodWithParameters_ FallsThroughAndThrows	A	90–100	No issues found.
new	MemberConditionAttributeTests. IsConditionMet_ MissingMember_ ThrowsInvalidOperation	A	90–100	No issues found.
new	MemberConditionAttributeTests. IsConditionMet_ MultipleMembers_ AndsValues	A	90–100	Covers all-true and two distinct one-false cases; thorough AND-logic coverage.
new	MemberConditionAttributeTests. IsConditionMet_ NonBoolProperty_ ThrowsInvalidOperation	A	90–100	No issues found.
new	MemberConditionAttributeTests. IsConditionMet_ NonPublicStaticProperty_ ThrowsInvalidOperation	A	90–100	No issues found.
new	MemberConditionAttributeTests. IsConditionMet_ ParameterlessMethodWithNonBoolReturn_ ThrowsInvalidOperation	A	90–100	No issues found.
new	MemberConditionAttributeTests. IsConditionMet_ StaticParameterlessMethod_ ReturnsValue	A	90–100	Both true and false cases verified; clear positive/negative pair.
new	MemberConditionAttributeTests. IsConditionMet_ StaticPublicField_ ReturnsValue	A	90–100	Both true and false cases verified; clear positive/negative pair.
new	MemberConditionAttributeTests. IsConditionMet_ StaticPublicProperty_ ReturnsValue	A	90–100	Both true and false cases verified; clear positive/negative pair.

_{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. · 256.1 AIC · ⌖ 14.1 AIC · [◷]( · ◷)