Add acceptance test combining --retry-failed-tests with --treenode-filter

[Evangelink]

Fixes #5673.

Investigation

The RetryOrchestrator (src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs:199-203) already strips both --treenode-filter and --filter-uid from the child-process arguments when launching a retry attempt, then injects a fresh --filter-uid <failed-uids>. Filters are therefore replaced, not stacked — exactly the concern raised in #5673. This was added as part of the earlier Retry simplification work.

What this PR adds

A new acceptance test RetryFailedTests_WithPreexistingTreeNodeFilter_ReplacesFilterOnRetry that guards the combined --retry-failed-tests + --treenode-filter flow, mirroring the existing WithPreexistingFilterUid_ReplacesFilterOnRetry test.

To support it, the dummy framework in the test asset now:

• Registers AddTreeNodeFilterService (via a small TreeNodeFilterExtension).
• Honors TreeNodeFilter in addition to TestNodeUidListFilter, matching nodes against /<DisplayName> paths.

The test verifies that:

1. First attempt honors --treenode-filter "/(TestMethod1|TestMethod2)" (TestMethod1 fails, TestMethod2 passes, TestMethod3 never runs).
2. Retry attempt drops --treenode-filter and uses only --filter-uid 1 (just the failed test).
3. The final top-level TRX contains TestMethod1 only.

Verification

• .\build.cmd -pack -c Debug succeeded.
• All 28 RetryFailedTests* acceptance tests pass on net8.0, net10.0 and net462, including the two new WithPreexistingTreeNodeFilter variants.

[Copilot]

Pull request overview

This PR adds an acceptance test to guard against #5673 by ensuring --retry-failed-tests does not stack with an existing --treenode-filter (the retry run must replace the original filter with --filter-uid <failed>).

Changes:

• Added a new acceptance test RetryFailedTests_WithPreexistingTreeNodeFilter_ReplacesFilterOnRetry.
• Updated the generated dummy test asset to register the --treenode-filter command-line option and to honor TreeNodeFilter in its execution logic.

Show a summary per file

File	Description
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/RetryFailedTestsTests.cs	Adds the new retry+treenode-filter acceptance test and extends the embedded test asset to support TreeNodeFilter.

Copilot's findings

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

[Evangelink]

Code Review

Dimension 13 — Test Completeness & Coverage (MAJOR) ✅ with one minor gap

Point 1 – First attempt honours --treenode-filter: Verified. The dummy framework's IsIncluded now respects TreeNodeFilter, so TestMethod3 is excluded because /(TestMethod1|TestMethod2) does not match /TestMethod3. The assertion Assert.DoesNotContain("TestMethod3", trxContent) guards this.

Point 2 – Retry uses only --filter-uid, not stacked filters: The RetryOrchestrator (lines 202–203) calls RemoveOption for both TreenodeFilter and FilterUidOptionKey before injecting a fresh --filter-uid for the failed tests. The test validates the observable outcome: the final TRX contains only TestMethod1, which could only happen if the retry wasn't also constrained by the treenode-filter (otherwise an empty-filter --filter-uid 1 that was also constrained by /(TestMethod1|TestMethod2) would still pass, but a mis-stacking that injected --treenode-filter /(TestMethod1|TestMethod2) --filter-uid 1 would also pass for this particular filter). The test as written catches the most important regression (filters not being stripped at all), but not a subtle double-filter stacking where both filters happen to agree.

Missing negative test (minor gap): The sibling WithPreexistingFilterUid test has the same shape and also doesn't explicitly assert "TestMethod2 was run in attempt 1". Both tests could add Assert.Contains("TestMethod2", ...) on the Retries/ TRX to confirm attempt-1 ran both filtered-in tests. Without it, a bug where the treenode-filter is ignored entirely (so TestMethod3 also runs) would not be caught by the DoesNotContain check on the top-level TRX alone. That said, this is a pre-existing gap mirrored from the sibling test, not a regression introduced here.

---

Dimension 15 — Code Structure & Simplification (MODERATE) ✅ / ⚠️

TreeNodeFilterExtension members: All string members returning nameof(TreeNodeFilterExtension) is fine for a test asset — Uid, DisplayName, and Description are metadata only used for display/lookup and any non-empty stable value is acceptable. Version returning the class name instead of "1.0.0" would be the only oddity, but "1.0.0" is used here which is conventional. No issue.

IsIncluded 4-parameter signature: The expansion from 2 to 4 parameters is straightforward and readable given it is a small private helper used only three times in the same class. A record or tuple wrapper would add ceremony without benefit at this scale. Acceptable.

new PropertyBag() in MatchesFilter: This is correct. TreeNodeFilter.ContainsPropertyFilters documents explicitly:

"When false, the PropertyBag argument ... is never inspected, and callers may safely pass an empty bag."

The filter expression /(TestMethod1|TestMethod2) contains no property clauses ([Key=Value]), so ContainsPropertyFilters will be false and the empty bag is never read. ✅

---

Dimension 14 — Data-Driven Test Coverage (MODERATE) ✅

TargetFrameworks.NetForDynamicData is the right choice — it matches the sibling test and covers the supported .NET (Core/5+) target frameworks. The PR description confirms the test was verified on net8.0, net10.0, and net462. Note that net462 is covered by TargetFrameworks.All (used in the broader matrix tests) but not by NetForDynamicData; since the retry+treenode-filter interaction is platform-level behaviour not framework-specific, limiting to NetForDynamicData is consistent with the sibling and acceptable.

---

Overall: The PR is well-structured and addresses the issue. The one actionable suggestion is to add an assertion on the attempt-1 TRX (under Retries/) to confirm both TestMethod1 and TestMethod2 ran in the first pass — this would make the "first attempt honours the treenode-filter" claim explicit rather than implied.

Generated by Expert Code Review (on open) for issue #8405 · ● 13.9M · ◷

[Evangelink]

Review Summary

10/10 applicable dimensions clean — one optional NIT.

Dimension	Result
Algorithmic Correctness	✅ LGTM
Test Isolation	✅ LGTM
Assertion Quality	✅ LGTM
Flakiness Patterns	✅ LGTM
Test Completeness	🔵 NIT (see inline)
Code Structure	✅ LGTM
Data-Driven Coverage	✅ LGTM
Naming & Conventions	✅ LGTM
Documentation Accuracy	✅ LGTM
Scope & PR Discipline	✅ LGTM

The implementation is correct. The IsIncluded filters are mutually exclusive at runtime (the request filter can only be one concrete type), so the AND logic is safe. The sentinel-file mechanism makes the retry deterministic. The one inline note is an optional enhancement to also validate attempt-1 behavior.

Generated by Expert Code Review (on open) for issue #8405 · ● 13.9M

[Copilot]

Copilot's findings

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

[Evangelink]

@copilot address review comments

[Copilot]

@copilot address review comments

Addressed review feedback in 5cd8d5d. I added first-attempt TRX assertions for the treenode-filter retry test, ran the pack build successfully, attempted the targeted acceptance test (blocked by transient package download errors for Microsoft.NETCore.App.Ref.10.0.7 / related runtime packs), and completed final validation with no review issues.