[Amaury Levé (Evangelink)]

Pull request

Summary

Restore MSTest adapter compilation after overlapping predicate-extraction PRs left two definitions of IsSingleTupleArgumentWithExplicitCancellationToken on main. Remove only the duplicate definition, retaining one unchanged implementation and all existing callers, guards, and evaluation order.

Fixes #12017.

Design and decision record

The original readability work has now landed through #12021, #12043, and #12045. Resolving the latest conflict revealed that main contains the same private tuple predicate twice, causing CS0111. Keep the first upstream implementation canonical and delete the redundant copy; both expressions are identical after whitespace normalization.

The remaining PR diff is one file and seven deleted lines. No separate design record is needed for removing a duplicate private member. No runtime behavior, API, dependency, test, or CI-policy change is intended.

Demonstration

A compiler transcript is more useful than screenshots for this change. Building the exact current-main adapter source on net8.0 produced this diagnostic and exited 1:

error CS0111: Type 'TestDataSourceHelpers' already defines a member called 'IsSingleTupleArgumentWithExplicitCancellationToken' with the same parameter types

After deleting the duplicate, the adapter builds across all non-UAP targets with zero warnings and zero errors.

Validation and regression proof

Base: 1a6a5f7b429320f4983e51416b23f76a1c8bfb6f. Validated and pushed head: 431b3e999aab9b8e6b05c78802e1f768b4041ff9. Configuration: Debug, repository-pinned SDK 12.0.100-alpha.1.26509.101.

Verified that the before-state file is byte-identical to main, reproduces CS0111, and contains two identical predicate expressions. The after-state retains exactly one of those expressions unchanged. The source UTF-8 BOM and git diff --check also pass.

Exact build commands and outcomes

# Before: exact main source, exit 1, CS0111 duplicate-member error.
& .\eng\build.ps1 -build -msbuildEngine dotnet -projects 'src\Adapter\MSTestAdapter.PlatformServices\MSTestAdapter.PlatformServices.csproj' '/p:TargetFramework=net8.0' '/p:SkipRuntimesInstall=true' '/p:SkipUapTargetFramework=true' -verbosity quiet

# After: all non-UAP adapter targets, exit 0, zero warnings/errors.
& .\eng\build.ps1 -build -msbuildEngine dotnet -projects 'src\Adapter\MSTestAdapter.PlatformServices\MSTestAdapter.PlatformServices.csproj' '/p:SkipRuntimesInstall=true' '/p:SkipUapTargetFramework=true' -verbosity quiet

# After: both focused test-host builds, each exit 0, zero warnings/errors.
foreach ($framework in @('net8.0','net462')) {
    & .\eng\build.ps1 -build -msbuildEngine dotnet -projects 'test\UnitTests\MSTestAdapter.PlatformServices.UnitTests\MSTestAdapter.PlatformServices.UnitTests.csproj' "/p:TargetFramework=$framework" '/p:EnableCodeCoverage=false' '/p:SkipRuntimesInstall=true' '/p:SkipUapTargetFramework=true' -verbosity quiet
    if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
}

Classic UAP, package/acceptance tests, and the full suite were not run locally. Newer .NET/Windows targets were compiled but not executed. CI on the new head must complete separately; no CI success is claimed here.

Test-value evidence

Existing TestDataSourceHelpersTests, TestMethodInfoTests, and MethodInfoExtensionsTests cover tuple-versus-array classification, explicit and injected tokens, argument binding, and invocation. On the resolved source, 196 net8.0 tests and 194 net462 tests passed, with zero failures and zero skips.

The duplicate-member baseline cannot execute tests because it does not compile. Its compiler failure is compilation-sensitivity evidence, not a behavioral red-phase test result. No observable predicate behavior changed: the retained expression matches both deleted/retained upstream definitions exactly after whitespace normalization. No new test cases were needed to enforce the compiler's duplicate-member rule.

Reports remain local under the result paths below and have not been uploaded.

# 196 passed, 0 failed, 0 skipped; exit 0.
& .\.dotnet\dotnet.exe artifacts\bin\MSTestAdapter.PlatformServices.UnitTests\Debug\net8.0\MSTestAdapter.PlatformServices.UnitTests.dll --treenode-filter '/*/*/(TestDataSourceHelpersTests)|(TestMethodInfoTests)|(MethodInfoExtensionsTests)/*' --report-trx --report-trx-filename adapter-dedup-net8.trx --results-directory artifacts\TestResults\predicate-clarity\dedup\net8\adapter

# 194 passed, 0 failed, 0 skipped; exit 0.
& .\artifacts\bin\MSTestAdapter.PlatformServices.UnitTests\Debug\net462\MSTestAdapter.PlatformServices.UnitTests.exe --treenode-filter '/*/*/(TestDataSourceHelpersTests)|(TestMethodInfoTests)|(MethodInfoExtensionsTests)/*' --report-trx --report-trx-filename adapter-dedup-net462.trx --results-directory artifacts\TestResults\predicate-clarity\dedup\net462\adapter

[Copilot]

🟢 Approval recommended

Both bounded private-helper extractions preserve existing logic, with no unresolved findings.

No open issues 🎉

What changed in this PR

Improves readability of MSTest cancellation-token data handling without changing behavior or APIs.

Changes:

• Extracts the adapter’s tuple-row condition into a private predicate.
• Extracts the framework’s display-name condition into a separate private predicate, preserving evaluation order.

Test-value evidence: The PR reports 243 focused net8.0 tests passing before and after (623f195 → 718c1a8). Execution artifacts were not independently verified.

🟢 Confidence at a glance — predicate extraction

Both supplied diffs preserve the original expressions, guards, and short-circuit ordering.

File	Description
src/​TestFramework/​TestFramework/​Internal/​TestDataSourceUtilities.cs	Names the predicate controlling display-name argument grouping.
src/​Adapter/​MSTestAdapter.PlatformServices/​Helpers/​TestDataSourceHelpers.cs	Names the predicate identifying tuple rows supplying a token.

_{Review effort: Balanced}

[github-actions]

🧪 Test quality review — PR #12044

Compared merge base c83be2a87a95cdd8db8ce526d1827c5d480875f7 with head
3a5f2ad4aacfb1e0f784bf6685b85f9e280ac04a. Evidence applies to this head, not
every intermediate commit.

Test-value evidence

This PR's only change is a pure private-helper extraction in src/Adapter/MSTestAdapter.PlatformServices/Helpers/TestDataSourceHelpers.cs: the compound condition previously inlined in IsDataConsideredSingleArgumentValue is moved verbatim into a new private method IsSingleTupleArgumentWithExplicitCancellationToken, with identical operands, operators, and short-circuit evaluation order (parameters.Length == 2 && data.Length == 1 && TryHandleTupleDataSource(...) && IsDataWithExplicitCancellationToken(...)). No other line in the file changed.

This is a refactor, not a behavioral change, so the applicable contract is: calling IsDataConsideredSingleArgumentValue with any given (data, parameters) pair must return the same boolean before and after the extraction. Static inspection confirms the extracted expression is a textual match for the removed inline expression — no operand, ordering, or null-handling difference exists. No buildable counterfactual (behavioral ablation) applies here since there is no new or changed observable behavior to falsify; a counterfactual is only meaningful for bug fixes or features.

Existing unchanged tests in test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Helpers/TestDataSourceHelpersTests.cs already exercise this exact contract through IsDataConsideredSingleArgumentValue_RejectsExplicitTokenTuplesForObjectArrayArgument and IsDataConsideredSingleArgumentValue_PreservesTupleAsObjectArrayData, covering both the "single tuple argument with explicit cancellation token" (rejected) and "tuple preserved as object[] data" (accepted) branches that the new private helper's name describes. These tests were not modified by this PR, so pass/fail evidence for them was not independently re-executed by this workflow (static prediction only, per this workflow's read-only mode); the PR description reports they were run locally (net8.0/net462) with 0 failures before and after the equivalent refactor, which is author-reported execution, not evidence this workflow directly verified.

No contract in this diff is left unprotected: the only changed code path is already covered by the tests above, and no test files, parallelization configuration, or other source files changed.

🧵 Parallel safety

PARALLEL-SAFETY RESULT
STATUS: NOT_APPLICABLE
ASSEMBLIES:
none
COUNTS: A=0; B=0; C=0; D=0; Critical=0; High=0; Warning=0; Info=0
TOP_ACTIONS:
none
FINDINGS:
none
NOTES: No changed tests or parallelization configuration were in scope.
END PARALLEL-SAFETY RESULT

Per-test assessment

No test files or test line ranges changed in this PR (test_count: 0), so there are no test methods to grade. The existing tests covering the refactored contract (noted above) were not modified and therefore fall outside this PR's grading scope.

Result	Quality	Test	Notes	How to improve
Not applicable — no test methods were added or modified in this PR.

No inline comments are published: the diff is a verified behavior-preserving extraction with no observable consequence to flag, and the tests protecting it are unchanged and pre-existing.

_{This advisory review is heuristic and non-blocking.}

Re-run the combined review with /review-tests, or only the parallel-safety
specialist with /parallel-audit.

🤖 Automated content by GitHub Copilot. Generated by the Test Quality Review on PR (on open / sync) workflow. · copilot · auto · 49.1 AIC · ⌖ 8.76 AIC · ⊞ 22.3K · ◷

[Copilot]

🟢 Approval recommended

The bounded private-helper extraction preserves the original conditions and evaluation order without introducing actionable defects.

No open issues 🎉

_{Review effort: Balanced}

[github-actions]

🔍 Build Failure Analysis

Summary — Azure DevOps reports Build Windows Release as the only failed build leg, but its binary log could not be decoded within the binlog-mcp deadline, so no reliable root cause or code fix can be proposed from this run.

The Windows Debug, Linux Release, and Windows application-model acceptance checks succeeded. All 13 application-model acceptance binlogs also report successful MSBuild executions with no errors. No failed leg is missing a published log; the remaining evidence gap is specifically the Windows Release binlog, whose error and overview queries repeatedly timed out. Access to the public Azure DevOps log fallback was also unavailable in this execution environment.

Please inspect or rerun the [Windows Release build log]((dev.azure.com/redacted) No inline suggestion or automated fix was requested because the failure evidence could not be read safely.

---

_{🤖 Generated by the Build Failure Analysis workflow using (a href="(dev.azure.com/redacted) · commit 3a5f2ad4aacfb1e0f784bf6685b85f9e280ac04a}

🤖 Automated content by GitHub Copilot. Generated by the Build Failure Analysis workflow. · copilot · gpt56 · 65.3 AIC · ⌖ 9.25 AIC · ⊞ 45.1K · [◷]( · ◷)

[github-actions]

🧪 Test quality review — PR #12044

Compared merge base 1a6a5f7b429320f4983e51416b23f76a1c8bfb6f with head
431b3e999aab9b8e6b05c78802e1f768b4041ff9. Evidence applies to this head, not
every intermediate commit.

Test-value evidence

This PR's only change deletes a duplicate private member in src/Adapter/MSTestAdapter.PlatformServices/Helpers/TestDataSourceHelpers.cs: the base revision contained two byte-for-byte identical definitions of IsSingleTupleArgumentWithExplicitCancellationToken(object?[] data, ParameterInfo[] parameters) (differing only in whitespace/comment wording), which is a C# CS0111 compile error (duplicate member with the same parameter types). The diff removes the second (redundant) definition and retains the first, unchanged.

This is a compile-fix / de-duplication, not a behavioral change to any executable predicate: the retained implementation is textually identical to the one callers (IsDataConsideredSingleArgumentValue) already used, so the only observable effect is that the project compiles again. A behavioral counterfactual doesn't apply — there is no single correct/incorrect runtime behavior to ablate, since the "bug" was a build break, not a wrong boolean result. The counterfactual that matters here is compilation: static inspection confirms the base-revision file (at merge_base) contains two definitions of the same signature, which is independently verifiable as a compile error; the head revision contains one, which compiles.

The retained predicate's existing behavior (tuple-row classification with a trailing explicit CancellationToken) is already exercised by unchanged tests in test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Helpers/TestDataSourceHelpersTests.cs, notably IsDataConsideredSingleArgumentValue_RejectsExplicitTokenTuplesForObjectArrayArgument and IsDataConsideredSingleArgumentValue_PreservesTupleAsObjectArrayData, which cover both the rejected (explicit-token tuple) and accepted (plain tuple-as-array) branches. These tests were not modified by this PR (test_count: 0), so no pass/fail evidence for them was independently re-executed by this workflow — this is a static prediction only, per this workflow's read-only mode. The PR description reports 196 net8.0 and 194 net462 tests passing locally against the resolved (de-duplicated) source; that is author-reported execution evidence, not something this workflow directly verified.

No contract in this diff is left unprotected: the sole changed code path (the retained predicate) is already covered by the unmodified tests above, and no test, parallelization-configuration, or other source files changed in this PR.

🧵 Parallel safety

PARALLEL-SAFETY RESULT
STATUS: NOT_APPLICABLE
ASSEMBLIES:
none
COUNTS: A=0; B=0; C=0; D=0; Critical=0; High=0; Warning=0; Info=0
TOP_ACTIONS:
none
FINDINGS:
none
NOTES: No changed tests or parallelization configuration were in scope (has_parallel_scope=false).
END PARALLEL-SAFETY RESULT

Per-test assessment

No test files or test line ranges changed in this PR (test_count: 0), so there are no test methods to grade. The existing tests covering the retained predicate (noted above) were not modified and fall outside this PR's grading scope.

Result	Quality	Test	Notes	How to improve
Not applicable — no test methods were added or modified in this PR.

No inline comments are published: the diff is a verified duplicate-member removal (compile-fix) with no observable behavioral consequence to flag, and the tests protecting the retained predicate are unchanged and pre-existing.

_{This advisory review is heuristic and non-blocking.}

Re-run the combined review with /review-tests, or only the parallel-safety
specialist with /parallel-audit.

🤖 Automated content by GitHub Copilot. Generated by the Test Quality Review on PR (on open / sync) workflow. · copilot · auto · 47.9 AIC · ⌖ 8.43 AIC · ⊞ 22.3K · ◷

[Copilot]

🟢 Approval recommended

The narrowly scoped deletion resolves the duplicate-member error without changing the retained logic or callers.

No open issues 🎉

_{Review effort: Balanced}

[github-actions]

Final test-triage resolution — Azure Pipelines build 1642270

This resolution supersedes any earlier preliminary comment for this PR/revision (head 431b3e999aab9b8e6b05c78802e1f768b4041ff9, merge 7f53512661fd674c149f5cf4ec3e55cf48166af1).

Summary: Evidence is incomplete, and no actionable test defect can be confidently attributed to this change (TestDataSourceHelpers.cs).

• Build Windows Release failed its "Test affected changes" step. The build timeline recorded one Code error: FSharpModuleTests.RunsNamedNestedExplicitAndCustomModulesAlongsideExistingClass ("net11.0", True) asserted the assembly marker should read assembly-initialize\nassembly-cleanup but observed assembly-cleanup written twice (AssemblyCleanup appears to have run twice for the useVstest=True / net11.0 combination on Windows).
• This is the only test-product signal in the failing leg, but the normalized evidence collector could not download the TestResults_Windows_Release_Attempt1 artifact (1 artifact-download failure), so no TRX/CTRF record, full stack trace, or retry/attempt detail is available for this specific failure beyond the raw pipeline log line above.
• The same test (net11.0, both useVstest values) passed on Linux Release in this build via CTRF and JUnit (two attempts each), so the failure is isolated to the Windows/VSTest leg and is not reproduced elsewhere in this run.
• history.json is itself marked incomplete (artifact-download-failed, download-budget-exhausted) and its tracked candidate set covers only NativeTests.Outcome, RetryFailsFirstAttempt, and related Windows_App_Model_Diagnostics probe names — it does not include FSharpModuleTests, so no prior-build recurrence data exists for this specific test.
• The other 6 raw "failed" records in this run (NativeTests.Outcome, WinUIMtpPackagedApp.UnitTest1.RetryFailsFirstAttempt, PackagedWinUI...RetryTestMethod_PassesOnSecondAppContainerHost) are deliberately-designed first-attempt-fails probes exercising the Retry extension's own fail-then-pass behavior (assertion text explicitly states "This probe deliberately fails the first retry attempt..."/"expected host failure"). The same failCount:4 / passCount:11 pattern recurs across 5 of the last 8 historical builds, consistent with expected-by-design test fixtures rather than flakiness or a regression — not actionable.
• The PR's only source change (src/Adapter/MSTestAdapter.PlatformServices/Helpers/TestDataSourceHelpers.cs) is unrelated to F# module/assembly-fixture lifecycle code, so there is no direct code-path link between the change and the assembly-cleanup duplication.

Confidence: Low-to-moderate that this is a real, Windows/VSTest-specific AssemblyCleanup double-invocation defect, but evidence is insufficient to confirm root cause, recurrence, or whether this is a pre-existing environment/timing issue versus a product bug, because the authoritative Windows Release TRX/CTRF artifact could not be retrieved.

Next diagnostic step: Re-run (or retrigger) the Windows Release leg to recover the TestResults_Windows_Release_Attempt1 artifact, and if the failure recurs, inspect the VSTest host log for the net11.0/useVstest=True run to determine whether AssemblyCleanup is invoked twice by the VSTest bridge path specifically (vs. MTP native). No issue is being filed because the only durable detail (recurrence across independent builds) is unavailable, and this is not shown to recur in Linux or in the un-downloadable Windows history.

🤖 Automated content by GitHub Copilot. Generated by the Pipeline Test Triage workflow. · copilot · auto · 212.4 AIC · ⌖ 6.36 AIC · ⊞ 9.8K