test(931): remove scheduler and file-handle dependence from breadcrumb thread-affinity and FileInfoWrapper tests - #939
Merged
drmoisan merged 17 commits intoSep 30, 2026
Conversation
…an active full-bug feature folder Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com Claude-Session: https://claude.ai/code/session_01KoweznWqwJTkNCf6756FoF
…rapper fixture Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com Claude-Session: https://claude.ai/code/session_01KoweznWqwJTkNCf6756FoF
…nd acceptance criteria Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com Claude-Session: https://claude.ai/code/session_01KoweznWqwJTkNCf6756FoF
…eInfoWrapper test fixes Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com Claude-Session: https://claude.ai/code/session_01KNZiXntshsLY8vqqCHUvHm
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com Claude-Session: https://claude.ai/code/session_01KNZiXntshsLY8vqqCHUvHm
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com Claude-Session: https://claude.ai/code/session_01KNZiXntshsLY8vqqCHUvHm
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com Claude-Session: https://claude.ai/code/session_01KNZiXntshsLY8vqqCHUvHm
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com Claude-Session: https://claude.ai/code/session_01KNZiXntshsLY8vqqCHUvHm
… test fix Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com Claude-Session: https://claude.ai/code/session_01KNZiXntshsLY8vqqCHUvHm
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com Claude-Session: https://claude.ai/code/session_01KNZiXntshsLY8vqqCHUvHm
…b affinity and FileInfoWrapper tests Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com Claude-Session: https://claude.ai/code/session_01KNZiXntshsLY8vqqCHUvHm
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com Claude-Session: https://claude.ai/code/session_01KNZiXntshsLY8vqqCHUvHm
…ntrolled-environment test fix Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com Claude-Session: https://claude.ai/code/session_01KNZiXntshsLY8vqqCHUvHm
…ironment-931 Brings in the analyzer-path hotfix 89e202e and the merged sibling items. The only item-scope file touched is QuickFiler.Test/QuickFiler.Test.csproj, and only its Analyzer Include version paths changed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Policy audit, code review and feature audit at 2026-09-29T20-15 against head ce744bb. All 19 acceptance criteria verified; no remediation cycle required. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s with a placeholder The reviewer's final pass rewrote one line in each of the policy and feature audits after they were committed, so the committed evidence carried the account name. No figure or verdict changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Sep 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Threadand asserts inside that thread that it is not the owner thread, instead of relying onTask.Runto provide a different thread.FileInfoWrapper_Testsin UtilitiesCS.Test. The tests no longer locate or open the repository'sTaskMaster.sln;OpenReadis exercised through the existing internalFileInfoWrapper(IFileInfo)seam with a test-ownedFileStream, and the three metadata tests use a rooted literal path.QuickFiler.Test.TestSupport.DedicatedWorkerThread.Run(Action), and splitsItemViewerBreadcrumbThreadAffinityTestsinto two partial files so each stays under the 500-line limit..claudeorconfigfile is modified. The parallel regime (Workers zero, Scope ClassLevel) is unchanged.Why
Both defects share one root cause: a unit test depended on environment state it does not control.
Task.Runguarantees a thread-pool work item, not a thread different from the caller. Under the parallel run every test executes on a pool thread, and a blocking wait on a not-yet-started task can execute the work item inline on the waiting thread. A guard that decides by thread identity then either admits the call (the test passes without exercising the guard) or produces a spurious failure. Of the 22Task.Runsites in QuickFiler.Test, the research triage classified exactly two as affected:BreadcrumbPopupBoundaryCoverageTests.Dispatcher_OwnerOnlyWorker_ReportsWithoutRunningActionandItemViewerBreadcrumbThreadAffinityTests.InitializeBreadcrumbPipeline_NullOwningDispatcher_DoesNotThrow. The other 20 reach guards that decide bySynchronizationContextreference or only complete aTaskCompletionSource, so they are left unchanged.OpenRead_ShouldReturnReadableStreamForWrappedFileopenedTaskMaster.slnwith the defaultFileShare.Read. Any other process holding that file with a share mode excluding readers (for example a resident MSBuild node) made the test throwIOException, so its outcome depended on build history rather than on the wrapper.The fix does not serialise the run: no
[DoNotParallelize], Workers change, retry, sleep, timeout or wall-clock wait is introduced.What Changed
Tests (QuickFiler.Test):
TestSupport/DedicatedWorkerThread.cs(new):internal static Exception Run(Action action)starts a backgroundThread, joins it without a timeout, and returns the exception the delegate threw or null. It carries no assertion; each test states its own distinctness precondition.Viewers/BreadcrumbPopupBoundaryCoverageTests.cs:Dispatcher_OwnerOnlyWorker_ReportsWithoutRunningActioncaptures the owner thread id, runsDispatchthroughDedicatedWorkerThread.Run, and asserts in-thread thatEnvironment.CurrentManagedThreadIddiffers from the owner id. The two existing assertions are unchanged.Viewers/ItemViewerBreadcrumbThreadAffinityTests.csandViewers/ItemViewerBreadcrumbThreadAffinityTests.Part2.cs(new): the class is nowpartial. The three cross-thread tests andClearViewerDispatchermove to Part2 and call the shared helper; the privateRunOnDedicatedWorkerThreadis removed.InitializeBreadcrumbPipeline_NullOwningDispatcher_DoesNotThrowcaptures the owningDispatcherand assertsCheckAccess()is false inside the dedicated thread. No test is renamed or removed.QuickFiler.Test.csproj: twoCompile Includeentries for the new files.Tests (UtilitiesCS.Test):
HelperClasses/FileInfoWrapper_Tests.cs:GetSolutionFile()and everyTaskMaster.slnreference removed.OpenRead_ShouldReturnReadableStreamForWrappedFileuses a strictMock<IFileInfo>returning aFileStreamopened over the test assembly's own location withFileAccess.ReadandFileShare.ReadWrite, and asserts same-instance identity,CanRead, andLength > 0. The three metadata tests use the rooted literalC:\Repo\fixture.sln. No file is created, written or deleted.Docs:
docs/features/active/2026-09-28-tests-depend-on-uncontrolled-environment-931/: research, spec (19 acceptance criteria), atomic plan, baseline, regression-testing and QA-gate evidence projections, and the policy-audit, code-review and feature-audit artifacts.docs/features/potential/promoted/2026-09-28-tests-depend-on-uncontrolled-environment.md: promotion record written by the promotion lifecycle.Merge: origin/main (c4ff0e2) was merged at 55a50e9 with no conflicts. The only item-scope file it touched is
QuickFiler.Test.csproj, and only its Analyzer Include version paths (taken from main).Architecture / How It Fits Together
BreadcrumbUiDispatcher.IsCurrentBoundary()and the null-owner escape inItemViewer's UI-boundary guard. AThreadconstructed by the test is distinct from every live thread by construction, so the in-thread precondition holds under any scheduler, and an untimedJoin()does not park a pool slot waiting on another pool slot.FileInfoWrapper's contract forOpenReadis pure delegation, so the test verifies it through the internal seam (reachable via the existingInternalsVisibleTo("UtilitiesCS.Test")). The public constructor path and the explicitDirectoryInfoWrappercast remain covered by the three metadata tests.Verification
Completed (evidence committed under the feature folder):
action()inserted as the first statement ofDedicatedWorkerThread.Run(all four dedicated-thread tests fail at their precondition); OpenRead mock pointed at a second stream.dotnet tool run csharpier check .exit 0; analyzermsbuild ... /t:Rebuild ... /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=trueexit 0;msbuild ... /t:Rebuild ... /p:TreatWarningsAsErrors=trueexit 0; zero "Skipping target CoreCompile" lines in both rebuilds; MSTest-with-coverage route exit 0.Recommended:
Backward Compatibility / Migration Notes
Risks and Mitigations
post-merge-coverage-final.mdattributes this to production edits already on main (ILGlobals.cs,UiThread.cs) plus run-to-run collector variance in unchanged files; this change modifies no production line. Recorded as a non-blocking disclosure in the audits.Review Guide
QuickFiler.Test/TestSupport/DedicatedWorkerThread.cs(48 lines).QuickFiler.Test/Viewers/BreadcrumbPopupBoundaryCoverageTests.cs(one test body).QuickFiler.Test/Viewers/ItemViewerBreadcrumbThreadAffinityTests.csand.Part2.cs: mostly a mechanical move of three tests and one helper; review the rewrittenInitializeBreadcrumbPipeline_NullOwningDispatcher_DoesNotThrowbody and remark.UtilitiesCS.Test/HelperClasses/FileInfoWrapper_Tests.cs(four test bodies).QuickFiler.Test/QuickFiler.Test.csproj(two Compile entries).Follow-ups
Deferred, not fixed here. They are not filed from this branch because the promotion record would fall outside this change's footprint criterion; they are recorded in
evidence/qa-gates/p4-t13-follow-up-handoff.2026-09-29T09-46.mdand in the spec's Rollout and Follow-up section for filing from a separate branch:UtilitiesCS.Test/HelperClasses/PhysicalFileSystemAdapters_Tests.cslocates and opens the repository solution file and swallowsIOException; same defect class.UtilitiesCS.Test/HelperClasses/DirectoryInfoWrapper_Tests.csasserts thatTaskMaster.slnis enumerated from the repository root; same defect class.QuickFiler.Test/Viewers/BreadcrumbUiThreadDispatchTests.cs: the asserted message wording "cannot marshal cross-thread UI work" is broader than theDispatchValuemechanism (wording only).DispatchValuesite to the owner-thread-id check.Also noted by the review:
quality-tiers.ymlis absent at the repository root (pre-existing).GitHub Auto-close
🤖 Generated with Claude Code