Design explicit tests for MSTest - #10639
Closed
Jakub Jareš (nohwnd) wants to merge 7 commits into
Closed
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 🤖
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. 🤖
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>
Contributor
There was a problem hiding this comment.
Pull request overview
Defines MSTest explicit-test semantics consistently across VSTest and Microsoft.Testing.Platform for issue #5346.
Changes:
- Proposes
[Explicit]and data-row APIs. - Defines activation, filtering, precedence, configuration, compatibility, and testing behavior.
- Documents implementation surfaces for both hosts.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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>
Member
Author
|
Closing: this was opened against The two commits are now on #10606 itself (fast-forwarded to Cause is fixed: delivery now pushes onto the branch of the pull request the work is about, and refuses rather than opening a new one. 🤖 |
Contributor
There was a problem hiding this comment.
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:552
TestMetadataPropertycannot provide the documented non-trait, non-filterable transport. MTP serializes every such property undertraits(DiscoveredTestsJsonSerializer.cs:21-22, 80, 129-134), andTreeNodeFilter.IsMatchingPropertymatches everyTestMetadataPropertykey/value (TreeNodeFilter.Matching.cs:170-173). ThusExplicit/ExplicitReasonsurface as traits and[ExplicitReason=...]becomes filterable, contradicting lines 176, 546, and 551-552. Use a dedicated non-filterable reason property and either a dedicated matchable explicit flag or explicitly align the trait contract across both hosts.
`ExplicitReason` only when a non-empty reason exists, on both discovered and result nodes.
`Explicit` is a `TestMetadataProperty`, which is the property type `[Key=Value]` in a tree node
filter matches, so `*[Explicit=True]` works with no platform matcher change. It is still not a
trait: it is produced from `IsExplicit` rather than from `[TestCategory]` or `[TestProperty]`, it
is not in `UnitTestElement.Traits`, and it does not show up as a user authored category. UIDs do
Comment on lines
+502
to
+505
| 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 | ||
| produces its own skipped `UnitTestResult`, which keeps folded result cardinality exactly as it is |
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.
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.Data-source explicitness follows the split the ignore metadata already uses:
ITestDataSourceExplicitCapabilitygoes on sources, andTestDataRow<T>declaresIsExplicitandExplicitReasondirectly, next toIgnoreMessage. The gate sits after the assemblyITestFilterand before type loading, so an unactivated test loads no type and runs no fixture, constructor or body; the filter and folded rows are documented as what it does not cover.Also covers 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
🤖