Skip to content

test(940): remove repository-root and TaskMaster.sln dependence from the file-system wrapper tests - #955

Merged
drmoisan merged 9 commits into
mainfrom
bug/filesystem-wrapper-tests-open-repository-solution-file-940
Sep 30, 2026
Merged

drmoisan merged 9 commits into
mainfrom
bug/filesystem-wrapper-tests-open-repository-solution-file-940

Conversation

@drmoisan

Copy link
Copy Markdown
Owner

Suggested title

test(940): remove repository-root and TaskMaster.sln dependence from the file-system wrapper tests

Summary

  • Rewrites UtilitiesCS.Test/HelperClasses/PhysicalFileSystemAdapters_Tests.cs and UtilitiesCS.Test/HelperClasses/DirectoryInfoWrapper_Tests.cs so that no test locates the repository root or opens, enumerates or mutates TaskMaster.sln or any other tracked repository file.
  • Removes the IOException swallowing that hid file-handle contention instead of removing it.
  • Every wrapper and adapter member the old tests exercised is still exercised, against a Moq mock, a test-owned read-only fixture under the test assembly output directory, or a documented non-existent-path outcome. No temporary file or directory is created.
  • Test-only change: no production file is modified. The three production files the tests cover (PhysicalDirectoryInfoAdapter.cs, PhysicalFileInfoAdapter.cs, DirectoryInfoWrapper.cs) are byte-identical to main.
  • Eleven negative controls show each rewritten test failing against a deliberately broken wrapper or adapter, and passing again after the revert.
  • The three further root-walk sites named in the bug report are classified in evidence and left unmodified.

Why

The wrapper tests walked up to the repository root and used the real TaskMaster.sln as their fixture. Their outcome therefore depended on the repository layout and on whether another process (a resident MSBuild node or an IDE) held the solution file open with a share mode that excludes readers. This is the same defect class that #931 (PR #939) removed from FileInfoWrapper_Tests. Several tests also caught IOException, which tolerated the contention rather than eliminating it. Tests must run in parallel (Workers=0, ClassLevel), so the fix does not serialise the run.

What Changed

Tests

  • PhysicalFileSystemAdapters_Tests.cs: the GetRepositoryRoot and GetSolutionFile helpers are removed. Read-only members are exercised against the test assembly's own loaded image and output directory. Mutating members (timestamps, attributes, read-only flag, access control, create, copy, replace, move, delete) are exercised only against a path under the output directory that is asserted not to exist before the call, or against an existing owned entry on which the call is a no-op by construction. Three test methods were added to keep every previously exercised member covered.
  • DirectoryInfoWrapper_Tests.cs: the repository-root walk is removed; enumeration delegation is asserted through Moq-based IDirectoryInfo mocks and a rooted fixture path.
  • No DoNotParallelize, worker-count or scope change, retry, sleep or timeout is introduced.

Documentation and evidence

  • Feature folder docs/features/active/2026-09-29-filesystem-wrapper-tests-open-repository-solution-file-940/: issue, research, atomic plan (version 1.3), Phase 0 baselines, fail-before exception dossier, the eleven mutation controls, final QC evidence and the reduced-audit artifacts.
  • AC8 in issue.md was amended on 2026-09-30 per a coordinator ruling (dated note under the acceptance criteria): the package-level and repository-level not-lower coverage comparison was replaced by a per-file no-regression rule, for covered lines and covered branches, over the three changed wrapper and adapter files, with the first-party floors unchanged (line at least 80 percent, branch at least 75 percent). Two identical coverage runs of the same committed tree had shown run-to-run variance in files this change does not touch (PropertyStore.cs, OlTableExtensions.Etl.cs, SubjectMapSco.Orchestration.cs).

Architecture / How It Fits Together

The production seams are unchanged. PhysicalDirectoryInfoAdapter and PhysicalFileInfoAdapter wrap System.IO.DirectoryInfo and System.IO.FileInfo behind the internal IDirectoryInfo and IFileInfo interfaces, and DirectoryInfoWrapper delegates to an IDirectoryInfo. The tests now supply either a mock of those interfaces or a real DirectoryInfo/FileInfo whose target the test assembly owns, so delegation is verified without depending on repository layout or on other processes.

Verification

Completed (evidence committed under the feature folder)

  • C# toolchain, final clean pass (evidence/qa-gates/toolchain-pass.md, LOOP: CLEAN PASS, one iteration): dotnet tool run csharpier format . (0 files rewritten) and dotnet tool run csharpier check . exit 0; analyzer rebuild (msbuild TaskMaster.sln /t:Rebuild ... /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true) and nullable rebuild (... /p:TreatWarningsAsErrors=true) exit 0 with 0 errors and 0 skipped CoreCompile targets, re-run after merging main.
  • Scoped run of the two rewritten classes in the parallel regime: 15 of 15 passed (evidence/regression-testing/test-run-final.md).
  • Full MSTest coverage run on the merged head (MEASUREMENT 3): 7327 of 7327 passed; first-party coverage lines 56092/65736 (85.33 percent), branches 13597/17054 (79.73 percent) (evidence/qa-gates/coverage-final.md).
  • Per-file no-regression rule (AC8 as amended), MEASUREMENT 3 against baseline:
    • PhysicalDirectoryInfoAdapter.cs: lines 81/91 to 91/91, branches 36/42 to 36/42.
    • PhysicalFileInfoAdapter.cs: lines 69/75 to 71/75, branches 6/12 to 6/12.
    • DirectoryInfoWrapper.cs: lines 123/123 to 123/123, branches 3/4 to 3/4.
  • In-memory negative control of the per-file check: lowering any one of the three files by one covered line or one covered branch reports NOT-LOWER=False in every case; the coverage document hash is unchanged before and after.
  • Eleven mutation controls, each failing with the predicted message and passing after revert, with a clean-tree proof afterwards (evidence/regression-testing/mutation-*.md, p1-t31-post-control-clean-tree).
  • Scope boundary: the committed source footprint is exactly the two test files; production diff against main exits 0; hygiene sweep 0 findings (p2-t10-scope-boundary, p2-t11-hygiene-sweep, p2-t20-closure).
  • Reduced audit: policy audit, code review and feature audit dated 2026-09-30T12-00, verdict PASS, 0 blocking findings, all 8 acceptance criteria verified.
  • Local runs excluded four UtilitiesCS.Test shell-icon classes and OSBrowser_Tests through a fixed filter because they stall on the local workstation (reproduced on main); CI runs them.

Recommended

  • CI on this pull request (format check, analyzer and nullable builds, MSTest with coverage) is the closing gate, including the CSharpier check over the files merged from main.

Backward Compatibility / Migration Notes

None. Only test code changed; no public API, production behaviour, build configuration or runsettings file changed.

Risks and Mitigations

  • Risk: SetAccessControl with an unmodified security object is a real, idempotent write on the owned output directory and loaded image. Mitigation: it is admitted by the amended AC3/AC4 wording as a no-op by construction; seaming it is listed as a follow-up.
  • Risk: the enumeration test assumes the output directory's parent contains exactly one Debug-named directory. Mitigation: this matches the standard build layout locally and in CI; recorded as a non-blocking review finding.
  • Rollback: revert this pull request; no production or data state is affected.

Review Guide

  1. UtilitiesCS.Test/HelperClasses/PhysicalFileSystemAdapters_Tests.cs (the main rewrite).
  2. UtilitiesCS.Test/HelperClasses/DirectoryInfoWrapper_Tests.cs.
  3. issue.md acceptance criteria and the AC8 dated note.
  4. evidence/qa-gates/coverage-final.md (MEASUREMENT 3, per-file comparison, negative control, variance tables).
  5. The remaining evidence and the plan are supporting audit trail and can be skimmed.

Follow-ups

To be filed by the coordinator; not filed from this branch.

  • UtilitiesCS.Test/EmailIntelligence/SortEmail_Tests.cs TrySaveAttachmentAsync_WhenSaveSucceeds_ReturnsTrueAndCallsSaveAsFile is classified as the same defect class (production Directory.CreateDirectory under the repository root); left unmodified as required.
  • Seam SetAccessControl on both physical adapters so the tests need no real DACL write.
  • PhysicalFileInfoAdapter constructor null-guard branches remain uncovered (6 of 12 branches, pre-existing).
  • Coverage of PropertyStore.cs, OlTableExtensions.Etl.cs and SubjectMapSco.Orchestration.cs varies between identical runs, which makes package-level and repository-level not-lower comparisons unreliable as gates.
  • Minor test-shape findings from the code review (coarse-grained assertion methods, missing AAA markers in four DirectoryInfoWrapper tests, redundant sentinel stream opens).

GitHub Auto-close

🤖 Generated with Claude Code

…C, cleared plan)

Adds the active minor-audit folder for issue 940: issue.md with explicit Acceptance Criteria AC1-AC8, research, the atomic plan cleared by three atomic-executor preflight rounds (PREFLIGHT: ALL CLEAR), and the preflight clearance record. Restores the promoted potential record from the session branch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Claude-Session: https://claude.ai/code/session_01GG13mBq6qiRbu6ZDYRyBn9
… fix

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…es instead of the repository solution file

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…, control records and plan check-offs

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…or per-file coverage ruling

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…-system wrapper test fix

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…(0 blocking)

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
@drmoisan
drmoisan merged commit 039cf77 into main Sep 30, 2026
7 checks passed
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.

1 participant