From 8bda466eed2fb1add72ae2599dab6c8e4aeac9dc Mon Sep 17 00:00:00 2001 From: arpan Date: Tue, 15 Sep 2026 02:37:46 +0530 Subject: [PATCH] The cookbook race that reddened two branches in one afternoon `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//` 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 --- tests/test_cookbook.py | 42 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/tests/test_cookbook.py b/tests/test_cookbook.py index 99b9fe76..169c5c7d 100644 --- a/tests/test_cookbook.py +++ b/tests/test_cookbook.py @@ -18,6 +18,7 @@ from __future__ import annotations import os +import shutil import subprocess import sys from pathlib import Path @@ -40,7 +41,29 @@ ) -def _run(recipe: str, no_network: Path) -> subprocess.CompletedProcess[str]: +def _sandbox(recipe: str, tmp_path: Path) -> Path: + """A private copy of one recipe's directory. + + **Recipes used to run in `examples/cookbook//` itself, and two tests share a recipe.** + Under `pytest -n auto` they land on different workers and run concurrently in that one + directory, so `verify-in-github-actions`, whose script writes `verify-report.json`, reads it + and then `rm -f`s it, raced itself: one worker's delete landed between the other's write and + read, `bash -euo pipefail` turned the `FileNotFoundError` into a non-zero exit, and the run + went red with nothing wrong in the library. + + It reddened two different branches in one afternoon before it was reproduced, which is the + cost worth naming: 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. + + Copying also stops the suite writing into the working tree at all, so `git status` after a + test run says what it should. + """ + destination = tmp_path / recipe + shutil.copytree(COOKBOOK / recipe, destination) + return destination + + +def _run(recipe: str, no_network: Path, where: Path) -> subprocess.CompletedProcess[str]: environment = dict(os.environ) environment["PYTHONPATH"] = os.pathsep.join( part for part in (str(no_network), environment.get("PYTHONPATH", "")) if part @@ -48,7 +71,7 @@ def _run(recipe: str, no_network: Path) -> subprocess.CompletedProcess[str]: environment["PYTHONDONTWRITEBYTECODE"] = "1" for name in ("CTRLRUN_CONFIG", "CTRLRUN_STATE", "CTRLRUN_STORE_URL"): environment.pop(name, None) - script = COOKBOOK / recipe / "main.py" + script = where / "main.py" command = ( [sys.executable, str(script)] if script.exists() else ["bash", "-euo", "pipefail", "run.sh"] ) @@ -58,7 +81,7 @@ def _run(recipe: str, no_network: Path) -> subprocess.CompletedProcess[str]: ) return subprocess.run( command, - cwd=COOKBOOK / recipe, + cwd=where, env=environment, capture_output=True, text=True, @@ -68,17 +91,20 @@ def _run(recipe: str, no_network: Path) -> subprocess.CompletedProcess[str]: @pytest.mark.parametrize("recipe", RECIPES) -def test_every_recipe_runs_offline_and_is_repeatable(recipe, no_network): - first = _run(recipe, no_network) +def test_every_recipe_runs_offline_and_is_repeatable(recipe, no_network, tmp_path): + """Twice in **one** directory, which is the point, and that directory is this test's own.""" + where = _sandbox(recipe, tmp_path) + + first = _run(recipe, no_network, where) assert first.returncode == 0, f"{recipe} failed:\n{first.stdout}\n{first.stderr}" - second = _run(recipe, no_network) + second = _run(recipe, no_network, where) assert second.returncode == 0, f"{recipe} is not repeatable:\n{second.stdout}\n{second.stderr}" @pytest.mark.parametrize("recipe", RECIPES) -def test_every_recipe_refuses_something_and_says_so(recipe, no_network): +def test_every_recipe_refuses_something_and_says_so(recipe, no_network, tmp_path): """The share unit is a failure and a refusal: every recipe's output shows one.""" - output = _run(recipe, no_network).stdout.lower() + output = _run(recipe, no_network, _sandbox(recipe, tmp_path)).stdout.lower() refusals = ( "refused", "blocked",