[file-diet] Refactor TestMethodInfo.Execution.cs (574 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
- 48/100
Research direction
Start with src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.Execution.cs and read the complete method bodies, then inspect the neighboring TestMethodInfo.*.cs partials and .github/skills/behavior-preserving-refactor/SKILL.md. Trace consumers, project includes, source links, and the NETFRAMEWORK conditional before moving code. Done means focused files stay under 300 lines where safe, behavior and API contracts are preserved, and affected builds and tests pass.
Written by the indexing model from the issue text.
Description
Overview
The file src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.Execution.cs has grown to 574 lines, making it harder to navigate and maintain. This task involves refactoring it into smaller, more focused files.
Current State
- File:
src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.Execution.cs - Size: 574 lines
- Language: C#
Structural Analysis
This file is already one of six partial class TestMethodInfo files in the Execution folder (alongside .ArgumentResolution.cs, .ExceptionHandling.cs, .Lifecycle.cs, .TestContext.cs, and .cs), so the project already follows a per-concern partial-class convention — this file itself has simply outgrown that convention and can be split further along the same lines.
From the bounded sample (first 100 lines + a member-signature grep), the file contains:
GetTestTimeout()— resolves the effectiveTimeoutAttribute/default timeout for a test method.ThrowMultipleAttributesException/ThrowMultipleClassAttributesException— small[DoesNotReturn]helpers that raiseTypeInspectionExceptionfor duplicate attribute usage.ExecuteInternalAsync(...)(lines ~77–336) — the large core method that orchestrates global/local test initialization, test invocation, and global/local cleanup.DefaultTestClassInstanceLease(lines ~336–363) — a private nestedsealed classimplementingITestClassInstanceLease.ExecuteInternalWithTimeoutAsync(...)(lines ~363–526) — the timeout-wrapping variant of execution.TestInvocationState(lines ~526–570) — a private nested class tracking activation/cleanup state (PublishActivation,CompleteTestContextSetup).TestInvocationCleanupSnapshot(line ~570+) — a private nestedreadonly record structcapturing cleanup snapshot data.
Refactoring Strategy
Proposed File Splits
Based on the bounded structural sample, investigate these proposed splits as provisional — the sample used for this issue did not read the full method bodies or verify consumer/test coupling, so their safety must be confirmed during implementation preflight before moving any code:
-
TestMethodInfo.Execution.cs(retained, trimmed)- Contents:
GetTestTimeout(),ThrowMultipleAttributesException,ThrowMultipleClassAttributesException, and the primaryExecuteInternalAsync(...)entry point. - Responsibility: Core non-timeout test execution orchestration and small attribute-validation helpers.
- Contents:
-
TestMethodInfo.Execution.Timeout.cs- Contents:
ExecuteInternalWithTimeoutAsync(...)and theTestInvocationStatenested class it uses to track invocation/cleanup state. - Responsibility: Timeout-bounded test execution path, isolated from the non-timeout path.
- Contents:
-
TestMethodInfo.Execution.Types.cs(if needed)- Contents:
DefaultTestClassInstanceLeaseandTestInvocationCleanupSnapshotnested types. - Responsibility: Small supporting types (lease/snapshot) shared by both execution paths, separated from the executable logic.
- 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 (e.g. the
#if NETFRAMEWORKblock at the top of the file), and tracked public/internal API surfaces. Keep names, accessibility, attributes, and native interop contracts unchanged. - 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 (following the existing
TestMethodInfo.*.cspartial-class convention) - 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 (including the
NETFRAMEWORK-conditional code path); 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 AIC · ⌖ 10.4 AIC · ⊞ 12.9K · [◷]( · ◷)
- expires on Oct 21, 2026, 7:02 PM UTC
- Dominant language
- C#
- Stars
- 1.1k
- Forks
- 312
- Avg merge
- 8h 33m
- Merged PRs (30d)
- 411
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 4/5 3-5 days Newbie friendliness 35/100
Maintainers usually reply within 1 day
-
needs/triage
Difficulty 4/5 3-5 days Newbie friendliness 48/100
Maintainers usually reply within 1 day
-
agentic-workflows type/automation type/tech-debt
Difficulty 1/5 Under an hour Newbie friendliness 1/100
Maintainers usually reply within 1 day
-
area/mtp-github-actions-report external/dotnet-test needs/triage
Difficulty 4/5 3-5 days Newbie friendliness 56/100
Maintainers usually reply within 1 day
-
area/dump needs/triage
Difficulty 2/5 1-3 hours Newbie friendliness 45/100
microsoft/testfx#11981 · 2 comments ·
Maintainers usually reply within 1 day
All issues in microsoft/testfx
Similar issues
-
bug priority: P2
Difficulty 1/5 1-3 hours Newbie friendliness 84/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 77/100
Maintainers usually reply within 1 day
-
[Docs] README/PRIVACY still name AppCenter although v1.5.6.0+ uses the Microsoft Store Services SDKOpen
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
needs-triage
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
Azure/azure-sdk-tools#17317 ·
Maintainers usually reply within 1 day
-
Test
Difficulty 2/5 1-3 hours Newbie friendliness 69/100
Maintainers usually reply within 1 day