Design explicit tests for MSTest - #10606
Conversation
Define the API, selection model, data-row behavior, host integration, diagnostics, compatibility, and test plan for microsoft#5346. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 22c4cda6-f4c9-4492-89a0-9e2d80516d43 🤖
There was a problem hiding this comment.
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.
The direct-selection table classified every node matched by --treenode-filter or a server graph filter as activated. That grammar has negation, != property predicates, and wildcards, so an exclusion-only or match-all filter would have activated explicit tests and /** would have been Run All that runs them. Give the tree grammar its own (matches, activates) algebra, require a discriminating non-root segment to activate, and state the fail-closed rule that activation is never assumed when it cannot be determined. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Cut it from 1081 to 604 lines. Every rule is still there, what went away is the repository history recital, the per-file implementation table, and the numbered acceptance test lists. Motivation now opens with three tests you would actually mark explicit, and there is a section showing how you run them, from Test Explorer, from dotnet test, and from an opt-in CI job. Added a prior art table for NUnit, xUnit v3 and TUnit, and why we follow NUnit rather than TUnit. 🤖
There was a problem hiding this comment.
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 implementITestDataSource, so making it implement the source-named capability creates a misleading public type relationship. The existing ignore design keepsITestDataSourceIgnoreCapabilityon actual sources (DataRowAttribute/DynamicDataAttribute) while exposingIgnoreMessagedirectly onTestDataRow<T>and internalITestDataRow. 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
ITestFilterruns first. The current path loads the test assembly, instantiates the registered filter, and callsFilterbefore resolving the test method, so module initializers, the filter constructor, andFilteritself 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.
TestDataRow<T> is a row, not an ITestDataSource, so making it implement ITestDataSourceExplicitCapability claimed a type relationship that does not exist. It now declares IsExplicit and ExplicitReason directly, next to IgnoreMessage, and the internal ITestDataRow exposes them, which is exactly how the ignore metadata is already split between sources and rows. Also narrow the gate guarantee. The explicit check runs after the assembly ITestFilter, and filter discovery loads the test assembly and then constructs and calls a registered [TestFilterProvider], so "runs no user code at all" was wrong. The guarantee is now no type load, no fixtures, no constructor, no TestInitialize and no body, the filter keeps its place ahead of the gate because it decides whether the test is in the run at all, and an ordering test pins the boundary. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Both suppressed comments in the last review were right, fixed in 0b44dbd. On On the no user code guarantee: right as well, and it was wrong in two ways rather than one. 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 🤖 |
There was a problem hiding this comment.
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.GetDatareturnsIEnumerable<object?[]>, so a custom source cannot returnTestDataRow<T>values directly; it must wrap each row in a single-element array. DirectIEnumerable<TestDataRow<T>>works here only becauseDynamicDataadapts 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-185encodes categoryHardwareas metadata keyHardwarewith an empty value, whileTreeNodeFilter.Matching.cs:170-173interprets 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: falseformstest.execution(docs/testconfig.schema.json:89-93), so the documentedexplicitTestModeconfiguration 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
ITestDataSourceExplicitCapabilityinstance beforeGetDataruns, just as source-wide ignore is checked before enumeration inTestMethodRunner.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 beforeGetDataand 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
The fail-closed section said an unsupported filter constrains the run and activates nothing, while the next paragraph said an unparseable one fails. A filter whose semantics are unknown cannot constrain anything either, so split the two questions: an unevaluable constraint keeps its existing failure, and an evaluable one whose activation cannot be classified activates nothing. Also: - Say how *[Explicit=True] reaches TreeNodeFilter. It matches [Key=Value] only against TestMetadataProperty, so Explicit is one, and pre-node filtering in MtpTestElementFilter reads it from the same source. - Scope ignore precedence to metadata that is actually reached, and document the folded parent that is explicit and unactivated, whose data source never runs. - Stop calling ExplicitTestMode=Run and --filter "Explicit=True" equivalent. Run widens activation, the filter narrows selection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Moving the Explicit=True paragraphs put two paragraphs between "That gap" and the gap it referred to. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| The risk worth calling out in release notes is version skew. A new adapter with an old framework sees | ||
| no declarations and behaves as today, but a new framework with an old adapter can load `[Explicit]` | ||
| without recognizing it, and may run the test during Run All. The usual MSTest package alignment check | ||
| is the answer. Making the attribute derive from `[Ignore]` would paper over this and would also make | ||
| the test impossible to select on any adapter, so it is not done. |
| 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 |
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/--filterexpression, 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, theExplicitTestModeoverride, 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