The cookbook race that reddened two branches in one afternoon - #207
Conversation
`test_every_recipe_runs_offline_and_is_repeatable[verify-in-github-actions]` failed once on `release/0.11.0` and once on a v0.12 branch, passed on re-run, and passed 19/19 in isolation. I recorded it in #206 as an unexplained flake rather than as fixed. It is neither: it is a shared-directory race with a one-line cause. Recipes ran in `examples/cookbook/<name>/` itself, and **two tests share a recipe**. Under `-n auto` they land on different workers and run concurrently in that one directory. `verify-in-github-actions` writes `verify-report.json`, reads it, then `rm -f`s it, so one worker's delete lands between the other's write and read. `_run` invokes `bash -euo pipefail`, which turns the resulting `FileNotFoundError` into a non-zero exit, and the assertion fires with nothing wrong in the library. Measured, four concurrent runs of that recipe with the flags the test uses: shared directory 2 rc=0 2 rc=1 one copy per run 4 rc=0 Each test now copies the recipe into its own `tmp_path`. The repeatability test still runs twice in **one** directory, which is its whole point, and that directory is now its own rather than the repository's. The suite also stops writing into the working tree, so `git status` after a run says what it should. Worth stating plainly: a suite that fails once in a while teaches people to re-run it, and this project's gate is only worth anything while green means green. I should have chased this the first time instead of labelling it. Signed-off-by: arpan <contact@arpanghoshal.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughCookbook tests now copy each recipe into a temporary per-test directory. Recipe processes run from the copy. Repeatability checks execute twice in the same sandbox, and refusal checks use a separate sandbox. ChangesCookbook test isolation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to This test-only change correctly isolates cookbook runs and removes the reported shared-directory race without introducing a demonstrated merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The red This branch has no same-named docs branch, so CI pairs it against docs The |
2 participants
test_every_recipe_runs_offline_and_is_repeatable[verify-in-github-actions]failed once onrelease/0.11.0and once on a v0.12 branch, passed on re-run, and passed 19/19 in isolation. I recorded it in #206 as an unexplained flake rather than as fixed. It is neither.The cause
Recipes ran in
examples/cookbook/<name>/itself, and two tests share a recipe. Under-n autothey land on different xdist workers and run concurrently in that one directory.verify-in-github-actionsdoes this:One worker's
rm -flands between the other's write and read._runinvokesbash -euo pipefail, so theFileNotFoundErrorbecomes a non-zero exit and the assertion fires with nothing wrong in the library.ctrlrun verifyspawns eight processes forG4, so the window is widest exactly when the suite is busiest, which is why the full parallel run reproduced it and-k cookbooknever did.Measured
Four concurrent runs of that recipe, with the flags the test uses:
The fix
Each test copies the recipe into its own
tmp_path. The repeatability test still runs twice in one directory, which is its whole point, and that directory is now its own rather than the repository's. The suite also stops writing into the working tree, sogit statusafter a run says what it should.Gate
Worth stating plainly: a suite that fails once in a while teaches people to re-run it, and this project's gate is only worth anything while green means green. I should have chased this the first time instead of labelling it.
🤖 Generated with Claude Code
Summary by CodeRabbit