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, and it needs approval on the public API, the activation model, the configuration override, 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 too, same commit, 35ad4c4.

The localized default in the reason chain. It contradicted two things at once: the statement a few lines above that blank values mean no reason, and the reporting contract, which always emits the base skip message and appends Reason: only when a declaration supplied one. A default at the end of the chain would have appended the message to itself. Removed, with a line saying why: a test whose declarations all lack a reason has no reason, and the skip message already says why it did not run.

The skip message under ExplicitTestMode=Skip. Right, and it is the one mode where the message is simply false. Skip never activates, not even for a directly selected test, so a test that was selected is told it was not, which sends its author looking for a selection problem that does not exist. That mode gets its own message now, The test is explicit and ExplicitTestMode is Skip., with the reason appended to either one.

That is eleven rounds. I am not pushing again unless something is actually wrong, and I want to be plain about why this one was worth another push after I said I was stopping: the inline comment was a third instance of the same safety hole, where a corrupted or inconsistent persisted value could run an opt-in test under Run All. That is the failure this whole design exists to prevent, so it did not seem like a place to stop on a technicality.

What is left is a person. The design decisions taken across these rounds are in my earlier comments, and the three that move the model most are the folded parent carrying separate selection state and an ExplicitScope that travels on the VSTest wire, the reservation of Explicit and ExplicitReason as property names, which is the one compatibility break and now affects VSTest filters as well as MTP ones, and a retry attempt inheriting the activation of the attempt it is retrying.

🤖

[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:694

• ExplicitScope is still the only persisted declaration-provenance field, so a reconstructed folded case containing Explicit=True and ExplicitScope=SomeSources cannot reveal that it was originally explicit at class scope with ordinary sources. The folded-shape check described here accepts SomeSources, defers to per-source checks, and those checks run every ordinary row—the exact corruption vector that lines 820–823 claim will skip. Persist an independently checkable class/method provenance marker (or otherwise make deferral verifiable), or remove the unsupported corruption guarantee.

  `Explicit` is. `SomeSources` is honored only on a folded data-driven parent, the one shape with a
  per-source check downstream, and the case says which shape it is in metadata it already carries, so
  no type load is needed to ask. On any other shape it is inconsistent rather than a deferral and the
  gate fires, as it does for a missing or unrecognized value on any shape. Over-skipping costs a folded
  parent with disagreeing sources one explicit skip instead of its ordinary source's rows, which is

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

• The “different reasons” exception does not cover one source with a reason and a sibling with no reason, or a source-specific reason beside a broader method/class reason. Choosing the most specific available reason in those cases attributes one source's explanation to the aggregate skip representing every source. Define the parent reason as the common effective reason across all represented sources; any disagreement, including reason versus no reason, should produce no appended reason.

The reason on a gated parent follows the ordinary precedence, the most specific declaration that has
one. When the declaration is source-only and several sources carry different reasons, the parent
reports no reason rather than picking one, because a result that stands for every source cannot
attribute itself to one of them. Each source's own reason is still reported when the sources are
resolved individually.

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

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

[Amaury Levé (Evangelink)]

The normative gate still seems unsafe/undefined for unfolded rows. ExplicitFromSources is described as what all sources on the method say; with three [DataRow] sources and one explicit row, that naturally becomes Some. Unfolding clones the parent element (AssemblyEnumerator.cs:330), so the explicit row can inherit ExplicitFromDeclaration=false / ExplicitFromSources=Some, take this deferral, and then has no folded per-source check to stop it. Run All would execute the motivating 20-minute row, contradicting lines 582–585.

Line 798 does not settle this: it explicitly scans source declarations before deciding whether to unfold, while row merging supplies per-element state. Please either make the shape rule normative here (deferral is valid only for a folded parent) or define ExplicitFromSources as element-scoped for unfolded rows (the producing source is All), and add a default-unfolding cross-host test with ordinary and on-demand sibling rows.

[Amaury Levé (Evangelink)]

TestMetadataProperty is a downstream trait surface regardless of whether the value originated from UnitTestElement.Traits. Server serialization emits it under "traits", --list-tests JSON emits it in the traits array, TRX writes it under <Properties>, and OpenTelemetry emits test.metadata.<key>. Writing Explicit (later OnDemand) to every node therefore changes all of those surfaces for every MSTest-on-MTP user, including projects with no declaration.

That contradicts the compatibility statement at 920–922 and makes the planned assertion at 850 (“appears in neither the trait nor category surfaces”) impossible. Please either document these additions as compatibility changes and update the test plan, or use a dedicated non-TestMetadataProperty node property with explicit matcher support.

[Amaury Levé (Evangelink)]

VSTest does not synthesize False for an unset registered property. TestMethodFilter.PropertyValueProvider returns a value only when currentTest.Properties.Contains(testProperty); otherwise it returns null (TestMethodFilter.cs:101–109). Equality against that missing value does not match and inequality does, which is the same missing-property asymmetry described for MTP here.

So --filter "Explicit=False" matches no ordinary VSTest cases unless the adapter also writes Explicit=False onto every ordinary TestCase, while the proposed MTP node does match them. Please either specify the universal write on both hosts (and include its compatibility/size cost), or document matching missing values consistently on both. The four operator/value vectors at 834–838 should pin this, including VSTest's reserved None value.

[Amaury Levé (Evangelink)]

The retry pipe is not sufficient to distinguish an inherited retry selection: RetryArgumentsBuilder.BuildAttemptArgumentsAsync adds --internal-retry-pipename on every attempt, including the first; only the failed --filter-uid replacement is conditional. Therefore attempt 1 of a user-selected UID run and attempt 2 of Run All can both carry the pipe plus --filter-uid.

The design needs a distinct signal carrying the original activation, plus fail-closed behavior when that signal is absent. Absence matters under package version skew: an older Retry extension with newer MSTest supplies only the UID, which this RFC otherwise treats as positive selection and can run a folded on-demand row on attempt 2. Please name the signal/protocol and add a missing-signal or version-skew retry vector.

[Amaury Levé (Evangelink)]

The collision handling covers [TestProperty], but MTP writes each [TestCategory("X")] into the same metadata key space as TestMetadataProperty("X", "") (MSTestTestNodeConverter.cs:182–185). [TestCategory("Explicit")] (later OnDemand) would therefore add a second value under the reserved built-in key beside True/False, and the matcher succeeds on any matching value.

Please extend the primary-key collision rule and warning to category names as well, or move the built-in value to a dedicated property type/key space. Add the category form to the collision vectors; otherwise the reservation can still be bypassed without using [TestProperty].

[Amaury Levé (Evangelink)]

The reason was removed from node metadata at 753–756 and is deliberately unfilterable on VSTest, so no built-in filterable surface now occupies ExplicitReason (later OnDemandReason). Reserving that name no longer prevents a collision; it only breaks existing [TestProperty("ExplicitReason", ...)] users on both hosts.

I suggest reserving only the primary Explicit/OnDemand key. Keep the reason name reserved only if the design intends to expose it on a matchable metadata surface later, in which case that intent should be stated here.