Remove misleading TreeNodeFilter test added by PR #7415

[Evangelink]

Context

PR #7415 (commit 32a26954f) addressed --treenode-filter issues reported in #7300:

• Case 4 ((/*/*/*/MyTest1)|(/*/*/*/MyTest2) crashing with a confusing message) — correctly fixed with FullPathOrInsideParenthesizedExpressions_IsNotSupported_ThrowsActionableMessage.
• Cases 1-3 (/*/*/*/(MyTest1) and friends silently matching zero tests) — hypothesised to be caused by the framework appending a parameter-list suffix (e.g. MyTest1()) to test node IDs, and "documented as a limitation" via LiteralSegment_RequiresWildcardToMatchNodesWithAdditionalSuffix.

Why this is wrong

The premise behind the new "limitation" test is incorrect, and the test should not stay as-is referencing #7300:

1. TUnit (the framework that surfaced the issue) does not append any suffix. TUnit's TestFilterService.BuildPath emits clean /{assembly}/{namespace}/{className}/{methodName} paths — no ().
2. MTP's TreeNodeFilter actually matches all the patterns from the issue. A direct reflection probe against Microsoft.Testing.Platform 2.0.2 (the version TUnit currently bundles) returned True for every failing case in the issue, including /*/*/*/(MyTest1) against /YourTestProjectNameHere/YourTestProjectNameHere/MyTests/MyTest1. The kept regression test OrExpression_WorksForSinglePathSegmentInsideParentheses already proves this.
3. The real bug lives in TUnit's MetadataFilterMatcher.ExtractFilterHints. It does a naive filterString.Split('/') and treats parts[4] as a literal method-name hint unless IsWildcard returns true. IsWildcard only checks for */?, so "(MyTest1)" is treated as a literal method name; AotTestDataCollector.CouldDescriptorMatch then rejects every descriptor before MTP's filter ever sees them. Filed upstream as MetadataFilterMatcher.ExtractFilterHints misinterprets parenthesised path segments as literal method names thomhurst/TUnit#6026.

What this PR does

• Removes LiteralSegment_RequiresWildcardToMatchNodesWithAdditionalSuffix. Its only behavioural assertion (anchored-regex matching: MyTest1 does not match MyTest1()) is already covered by MatchWildcard_MatchesSubstrings and is unrelated to --treenode-filter pattern matching is at least non obvious, potentially errored #7300; the misleading comment and #7300 link were the main reason it existed.
• Keeps the genuinely useful regression OrExpression_WorksForSinglePathSegmentInsideParentheses and the actionable-error test FullPathOrInsideParenthesizedExpressions_IsNotSupported_ThrowsActionableMessage.

Validation

• Microsoft.Testing.Platform.UnitTests build: ✅
• TreeNodeFilterTests (filter FullyQualifiedName~TreeNodeFilterTests): 52 passed, 0 failed (was 53 before removal of the misleading test).

Related

• Issue: --treenode-filter pattern matching is at least non obvious, potentially errored #7300
• Original PR: Fix TreeNodeFilter OR-pattern diagnostics #7415
• Upstream TUnit bug: MetadataFilterMatcher.ExtractFilterHints misinterprets parenthesised path segments as literal method names thomhurst/TUnit#6026

[Copilot]

Pull request overview

This PR updates the Microsoft.Testing.Platform unit tests by removing a TreeNodeFilter regression test that was documenting an incorrect “limitation” tied to issue #7300, while retaining the genuinely useful OR-expression and actionable-error coverage.

Changes:

• Removes LiteralSegment_RequiresWildcardToMatchNodesWithAdditionalSuffix from TreeNodeFilterTests to avoid preserving misleading behavior documentation tied to #7300.
• Keeps existing regression coverage for single-segment OR inside parentheses and for the improved diagnostic when full-path OR is used inside parentheses.

Show a summary per file

File	Description
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Requests/TreeNodeFilterTests.cs	Removes a misleading TreeNodeFilter test while keeping the valid regression/diagnostic tests.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 0

[Evangelink]

Expert Review — PR #8663

#	Dimension	Verdict	Notes
1	Correctness	✅	Removing a test whose assertions were based on a false premise (TUnit appending () suffixes). The removed assertions are vacuously "correct" but mislead readers about MTP's actual behavior.
2	Public API	✅	No public API changes.
3	Backward Compatibility	✅	Test-only change; no production code touched.
4	Thread Safety	✅	N/A.
5	Test Quality	✅	The removed test documented a non-existent limitation. The behavior it was supposedly testing (anchored regex semantics) is already covered by MatchWildcard_MatchesSubstrings. Removing it improves the overall test suite honesty. The kept regression tests (OrExpression_WorksForSinglePathSegmentInsideParentheses, FullPathOrInsideParenthesizedExpressions_IsNotSupported_ThrowsActionableMessage) are correct and valuable.
6	Analyzer Correctness	✅	N/A.
7	Localization	✅	N/A.
8	Null Safety	✅	N/A.
9	TODO Hygiene	✅	No TODOs introduced.
10	Performance	✅	N/A.
11	Build / Packaging	✅	N/A.
12	Documentation	✅	The removed comment was actively misleading ("documents a current limitation" that doesn't exist). Removing it is strictly better for maintainability.
13	Style & Conventions	✅	N/A — only lines removed.
14	Security	✅	N/A.
15	Error Handling	✅	N/A.
16	CLI Options	✅	N/A.
17	MSBuild SDK	✅	N/A.
18	Resource Pipeline	✅	N/A.
19	Parallelism	✅	N/A.
20	Agentic Workflows	✅	N/A.
21	PR Description Quality	✅	Exemplary: clear root-cause analysis, reference to the upstream TUnit bug, reflection probe evidence, and precise description of what is kept vs. removed.

Overall verdict: COMMENT — PR is clean.

The change is well-motivated and strictly improves the test suite. The PR description provides thorough evidence (reflection probe against MTP 2.0.2, upstream TUnit issue thomhurst/TUnit#6026) that the removed test was based on a false premise. No issues found across all 21 review dimensions.

Generated by Expert Code Review (on open) for issue #8663 · sonnet46 1.4M