[file-diet] Refactor MtpServerClient.cs (581 lines) into focused partial files
Maintainers usually reply within 1 day
Assessment
This issue has not been assessed yet.
Description
Overview
The file src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs has grown to 581 lines, making it harder to navigate and maintain. This task involves refactoring it into smaller, more focused files.
Current State
- File:
src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs - Size: 581 lines
- Language: C#
Structural Analysis
MtpServerClient is the internal sealed class implementation of IMtpServerClient, a client over MtpJsonRpcConnection used to talk to an MTP server. Based on a bounded sample (first 100 lines plus a member-signature scan), it contains three distinguishable groups of responsibility:
- Construction / lifecycle: constructors,
Launch,LaunchAsync,LaunchInProcessAsync,InitializeAsync,Dispose,DetachHandlers,EnsureStarted— connecting to and tearing down the underlying transport/process. - Test execution:
DiscoverCoreAsync,RunCoreAsync,OnServerRequestAsync— sending discovery/run requests and handling server-initiated requests. - Event/notification plumbing:
OnNotificationReceived,RaiseTestNodesUpdated,RaiseLog,RaiseTelemetry,RaiseAttachments— translating raw JSON-RPC notifications into the publicTestNodesUpdated,LogReceived,TelemetryReceived,AttachmentsReceivedevents.
This file is internal sealed, which makes a partial class split across files a natural, low-risk option for a single-project source-included package (.Sources suffix suggests it is distributed as source, so public/internal surface for consumers outside this project must stay unchanged).
Refactoring Strategy
Proposed File Splits
Based on the bounded structural sample, investigate these proposed splits. Confirm their safety during implementation preflight before moving code — treat these as provisional until verified against the full file body, its callers, and existing tests:
-
MtpServerClient.cs(retained)- Contents: Class declaration, fields, constructors, public events, properties (
ServerRequestHandler,ProcessId,ServerExitCode,Capabilities),Launch/LaunchAsync/LaunchInProcessAsync/InitializeAsync,Dispose,DetachHandlers,EnsureStarted. - Responsibility: Client identity, connection lifecycle, and startup/teardown.
- Contents: Class declaration, fields, constructors, public events, properties (
-
MtpServerClient.Execution.cs(new partial)- Contents:
DiscoverCoreAsync,RunCoreAsync,OnServerRequestAsync. - Responsibility: Sending discovery/run requests to the server and responding to server-initiated requests.
- Contents:
-
MtpServerClient.Notifications.cs(new partial)- Contents:
OnNotificationReceived,RaiseTestNodesUpdated,RaiseLog,RaiseTelemetry,RaiseAttachments. - Responsibility: Deserializing JSON-RPC notification payloads and raising the corresponding public events.
- Contents:
Implementation Guidelines
- Establish Equivalence: Follow
.github/skills/behavior-preserving-refactor/SKILL.md. Before editing, read the complete affected bodies, inventory baseline behavior and relevant tests, and identify distinguishing cases. - Preserve Selection and Evaluation: Keep iteration order, short-circuit evaluation, exact type selection, first/last-match semantics, defaults, exceptions, and side effects. Cover reachable duplicate and malformed inputs; similar-looking helpers are not automatically interchangeable.
- Trace All Consumers: Check partial declarations, source-linked consumers, owning/consuming project includes, target-framework guards, and tracked public/internal API surfaces. Keep names, accessibility, attributes, and native interop contracts unchanged. Because this file lives under a
.Sourcesproject, verify how it is included (source link vs. compile item) by every consuming project and ensure all new partial files are included identically. - Move Before Modernizing: Move bodies unchanged first, one coherent responsibility at a time. Preserve allocation and performance behavior on hot paths; do not mix adjacent modernization or bug fixes into the split.
- Use Focused Checks: Run the smallest affected project/TFM builds and relevant tests before and after each coherent split. Add distinguishing equivalence cases where coverage is missing; a pure file move does not require the full solution. Report exact checks and outcomes, including unavailable checks.
- Surface Constraints: Measure resulting line counts. If a safe structural split cannot meet the size goal without changing semantics, report the remaining size and blocker rather than forcing behavior changes.
Acceptance Criteria
- Original file is split into focused modules
- Each new file is under 300 lines, or a concrete behavior-preservation blocker and remaining size are reported
- Baseline semantics, including reachable duplicate/malformed cases and evaluation order, are preserved by focused equivalence checks
- Relevant builds and tests pass for affected consumers/TFMs; exact checks and any validation gaps are reported
- Public and tracked internal API contracts, attributes, and conditional compilation are preserved
- Project includes, source links, and package layout still include the moved code for every affected consumer
Priority: Medium
Effort: Small
Expected Impact: Improved code navigability, easier testing, reduced merge conflicts
🤖 Automated content by GitHub Copilot. Generated by the Daily File Diet workflow. · copilot · auto · 39.6 AIC · ⌖ 5.19 AIC · ⊞ 13K · [◷]( · ◷)
- expires on Oct 15, 2026, 7:01 PM UTC
- Dominant language
- C#
- Stars
- 1k
- Forks
- 314
- Avg merge
- 8h 49m
- Merged PRs (30d)
- 442
Getting set up
- No Dockerfile or Docker Compose file
- Has a pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from microsoft/testfx
-
type/automation type/tech-debt
Difficulty 3/5 Half a day Newbie friendliness 45/100
Maintainers usually reply within 1 day
-
type/automation type/tech-debt
Difficulty 4/5 3-5 days Newbie friendliness 25/100
Maintainers usually reply within 1 day
-
[quality-improver] AOTSG0001–AOTSG0005 (AOT reflection source generator) diagnostics have no HelpLinkUri, unlike MSTEST0xxx rulesPossibly taken @Evangelink claimed this today. Opentype/automation type/tech-debt
Difficulty 3/5 1-2 days Newbie friendliness 40/100
microsoft/testfx#11920 · 1 reaction · 2 assignees ·
Maintainers usually reply within 1 day
-
area/server-mode-jsonrpc
Difficulty 5/5 Over a week Newbie friendliness 8/100
Maintainers usually reply within 1 day
-
testing type/automation type/test-gap
Difficulty 2/5 1-3 hours Newbie friendliness 20/100
Maintainers usually reply within 1 day
All issues in microsoft/testfx
Similar issues
-
clawsweeper:source-repro impact:ux-release-blocker issue-rating: 🦞 diamond lobster P0
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
openclaw/openclaw-windows-node#1723 · 1 comment · 1 reaction ·
Maintainers usually reply within 1 day
-
good first issue hacktoberfest help wanted new-language
Difficulty 2/5 Half a day Newbie friendliness 78/100
-
[Android] Controls.DeviceTests build fails with xUnit2031 analyzer errorPossibly taken @HarishwaranVijayakumar claimed this today. Openpartner/syncfusion s/triaged s/verified
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
dotnet/maui#39376 · 1 reaction ·
Maintainers usually reply within 1 day
-
documentation
Difficulty 1/5 1-3 hours Newbie friendliness 62/100
-
bug misleading tooling trivial
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
Maintainers usually reply within 1 day