Design explicit tests for MSTest

[Jakub Jareš (nohwnd)]

Adds docs/RFCs/024-Explicit-Tests.md, a design for [Explicit] in MSTest that behaves the same on VSTest and Microsoft.Testing.Platform.

Explicit tests are always discovered and displayed, but a broad Run All reports them as skipped. They run only when the request positively selects them. To make that precise without guessing at IDE intent, the RFC splits every request into a constraint ("is this test in the run?") and an activation ("did the user choose this test?"). An exclusion filter, a policy filter supplied by an extension, and an empty server selection can all constrain a run, but none of them can start a destructive test.

Activation is defined per request shape: concrete test-case and UID selections, a positive branch of a TestCaseFilter / --filter expression, and a discriminating segment of a tree-node or server graph filter. Anything whose activation cannot be determined activates nothing, so a future filter feature cannot quietly start running these tests before its semantics are designed.

Also covers data rows and folding, inheritance and precedence against [Ignore] and conditions, the ExplicitTestMode override, retry behavior, reporting, old-adapter compatibility, implementation surfaces, and the test plan for both hosts.

No production code changes. This is the design investigation asked for in the issue.

Open questions

Approval decides two things the RFC deliberately leaves open. Each changes every surface at once and neither can be revised after the API ships, so neither belongs to whoever edits the file last.

• What the concept is called. Explicit is what NUnit, xUnit v3 and TUnit call it, so someone arriving from any of them searches for [Explicit]. OnDemand describes the behavior this document spends most of its length defining, which Explicit does not do on its own. The document is written with Explicit because it needs one spelling to be readable, not because the question is settled. Manual is rejected under either name.
• What the attribute derives from. Attribute, or ConditionBaseAttribute with a virtual EvaluateCondition. The second closes the pre-3.10.0 adapter boundary from the framework side, because an old adapter finds the attribute through the base type and fails closed. It costs a public API change on a shipped base class and a second mechanism answering the same question after type load, since the early gate has to stay regardless.

Beyond those, approval covers the public API, the positive-filter activation model, the discriminating segment rule for tree node and graph filters, the three ExplicitTestMode values, and the documented legacy VSTest boundary.

Design for #5346
🤖

[Copilot]

Pull request overview

Defines RFC 024 for opt-in MSTest explicit-test behavior across VSTest and native MTP.

Changes:

• Specifies public APIs, activation rules, lifecycle, diagnostics, retries, and compatibility.
• Defines implementation surfaces and testing plans for both hosts.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

docs/RFCs/024-Explicit-Tests.md:143

• TestDataRow<T> is a row and does not implement ITestDataSource, so making it implement the source-named capability creates a misleading public type relationship. The existing ignore design keeps ITestDataSourceIgnoreCapability on actual sources (DataRowAttribute/DynamicDataAttribute) while exposing IgnoreMessage directly on TestDataRow<T> and internal ITestDataRow. Keep the explicit source capability and row properties separate in the same way.

`DataRowAttribute`, `DynamicDataAttribute`, and `TestDataRow<T>` implement the capability.

docs/RFCs/024-Explicit-Tests.md:435

• This no-user-code guarantee conflicts with the stated precedence that the assembly ITestFilter runs first. The current path loads the test assembly, instantiates the registered filter, and calls Filter before resolving the test method, so module initializers, the filter constructor, and Filter itself can execute user code. Either move the explicit gate ahead of that policy or narrow this guarantee so protected tests do not rely on it.

The explicit check happens in `UnitTestRunner.RunSingleTestAsync`, before the test type is loaded and
therefore before assembly initialization, class initialization, construction, `TestInitialize`, and
the body. An assembly whose only selected tests are unactivated class, method, or unfolded-row
explicit tests runs no user code at all. Folded row declarations are the exception, see below.

[Jakub Jareš (nohwnd)]

Both suppressed comments in the last review were right, fixed in 0b44dbd.

On TestDataRow<T> implementing ITestDataSourceExplicitCapability: the row is not an ITestDataSource, so implementing a source named capability claimed a relationship that does not exist, and it broke the split the ignore metadata already uses. ITestDataSourceIgnoreCapability sits on DataRowAttribute and DynamicDataAttribute, while IgnoreMessage is declared directly on TestDataRow<T> and on the internal ITestDataRow. IsExplicit and ExplicitReason now follow the same shape: the capability stays on sources, the row declares both properties itself as [DataMember] next to IgnoreMessage, and ITestDataRow exposes them so the existing unwrapping and serialization paths keep working. The API sketch shows both halves now instead of only the interface.

On the no user code guarantee: right as well, and it was wrong in two ways rather than one. ApplyTestFilter runs before _typeCache.GetTestMethodInfo, and TypeCache.LoadTestFilterForSource calls LoadAssembly before it can probe for the attribute, so the test assembly is loaded and its module initializers run for every selected test whatever the explicit state is. On top of that, a registered [TestFilterProvider] gets its constructor and Filter called per test.

I narrowed the claim rather than moving the gate. The filter answers whether the test belongs in the run at all, which is not the same question as whether the user asked for it, and a test the policy drops has to report as dropped rather than as an explicit skip, so putting the gate first would reverse that. The RFC now states the guarantee as no type load, no assembly or class initialization, no constructor, no TestInitialize and no body, and names the filter and folded rows as what it does not cover. Added an ordering test that registers a [TestFilterProvider] next to an unactivated explicit test and asserts the filter is still constructed and called, so the boundary is pinned by a test rather than only by prose.

🤖

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (4)

docs/RFCs/024-Explicit-Tests.md:156

• ITestDataSource.GetData returns IEnumerable<object?[]>, so a custom source cannot return TestDataRow<T> values directly; it must wrap each row in a single-element array. Direct IEnumerable<TestDataRow<T>> works here only because DynamicData adapts arbitrary enumerables. Clarify this distinction so implementers do not copy an invalid custom-source contract.

A custom `ITestDataSource` implements the capability to mark every row it produces, or returns
`TestDataRow<T>` to mark single rows:

docs/RFCs/024-Explicit-Tests.md:356

• This tree-filter example does not match MSTest categories. MSTestTestNodeConverter.cs:175-185 encodes category Hardware as metadata key Hardware with an empty value, while TreeNodeFilter.Matching.cs:170-173 interprets the bracket sides as metadata key/value, so [Category=Hardware] matches nothing. Use an actual key/value trait example, or include a compatible category-metadata change and define how key-only categories interact with the rule that [Name=*] is non-discriminating.

| `/*/*/*/*[Category=Hardware]` | yes |

docs/RFCs/024-Explicit-Tests.md:536

• The JSON schema currently sets additionalProperties: false for mstest.execution (docs/testconfig.schema.json:89-93), so the documented explicitTestMode configuration will be reported as invalid unless that schema is updated. Add the schema surface to the implementation plan so IDE validation and completion ship with the parser change.

| Settings | three `ExplicitTestMode` values with existing precedence, plus localized resources |

docs/RFCs/024-Explicit-Tests.md:476

• Source-wide explicit metadata is available on the ITestDataSourceExplicitCapability instance before GetData runs, just as source-wide ignore is checked before enumeration in TestMethodRunner.DataRow.cs:30-39. Deferring this gate until enumeration can execute a side-effecting data source even though it was not activated; the “metadata does not exist until the source runs” rationale applies only to row-level declarations. Gate source explicitness before GetData and reserve the enumeration exception for row metadata.

Folded rows have no discovery identity, so class and method explicitness is checked first, and source
and row declarations are checked as the data is enumerated, before per-row `TestInitialize`, before
test-class construction where construction is per row, and before the body. Each unactivated row

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

[Jakub Jareš (nohwnd)]

Both suppressed comments were right, fixed in 48a3ac1. The first one caught me asserting a guarantee the design could not deliver, which is worse than the hole it was meant to close.

The corruption guarantee was unsupported. Right. With one field carrying the provenance, a reconstructed folded parent saying Explicit=True and a mixed-source deferral is indistinguishable from a legitimate one, whatever it was originally. The shape check passes because it really is a folded parent, the per-source checks find nothing explicit, and every row runs. So the vector I added last round, a class-scope-explicit parent corrupted to claim deferral, would not have skipped, and I wrote a test plan line saying it would.

The defect was that a single field has nothing to be checked against, which is the same mistake as the boolean one round earlier, one size up. Split into ExplicitFromDeclaration and ExplicitFromSources. The gate skips whenever the test is explicit, unless the declaration flag is false and the source flag says Some. A class or method declaration is asserted separately and keeps gating on its own, so it cannot be talked out of gating by a value that only describes the sources, and that combination is now visibly inconsistent. Either field missing or unrecognized gates.

The parent reason rule was incomplete. Also right, and the gap was wider than the "different reasons" case I had written: one source with a reason beside a sibling with none, and a source-specific reason beside a broader method or class one, both ended up attributing one source's explanation to a result standing for all of them. Taking your definition. A parent gated by a class or method declaration takes that declaration's reason, since one declaration covers the whole method and nothing can disagree with it. A parent gated by its sources takes the reason they agree on and reports none on any disagreement, reason versus no reason included. Per-source reasons are still reported where they belong, when the sources are resolved individually.

Twelve rounds. I have said twice that I was stopping and pushed anyway, both times because the finding was in the same place: a persisted value that could let Run All execute an opt-in test. That is the one thing this design exists to prevent, so it has not felt like somewhere to stop on principle. It is worth saying plainly that three consecutive rounds found a defect in successive versions of the same fallback, each one my own, which is a fair signal that this corner deserves a human's eye more than another iteration of mine.

Everything is answered, lint is green, and every thread is resolved. The decisions taken across these rounds are in my earlier comments.

🤖

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

docs/RFCs/024-Explicit-Tests.md:589

• This aggregate rule conflicts with the API-level precedence at lines 200–205 and the resolved answer at line 928. When a folded parent is explicit at method scope and all sources are also explicit with a shared reason, those sections say the more-specific source reason wins, while this paragraph says the method reason wins. Define the folded aggregate exception explicitly (including mixed method/source reasons) and add a corresponding test vector so reporting is deterministic.

A gated parent reports one result standing for everything under it, so its reason has to be true of
everything under it. A parent gated by a class or method declaration takes that declaration's reason,
the more specific of the two, because one declaration covers the whole method and there is nothing to
disagree with it. A parent gated by its sources takes the reason its sources agree on, and reports
none on any disagreement, including one source carrying a reason where a sibling carries none. Picking
the most specific available reason there would attribute one source's explanation to a result standing
for all of them. Each source's own reason is still reported when the sources are resolved
individually, which is where a per-source explanation belongs.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

docs/RFCs/024-Explicit-Tests.md:256

• These commands pass MTP options to dotnet run itself, which does not recognize them. Add the -- application-argument separator so the test executable receives --filter-uid and --treenode-filter.

dotnet run --filter-uid <uid>
dotnet run --treenode-filter "/*/*/DeviceTests/*"

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

docs/RFCs/024-Explicit-Tests.md:793

• This discovery plan does not populate source metadata for the folded paths it is meant to protect. AssemblyEnumerator.TryUnfoldITestDataSources currently returns for either folding mode before it even reads the source attributes (AssemblyEnumerator.cs:216-231), and can return after the first source fails to unfold without inspecting later sources. If source declarations are merged only “while unfolding,” an all-explicit folded parent is neither gated nor selectable via Explicit=True. Pre-scan all source declarations before any folding/expansion early return; enumerate rows only when unfolding proceeds.

| Discovery | `TypeEnumerator` reads class and method declarations, `AssemblyEnumerator` merges source and row declarations while unfolding |

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

docs/RFCs/024-Explicit-Tests.md:767

• The reservation does not cover [TestCategory], even though native MTP converts each category into TestMetadataProperty(category, string.Empty) (MSTestTestNodeConverter.cs:175-185). Consequently, [TestCategory("ExplicitReason")] makes /**[ExplicitReason=*] filterable on MTP while VSTest does not resolve that category by the property name, contradicting the cross-host and reason-not-filterable guarantees. [TestCategory("Explicit")] also collides with the universal built-in key, so its normal [Explicit=*] tree filter now selects every node instead of that category. Define collision handling for category names too and include both cases in compatibility notes and cross-host vectors.

`Explicit` and `ExplicitReason` are reserved property names on both hosts, compared ordinal
case-insensitively, because both hosts match case-insensitively: `ValueExpression` builds its regex
with `RegexOptions.IgnoreCase`, and `TestMethodFilter`'s supported-property dictionary and its trait
fallback both use `OrdinalIgnoreCase`. Reserving only the exact spellings would leave
`[TestProperty("explicit", "True")]` colliding. A `[TestProperty]` whose name matches either reserved
name in any casing is not written to the metadata surface or to the traits, and discovery reports a
warning naming the test, so the built-in value is the only one either host can match.

[Amaury Levé (Evangelink)]

Could we rename this concept to OnDemand throughout ([OnDemand], IsOnDemand, OnDemandReason, OnDemandTestMode)? Explicit is framework jargon, whereas OnDemand communicates the behavior directly: the test remains discoverable but runs only when positively selected or when the run mode requests it. It also avoids Manual, which would imply the test cannot be automated even though CI filters can activate it.

[Jakub Jareš (nohwnd)]

Not renamed, and recorded as an open question rather than answered quietly. There is a new Naming section putting both names side by side, and it is now one of the two rows in ## Open questions and named in the approval sentence at the end.

The argument I did not want to lose is the prior-art table right above this: NUnit, xUnit v3 and TUnit all call it Explicit, so someone arriving from any of them searches for [Explicit]. Against that, your point stands on its own merits: OnDemand describes the behaviour this document spends most of its length defining, and Explicit describes nothing unless you already know the term. I do not think one of those obviously beats the other, and it is not a call to make by whoever edits the file last. It changes every surface at once and it cannot be revised after the API ships, so it belongs to approval.

Manual is rejected in the text under either name, for the reason you gave.

If you want to just decide it now, say the word and I will do the rename in one commit.
🤖

[Amaury Levé (Evangelink)]

Could OnDemandAttribute derive from ConditionBaseAttribute rather than directly from Attribute? This is another declarative execution gate that produces a skipped result, and the existing hierarchy already includes the unconditional IgnoreAttribute.

To preserve compatibility, we could add a non-nullable virtual EvaluateCondition(ConditionEvaluationContext context) whose base implementation delegates to IsConditionMet. OnDemandAttribute.IsConditionMet would return false so older adapters fail closed, while new adapters override EvaluateCondition to use context.IsTestActivated. New adapters would always call the method, and existing conditions would continue through the property.

This should remain separate from TestMethodAttribute so it composes with STA, UI, and user-defined test-method attributes and can also apply at class, data-source, and data-row scopes. The RFC would still keep its discovery metadata and early gate so unactivated tests are stopped before type or data-source loading.

[Jakub Jareš (nohwnd)]

Also left open rather than decided, in a new Base type section and the second row of ## Open questions.

The paragraph you were replying to argued that a condition cannot know what the user asked for. Your EvaluateCondition(ConditionEvaluationContext) shape answers that, so I have rewritten it rather than leaving a rebuttal that no longer rebuts anything.

The part I had missed, and which is the strongest thing in your comment, is what IsConditionMet => false buys on an old adapter. AttributeExtensions.IsIgnored finds conditions by pattern-matching the base type, so an adapter that predates [Explicit] still finds the attribute through ConditionBaseAttribute and skips. That closes the pre-3.10.0 boundary the Compatibility section had written off as unfixable, from the framework side, which is a real argument and not a stylistic one. I have said so in both places.

The cost I want on the record before we choose: conditions resolve inside IsTestMethodRunnable, after TypeCache.GetTestMethodInfo, so the early gate has to stay regardless and the condition path becomes a second mechanism answering the same question later. Two mechanisms that must agree is the thing that will bite us, not the extra virtual. Worth it for the old-adapter behaviour, I think, but it is your call and it is a public API change on a shipped base class.

One detail for whoever writes it up: ConditionBaseAttribute is AttributeTargets.Class | AttributeTargets.Method, so it reaches two of the four scopes. Data sources and rows are not attributes, so they keep the capability interface and the row properties either way.
🤖

[Copilot]

🟡 Changes recommended

Critical cleanup and runtime compatibility concerns, plus explicit-state aggregation gaps, must be resolved before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

docs/RFCs/024-Explicit-Tests.md:755

• The table lists four fields, not three. Keeping the count accurate matters here because the following paragraphs distinguish the gate fields from the reporting reason.

an execution request fails to activate it. Each `UnitTestElement` carries three fields, because

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