Skip to content

Fix flaky cache-sharing test on Windows - #49

Merged
wangjohn merged 1 commit into
mainfrom
fix-flaky-cache-sharing-test
Oct 14, 2026
Merged

wangjohn merged 1 commit into
mainfrom
fix-flaky-cache-sharing-test

Conversation

@wangjohn

@wangjohn wangjohn commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

Summary

Fixes the flaky tests/test_cache.py::test_processes_can_share_the_cache, which failed on PR #47 ("test (Python 3.11, windows-latest, syntax=false)") with one writer's last entry missing, though all three writers exited 0. A re-run passed and #47 did not touch the cache.

Root cause

The cache behaved as designed; the test asserted more than the cache promises.

  • close() gives up on a writer thread that has finished no batch for HAND_DEADLINE_S (5 s). It then sets problem = "the cache writer stopped responding", drops what is unwritten, and the daemon writer dies with the process. That is the cache's guarantee never to stall a run.
  • But a writer may legitimately wait up to TIMEOUT_S (10 s) for another process's SQLite lock, and writer startup (connect, ACL checks on Windows, PRAGMAs) counts as "no progress". On a loaded runner a writer can be stalled past 5 s, so close() abandons it. Each writer's last entry is in its final batch, written only at close(), which is why entry 299 went missing.
  • The child scripts never reported problem, and the test never looked at it, so a documented skip looked like data loss.

Evidence (Linux, deterministic): with another connection holding the write lock, a 3 s hold writes 300/300 rows. A 7 s hold, still inside TIMEOUT_S, makes close() return after 5.0 s with problem set and 0/300 rows on disk.

Not proven: I could not reproduce it naturally on Linux (3 writers under 16 CPU burners, 30 runs; 12 writers, 15 runs; all passed), so what stalled the writer on the Windows runner is inference (file locking, startup cost, a busy runner), not observed.

Fix (test only; src/styleprofile/cache.py is unchanged)

  • test_processes_can_share_the_cache now asserts the actual contract: no process fails, integrity_check is ok, every entry present is correct, every entry is present unless that writer reported a problem, and the store is readable and writable afterwards. The child writers use generous timeouts (documented in the test) and print problem, so a loaded runner does not trigger the give-up path in the first place.
  • New test_a_lock_released_in_time_loses_no_writes: a lock released within the deadline loses nothing and sets no problem.
  • New test_a_lock_held_too_long_skips_the_writes_and_says_so: close() returns while the lock is held, sets problem, and the file is intact and usable afterwards.
  • CHANGELOG entry under Unreleased / Fixed.

The assertion was not weakened to hide a bug: with no problem reported, every entry is still required.

Validation

  • make check: 913 passed, 10 skipped; ruff, format and pyright clean.
  • Repetition under load (16 CPU burners): 50/50 sequential runs and 40/40 runs (5 rounds of 8 parallel pytest processes) of the three cache-sharing tests passed.
  • make bench-quick not run: the cache code is unchanged.
  • CI on this PR: all 11 jobs green, including test (Python 3.11, windows-latest, syntax=false) and Windows platform regressions (Python 3.11). One Windows pass cannot show a flake is gone; the deterministic tests are what pin the behaviour.

Follow-ups (found, left alone)

  • HAND_DEADLINE_S (5 s) is below TIMEOUT_S (10 s), and writer startup counts as no progress. A cache run can therefore drop writes while a writer is still legitimately waiting on a lock. Aligning them would let close() stall up to ~10 s, which trades against the never-stall guarantee, so it needs a product decision.
  • Skipped writes are only visible through problem; I did not check whether a run's final output always surfaces it. Worth checking if users should be told.

🤖 Generated with Claude Code

https://claude.ai/code/session_01B2k6VAeJXgzMiegggNtJGn

The test asserted that a cache writer never drops entries, but the cache
gives up on a writer that makes no progress for HAND_DEADLINE_S (setting
problem). Assert the real contract, give the child writers generous
timeouts, and add deterministic tests for a lock released in time and one
held too long.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01B2k6VAeJXgzMiegggNtJGn
@wangjohn
wangjohn marked this pull request as ready for review October 14, 2026 06:39
@wangjohn
wangjohn merged commit c480dae into main Oct 14, 2026
11 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.

2 participants