Stop the test suite writing into its own repository (#431)

[jeremymanning]

Closes #431.

What was happening

tests/scenarios/test_network_failures.py generates pipeline YAML embedding whichever ephemeral port its mock server happened to bind, then wrote it into tests/scenarios/test_pipelines/ — a tracked directory. Every run left a diff:

-      url: "http://localhost:58198/test"
+      url: "http://localhost:54574/test"

Whichever port was committed last became the fixture everyone else got. Observed three separate times in one working session.

The part that wasn't in the issue

The directory was spelled as an absolute path to one developer's home directory:

self.test_pipelines_dir = Path("/Users/jmanning/orchestrator/tests/scenarios/test_pipelines")
self.test_pipelines_dir.mkdir(exist_ok=True)

mkdir(exist_ok=True) without parents=True raises FileNotFoundError when the parent is missing — so on any other machine, including CI, both classes in that file errored out rather than ran. Some of the legacy job's 195 errors are this.

Three more absolute paths did the same elsewhere:

File	Effect elsewhere
test_control_flow_conditional.py:11	sys.path.insert to a nonexistent dir
test_control_flow_dynamic.py:13	same
scenarios/test_real_world_examples.py:34	examples_dir missing → every example silently pytest.skipped

That last one is the worst of the three: it doesn't fail, it quietly reports nothing to test.

The fix

• Paths are repo-relative (Path(__file__).resolve().parents[...]).
• Generated pipelines go to pytest's tmp_path, via a small _UsesTemporaryPipelineDir mixin shared by the two classes that need it — no duplicated setup/teardown.
• All six YAML files in tests/scenarios/test_pipelines/ are deleted. Every one was written by this module and read by nothing else: outputs masquerading as fixtures. Verified by grep before removing.

CI guard

The fix alone regresses the next time someone saves test output "somewhere convenient", so the legacy job now asserts git diff --quiet afterwards. Deliberately not continue-on-error — the backlog is allowed to be red, it is not allowed to modify tracked files.

One change you may want to veto

While in that job, -rN → -rfE.

-rN suppresses the summary entirely, so the job publishes a single scalar. That means "did anything regress?" is unanswerable from CI: a net −2 is equally consistent with 5 fixed and 3 broken. I hit this twice today and had to re-run both sides locally to attribute changes — and I reported "nothing regressed" on #438 on aggregate evidence that didn't actually support it. -rfE names failures and errors so the backlog is diffable run to run.

The summary step reads the last line for its tally, which -rfE leaves in place, so that step is unaffected. Output gets longer — say the word and I'll drop this hunk.

Verification

A/B on the same 8 tests, same environment:

	result	tree
main	8 failed	2 tracked files modified
this branch	8 failed	clean

The 8 failures are the pre-existing "no control system provided and no models available" backlog — identical both ways. Only the mutation is gone; this PR neither fixes nor worsens those.

Blocking suite 456 passed, 13 skipped, 0 failed. Collection 3251, unchanged. uv lock --check, ruff on src and on every file touched, and ci.yml YAML parse all clean.

[jeremymanning]

CI 9/9 green, and the legacy numbers reconcile exactly — but only after I fixed my own comparison.

	main @ ed51346	this PR
failed	468	474
errors	195	189
passed	1846	1846
skipped	200	200
total red	663	663

Six tests moved ERROR → FAILED. Nothing else moved. That is precisely what this change predicts: those tests died in setup on FileNotFoundError from mkdir against the missing /Users/jmanning/... parent, and they now run and fail on the pre-existing "no control system provided and no models available" backlog. Total red unchanged, passed unchanged.

The remaining two of the eight in that file were already FAILED rather than ERROR — the test_web_search_timeout path never called mkdir, so it failed later and differently.

The mistake worth recording

My first pass at this compared against run 30715042500, which is the #439 PR-branch run (466/1848/195), not the post-merge main run (468/1846/195). That made it look like two passing tests had regressed, and I went looking for a bug that did not exist.

PR-branch runs and post-merge runs are different commits with different content. Only post-merge main runs are a valid baseline for the next PR. Consecutive main runs confirm the suite is stable enough for this to be signal rather than noise:

ed51346 (post-#439)  468 failed, 1846 passed, 195 errors
8d92913 (post-#438)  468 failed, 1846 passed, 195 errors
125e406 (post-#437)  471 failed, 1843 passed, 195 errors

Byte-identical across two consecutive runs, so a ±2 swing would have been real — which is exactly why I chased it.

The guard ran

The git diff --quiet step executed and passed, so CI now confirms the suite leaves the repository clean rather than my having to check by hand.

-rfE produced 663 named FAILED/ERROR lines, and I used them in this analysis to identify which six tests converted. Under -rN that question was unanswerable — which is the whole argument for the flag.