Skip to content

Backport client statefulness capability fix to rel/4.4 - #11293

Merged
Amaury Levé (Evangelink) merged 1 commit into
rel/4.4from
dev/amauryleve/backport-11037-rel-4-4
Sep 15, 2026
Merged

Amaury Levé (Evangelink) merged 1 commit into
rel/4.4from
dev/amauryleve/backport-11037-rel-4-4

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

Backports #11037 to rel/4.4.

This is part 1 of 4 and should be merged first.

Validation

  • .\\build.cmd -pack -bl — passed with 0 warnings and 0 errors.
  • Client capability, JSON serialization, adapter run-settings, and VSTest bridge tests — 184 passed.
  • Server-mode acceptance coverage passed as part of the targeted backport validation.

All MSBuild-based validation commands produced binary logs.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0d36ee80-2921-424a-aa64-3dab520a3d6a
@github-actions github-actions Bot added the state/needs-review Awaiting review from the team. label Sep 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #11293

Parallelization — one row per test assembly audited:

Test assembly Scope Workers Analyzer coverage
MSTestAdapter.UnitTests off (bespoke TestContainer engine — no MSTest parallel scheduler) n/a n/a
Microsoft.Testing.Extensions.VSTestBridge.UnitTests MethodLevel CPU count coverable once the parallel-safety analyzers ship (assembly attribute)
Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests MethodLevel CPU count coverable once the parallel-safety analyzers ship (assembly attribute)
Microsoft.Testing.Platform.UnitTests MethodLevel CPU count coverable once the parallel-safety analyzers ship (assembly attribute)
Microsoft.Testing.Platform.Acceptance.IntegrationTests MethodLevel CPU count coverable once the parallel-safety analyzers ship (assembly attribute)

No .runsettings / testconfig.json opt-ins were found for any of these assemblies, and this PR does not touch any parallelization-state file (no .runsettings, testconfig.json, Directory.Build.*, or [assembly: Parallelize]/[DoNotParallelize] change) — the config-files list for this PR is empty.

Findings: A (global-state) 0 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 0.

This backport (#11293, backporting #11037 to rel/4.4) changes 7 test files, all narrowly scoped to the IsStateful/DeclaredIsStateful nullable-capability rename and its new "undeclared" test cases:

  • MSTestRunSettingsTests.cs (MSTestAdapter.UnitTests, TestContainer-based — no live parallel risk regardless).
  • RunSettingsPatcherTests.cs, ObjectModelConvertersTests.cs (Microsoft.Testing.Extensions.VSTestBridge.UnitTests).
  • MtpServerClientTests.cs (Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests) — the new tests construct a fresh FakeMtpServer/TcpListener bound to an ephemeral port (new TcpListener(IPAddress.Loopback, 0)) per test instance, so there is no shared port or shared state across tests.
  • JsonTests.cs, ClientCapabilitiesExtensionsTests.cs (Microsoft.Testing.Platform.UnitTests) — pure serialization/deserialization assertions against locally-constructed records; no shared statics.
  • ClientCapabilities.cs (record definition used by acceptance tests) — a type-only change (boolbool?), no test logic touched.

I checked each changed test method and lifecycle member for: process-global state mutation (env vars, CWD, console, culture, static fields — category A), shared filesystem paths (category B), and [ResourceLock]/[DoNotParallelize] declaration mismatches (category C). None of the changed tests mutate environment variables, current directory, console state, culture, or any shared static; none touch the filesystem; and none of the surrounding classes declare [ResourceLock]/[DoNotParallelize] that this PR's changes would invalidate. All new/modified assertions operate on locally-scoped mocks, dictionaries, and freshly-constructed objects.

Nothing to flag for parallel-safety in this PR.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 114 AIC · ⌖ 4.09 AIC · ⊞ 23.9K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #11293

This PR renames ClientCapabilitiesService.IsStateful to DeclaredIsStateful (nullable, three-state: declared true / declared false / undeclared) and adds a GetIsStateful() extension so undeclared-vs-explicit-false can be distinguished. The changed/added tests exercise this new tri-state distinction across the MSTest adapter's MSTestRunSettings, the VSTestBridge RunSettingsPatcher, the wire-level JSON (System.Text.Json and Jsonite) serialization/deserialization paths, the MtpServerClient wire payload, and the new ClientCapabilitiesExtensions.GetIsStateful() helper itself.

GradeTestMutationNotesHow to improve
A (90–100) new ClientCapabilitiesExtensionsTests.
GetIsStateful_
ForwardsCustomImplementation
2/2 killed Data-driven test proves the extension falls back to IsStateful for non-ClientCapabilitiesService implementers.
A (90–100) new ClientCapabilitiesExtensionsTests.
IsStateful_
UndeclaredCapability_
DefaultsToFalse
1/1 killed Confirms the DeclaredIsStateful: null case defaults IsStateful to false via the record.
A (90–100) new MSTestRunSettingsTests.
UndeclaredNonVisualStudioClient
DoesNotSetDesignMode
1/1 killed Verifies undeclared statefulness on a non-VS client falls back to false (no VS heuristic).
A (90–100) new MSTestRunSettingsTests.
UndeclaredVisualStudioClient
SetsDesignModeForBackwardCompatibility
1/1 killed Verifies the legacy-VS backward-compat fallback only fires when statefulness is undeclared.
A (90–100) new MSTestRunSettingsTests.
StatelessVisualStudioClient
DoesNotSetDesignMode
1/1 killed Verifies an explicit false declaration overrides the VS backward-compat fallback.
A (90–100) new RunSettingsPatcherTests.
Patch_UndeclaredNonVisualStudioClient_
DoesNotSetDesignMode
1/1 killed Mirrors the MSTest-adapter case for RunSettingsPatcher.Patch directly.
A (90–100) new RunSettingsPatcherTests.
Patch_UndeclaredVisualStudioClient_
SetsDesignModeForBackwardCompatibility
1/1 killed Covers the VS legacy-compat branch when statefulness is undeclared.
A (90–100) new RunSettingsPatcherTests.
Patch_StatelessVisualStudioClient_
DoesNotSetDesignMode
1/1 killed Covers the explicit-false-overrides-VS-heuristic branch for RunSettingsPatcher.Patch.
A (90–100) new MtpServerClientTests.
InitializeAsync_DefaultOptions_
LeaveStatefulnessUndeclared
1/1 killed End-to-end over a real loopback connection; asserts the wire request omits isStateful by default.
A (90–100) new MtpServerClientTests.
SerializeClientCapabilities_
UndeclaredStatefulness_OmitsProperty
1/1 killed Verifies null IsStateful serializes by omitting the wire property entirely.
A (90–100) new MtpServerClientTests.
SerializeClientCapabilities_
DeclaredStatefulness_IncludesProperty
2/2 killed Data-driven true/false cases confirm the declared value round-trips onto the wire payload.
A (90–100) mod JsonTests.
Deserialize_InitializeRequest_
WithIsStatefulFalse_StjPath_
SurfacesStatelessClient
1/1 killed Renamed/repurposed to cover explicit false; matches the StJ deserializer's JsonValueKind.False branch.
A (90–100) new JsonTests.
Deserialize_InitializeRequest_
WithoutIsStateful_StjPath_
LeavesCapabilityUndeclared
1/1 killed Confirms a missing isStateful property deserializes to null, not a default.
A (90–100) new JsonTests.
Deserialize_InitializeRequest_
WithInvalidIsStateful_StjPath_Throws
1/1 killed Non-boolean JSON value hits the deserializer's MessageFormatException default arm.
A (90–100) new JsonTests.
Deserialize_InitializeRequest_
WithNullIsStateful_StjPath_Throws
1/1 killed JSON null for isStateful is neither True/False, so it throws as designed.
A (90–100) mod JsonTests.
Deserialize_ClientCapabilities_
WithIsStatefulFalse_JsonitePath_
SurfacesStatelessClient
1/1 killed Jsonite-path counterpart of the StJ false-case rename; matches SerializerUtilities's bool-cast branch.
A (90–100) new JsonTests.
Deserialize_ClientCapabilities_
WithoutIsStateful_JsonitePath_
LeavesCapabilityUndeclared
1/1 killed Confirms missing key on the Jsonite path deserializes to null.
A (90–100) new JsonTests.
Deserialize_ClientCapabilities_
WithInvalidIsStateful_JsonitePath_Throws
1/1 killed Non-bool value hits the Jsonite deserializer's type-check throw branch.
A (90–100) new JsonTests.
Deserialize_ClientCapabilities_
WithNullIsStateful_JsonitePath_Throws
1/1 killed JSON null value is not a bool, so the Jsonite path throws as designed.

No actionable inline findings were raised — every reviewed test asserts a value that traces to a distinct, verifiable branch of the DeclaredIsStateful/GetIsStateful() behavior (declared-true, declared-false, undeclared, and invalid-input paths), uses the assertion style native to its project (AwesomeAssertions in the adapter suite, MSTest Assert elsewhere), and each pseudo-mutation on the relevant production branch is killed.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 271.6 AIC · ⌖ 2.81 AIC · ⊞ 15.9K · [◷]( · )

@microsoft-github-policy-service microsoft-github-policy-service Bot added state/needs-review Awaiting review from the team. and removed state/needs-review Awaiting review from the team. labels Sep 15, 2026
@github-actions github-actions Bot added state/approved Proposal approved; ready for implementation. and removed state/needs-review Awaiting review from the team. labels Sep 15, 2026
@Evangelink
Amaury Levé (Evangelink) merged commit d9d6504 into rel/4.4 Sep 15, 2026
39 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/backport-11037-rel-4-4 branch September 15, 2026 10:12
@github-actions github-actions Bot removed the state/approved Proposal approved; ready for implementation. label Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants