[file-diet] Refactor oversized NamedPipeServer.cs (565 lines) into focused files
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 25/100
- Issue type
- Refactor
- Clarity
- Mostly clear
- Activity status
- Active
- Tech stack
- csharp
- Domain
- testing-qa
Research direction
Start with src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs and read the static pipe-naming helpers (GetPipeName, ResolvePipeDirectory, EnsurePathLengthWithinLimit) before the connection loop. Check every project that compiles this file via shared sources, and confirm the class can become partial without breaking them. Done means the split files each stay under 300 lines, behavior is unchanged, and the relevant MTP tests and builds pass.
Written by the indexing model from the issue text.
Description
Overview
The file src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs has grown to 565 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/IPC/NamedPipeServer.cs - Size: 565 lines
- Language: C#
Structural Analysis
The file defines a single internal sealed class NamedPipeServer : NamedPipeConnectionBase, IServer, mixing several distinct responsibilities:
- Construction / stream creation: multiple public constructor overloads,
RequiresExplicitSecurity, andCreateServerStream, which build the underlyingNamedPipeServerStream, optionally hardening it with security identities on Windows. - Connection lifecycle:
WaitConnectionAsync,InternalLoopAsync(the read/dispatch/write request loop),WaitForDisconnectAsync, and the smallTryLogDebugAsynclogging helper used during graceful shutdown. - Pipe path resolution (static, OS-specific):
GetPipeName,ResolvePipeDirectory(both overloads),EnsureDirectoryIsWritable, andEnsurePathLengthWithinLimit— this cluster deals with computing/validating the Unix domain socket or Windows pipe name, including theTESTINGPLATFORM_PIPE_DIRECTORYoverride and thesun_pathbyte-length limit. - Disposal:
Dispose()andDisposeAsync()at the end of the file (not fully inspected in this bounded sample, but visibly part of the instance lifecycle group).
Refactoring Strategy
Proposed File Splits
Based on the bounded structural sample, investigate these proposed splits. Confirm their safety during implementation preflight before moving code. Because the class is sealed and not currently partial, each new file would need NamedPipeServer to be declared partial — verify this does not conflict with source generation, reflection-based name lookups, or other partial declarations elsewhere.
-
NamedPipeServer.cs(retained core)- Contents: class declaration, constants/fields, all public constructors,
RequiresExplicitSecurity,CreateServerStream,PipeNameproperty,WasConnected,WaitConnectionAsync,InternalLoopAsync,TryLogDebugAsync - Responsibility: instance construction and the core connect/read/dispatch/write request loop
- Contents: class declaration, constants/fields, all public constructors,
-
NamedPipeServer.PipeNaming.cs- Contents:
GetPipeName,ResolvePipeDirectory(both overloads),EnsureDirectoryIsWritable,EnsurePathLengthWithinLimit - Responsibility: static, OS-specific pipe name/path computation and validation (Windows naming vs. Unix domain socket directory resolution,
TESTINGPLATFORM_PIPE_DIRECTORYoverride, writability probe,sun_pathlength limit)
- Contents:
-
NamedPipeServer.Lifecycle.cs(if needed)- Contents:
WaitForDisconnectAsync,Dispose(),DisposeAsync() - Responsibility: connection-disconnection waiting and resource disposal
- 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. Note this file is referenced from multiple platform/extension projects via shared compilation (
#if MTP_MSBUILD_TASKS, etc.) — confirm every consuming project's includes/links are updated for any new file. - Move Before Modernizing: Move bodies unchanged first, one coherent responsibility at a time. Preserve allocation and performance behavior on hot paths (this type is on the IPC hot path); 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: Medium
Expected Impact: Improved code navigability, easier testing, reduced merge conflicts
🤖 Automated content by GitHub Copilot. Generated by the Daily File Diet workflow. · copilot · auto · 41.7 AIC · ⌖ 6.61 AIC · ⊞ 13.1K · [◷]( · ◷)
- expires on Oct 16, 2026, 6:59 PM UTC
- Dominant language
- C#
- Stars
- 1k
- Forks
- 314
- Avg merge
- 9h 17m
- Merged PRs (30d)
- 436
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
-
[quality-improver] AOTSG0001–AOTSG0005 (AOT reflection source generator) diagnostics have no HelpLinkUri, unlike MSTEST0xxx rulesPossibly taken @Evangelink claimed this 1 day ago. 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
-
area/performance type/automation
Difficulty 5/5 Over a week Newbie friendliness 20/100
Maintainers usually reply within 1 day
All issues in microsoft/testfx
Similar issues
-
AzureBlobSnapshotStore.LoadAsync allocates the snapshot twice (growable MemoryStream + ToArray)Openenhancement
Difficulty 2/5 Half a day Newbie friendliness 72/100
-
bug ui-only
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
Maintainers usually reply within 1 day
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
ClickHouse/clickhouse-cs#689 ·
Maintainers usually reply within 2 days
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
Maintainers usually reply within 7 days
-
Difficulty 1/5 Under an hour Newbie friendliness 65/100
vchelaru/FlatRedBall2#1481 ·
Maintainers usually reply within 1 day