Skip to content

fix(quickfiler): make datamodel and dispatcher tests independent of wall-clock timing - #971

Merged
drmoisan merged 26 commits into
mainfrom
bug/quickfiler-tests-depend-on-wall-clock-timing-950
Oct 2, 2026
Merged

drmoisan merged 26 commits into
mainfrom
bug/quickfiler-tests-depend-on-wall-clock-timing-950

Conversation

@drmoisan

@drmoisan drmoisan commented Oct 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • Removes the real-time bounded waits from the QuickFiler datamodel tests by starting the QfcDatamodel background worker through an injectable WorkerStarter seam; tests run DoWork synchronously on the calling thread.
  • Production behavior is unchanged: both QfcDatamodel instance constructors assign worker => worker.RunWorkerAsync(), and the two RunWorkerAsync call sites in InitEmailQueue now call WorkerStarter(worker).
  • Fixes the intermittent Transaction_SecondCallerCannotInstallUntilTheFirstRestores (R4) failure by pinning a non-null UiThread dispatcher baseline inside the transaction gate, closing a race with a gate-free writer in another test class under class-level parallelism.
  • No retries, no [DoNotParallelize], no Workers=1, no lengthened timeouts, and no Thread.Sleep/Task.Delay waits were introduced; the repository runsettings are unchanged.

Why

Several QuickFiler.Test tests failed intermittently under load and stopped unrelated executor gates and the required CI check. Research identified two root causes:

  • Defect A (timing). QfcDatamodelLivenessTests, QfcDatamodelTeardownTests and QfcInitEmailQueueZeroBatchTests blocked on five-second SpinWait.SpinUntil and Task.Wait calls because InitEmailQueue started its worker on a thread-pool thread the test could not control.
  • Defect B (raced static state). R4 failed because UiThreadDispatcherFixture.EnsureDispatcher() writes the process-wide UiThread._dispatcher static without taking the transaction gate, racing the theme tests in QfcItemController_FocusAndThemeTests under Workers=0 / Scope=ClassLevel.

What Changed

Production

  • QuickFiler/Controllers/QfcDatamodel.cs: new documented internal Action<BackgroundWorker> WorkerStarter { get; set; } (mirrors the existing RemainingEmailLoader convention; null on GetUninitializedObject instances, so a misconfigured test fails fast instead of starting a thread). Assigned in both constructors; replaces both RunWorkerAsync call sites.

Tests

  • QuickFiler.Test/Controllers/QfcDatamodelLivenessTests.cs, QfcDatamodelTeardownTests.cs, QfcInitEmailQueueZeroBatchTests.cs: assign a synchronous starter (test-side BackgroundWorker subclass exposing OnDoWork) and, where a continuation must be observed after release, a test-owned drainable SynchronizationContext. All wall-clock waits removed.
  • QuickFiler.Test/Controllers/QfcItemController.UiThreadDispatcherFixtureTests.cs: R4 opens a using over QfcItemControllerTestSupport.EnsureUiThreadDispatcher() after transaction A acquires the gate and before the original read; both assertions kept; the R4 doc comment now names the gate-free writer as the cause and states the residual-writer invariant.

Docs / evidence

  • Feature folder docs/features/active/2026-09-30-quickfiler-tests-depend-on-wall-clock-timing-950/: spec, plan, research, baseline / regression / QA-gate evidence, negative-control records, and the policy, code-review and feature audits.

Architecture / How It Fits Together

InitEmailQueue sets _remainingLoadActive = true and then calls WorkerStarter(worker). In production the starter is RunWorkerAsync(), so control flow is identical to before. In tests the starter raises DoWork synchronously, so the producer's progress is driven by the test rather than by the thread pool, and each assertion observes a deterministic state.

Verification

Completed (local, recorded in the feature folder evidence):

  • dotnet tool run csharpier check .: pass.
  • msbuild TaskMaster.sln /t:Rebuild ... /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true: pass.
  • msbuild TaskMaster.sln /t:Rebuild ... /p:TreatWarningsAsErrors=true: pass.
  • Full MSTest coverage run (fallback route excluding the four shell-icon test classes, which fail on this workstation for an environmental reason that reproduces on main): 7361 of 7361 passed; first-party coverage lines 85.35% to 85.36%, branches 79.74% to 79.75%; no new failures.
  • Negative controls recorded for every rewritten test; each fails immediately with an assertion or exception rather than hanging.
  • Feature review: PASS, 0 Blocking, 6 Non-blocking.

Pending:

  • AC17 (the full toolchain through the standard route) is checked off only from this pull request's own CI run on the final head, per the coordinator ruling recorded in spec.md.

Backward Compatibility / Migration Notes

None. The seam is internal, and the default starter preserves the previous behavior.

Risks and Mitigations

  • A new QfcDatamodel construction path that bypasses both constructors would leave WorkerStarter null. Mitigation: it fails fast with a NullReferenceException at InitEmailQueue, which matches the existing RemainingEmailLoader behavior.
  • The theme tests still discard their dispatcher scope. Mitigation: R4's in-gate pin makes that write harmless to R4; the theme-test exposure is tracked separately.

Review Guide

  1. QuickFiler/Controllers/QfcDatamodel.cs (16 lines).
  2. QfcItemController.UiThreadDispatcherFixtureTests.cs (R4 pin and doc comment).
  3. The three datamodel test files.
  4. spec.md and feature-audit.2026-10-02T14-30.md in the feature folder.

Follow-ups

Routed by the coordinator; none filed from this branch.

  • Consolidate the three duplicated synchronous-worker test helpers into QuickFiler.Test/TestSupport/ when the test csproj is next edited.
  • Reword the _remainingLoadActive doc comment in QfcDatamodel.QueueProcessing.cs to name WorkerStarter.
  • Remove or relocate the apparently dead legacy loader members in QfcDatamodel.cs (495 of 500 lines).
  • Dispose the SynchronousBackgroundWorker instances in the liveness and zero-batch tests.
  • Add try/finally around transactionA in R4.
  • Have the two FocusAndThemeTests theme tests hold their ensure scope (theme-test dispatcher exposure).
  • Canonical artifacts/csharp/coverage.xml absent in agent worktrees (recurring).
  • Derive evidence timestamp labels from the clock for orchestrator-authored artifacts.

GitHub Auto-close

🤖 Generated with Claude Code

drmoisan and others added 26 commits October 1, 2026 07:19
Parent parallel-orchestrator committed the child preparation work in its current state on a coordinator COMMIT NOW order. The commit holds the promoted entry, issue.md, spec.md and the plan file as the child left them. The plan may still be a scaffold. Preflight has not cleared. The parent authored no plan or specification content.

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
Research artifact stopped early for quota. Establishes defect A (BackgroundWorker pool-thread start forces wall-clock waits) and defect B (gate-free EnsureDispatcher seeding races the transaction test across parallel classes). Spec, plan and preflight remain.

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

Claude-Session: https://claude.ai/code/session_01NcieP6KJzhzgHkRh4F21Po
…ision R2)

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

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
… baseline

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
… baseline

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

drmoisan commented Oct 2, 2026

Copy link
Copy Markdown
Owner Author

AC17 final-head confirmation (coordinator ruling, option (a)).

  • Final head: dd367e2 (documentation-only relative to d46738a and e591b11)
  • CI run: 36972561867, conclusion success; all seven jobs success
  • mstest-coverage job 110729428245: Test Run Successful; Total tests 7384; Passed 7384; Failed 0
  • First-party coverage: lines 56615/65855 (85.97%), branches 13684/17078 (80.13%)

Earlier runs recorded in evidence/qa-gates/ac17-ci-evidence.md: 36971702087 on d46738a and 36972215199 on e591b11, both 7384/7384, lines 85.96%, branches 80.11%. Local fallback: 7361/7361, lines 85.35% to 85.36%, branches 79.74% to 79.75%.

@drmoisan
drmoisan merged commit 860d67b into main Oct 2, 2026
7 checks passed
@drmoisan drmoisan mentioned this pull request Oct 2, 2026
1 of 5 tasks
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.

Bug: quickfiler-tests-depend-on-wall-clock-timing

1 participant