Three test-harness defects found during the 2026-08-14 test-shrink pass

[logbie]

Found while running the test-shrink skill on branch claude/test-shrink-skill-gdye4e. All three are outside that loop's scope (src/ changes or larger follow-ups), so they are recorded here rather than fixed in that pass. They are independent and can be taken separately.

---

1. Flaky test: interpreter::process_tests::test_capture_process_output

Where: src/interpreter/mod.rs:19109

The test spawns echo test output, sleeps a fixed 200 ms, then asserts the captured output:

let proc_id = client
    .spawn_process("echo", &["test output"], false, 0, 0)
    .await
    .expect("Failed to spawn process");

// Give process time to complete and output to be captured
tokio::time::sleep(tokio::time::Duration::from_millis(200)).await;

let output = client.read_process_output(&proc_id).await
    .expect("Failed to read process output");

assert!(output.contains("test output"), "Output should contain 'test output'");

Evidence it is flaky, not broken:

• Failed once during a loaded full-suite run (159 suites, machine under disk pressure):

  thread 'interpreter::process_tests::test_capture_process_output' panicked at src/interpreter/mod.rs:19132:9:
  Output should contain 'test output'
  

• Passed in a different full-suite run on the same commit.
• Passes 5/5 when run in isolation.

Cause: fixed-sleep synchronisation. 200 ms is enough on an idle machine and not always enough under load. Nothing about the assertion is wrong — the wait is.

Why it matters: per §8.2 of the binding Logbie Testing Policy (root testing.md), a flaky required test is a failing test, and required tests must never be made green via retries or relaxed assertions. So the fix should be to wait on the actual condition rather than lengthening the sleep — e.g. poll read_process_output until non-empty with a bounded timeout, or await process completion before reading.

Prior art: #239 was a similar sporadic-failure issue.

---

2. cargo test --all silently depends on a prior cargo build --release

Where: tests/binary_io_test.rs:7-17

fn wfl_binary() -> PathBuf {
    let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
    path.push("target");
    path.push("release");
    // ...
}

The path is hardcoded, so on a tree without a release build, six tests in this file fail with:

thread 'test_binary_read_write_roundtrip' panicked at tests/binary_io_test.rs:29:10:
failed to run wfl: Os { code: 2, kind: NotFound, message: "No such file or directory" }

CLAUDE.md does document that "integration requires release binary", but the failure surfaces as an opaque NotFound from .expect("failed to run wfl") rather than anything actionable. Anyone running cargo test --all on a fresh clone (or after cargo clean) hits six confusing failures.

Suggested fix: make the missing binary produce a clear diagnostic, e.g. check for existence first and panic with something like target/release/wfl not found — run 'cargo build --release' first. Worth applying to every test using the hardcoded release path, not just this one (see the wfl_release_exe() helper now in tests/common/mod.rs, which is the single place to add the check).

---

3. 30 file-I/O tests can leave files in the repo root when they fail

Where:

• tests/file_io_performance_test.rs (7 tests)
• tests/file_io_concurrent_test.rs (7 tests)
• tests/file_io_error_handling_test.rs (10 tests)
• tests/file_io_execution_test.rs (6 tests)

Each defines a local cleanup_test_files() that removes bare relative filenames (perf_small_0.txt, read_write_1.txt, readonly_test.txt, test_exec_exists.txt, concurrent_perf_0.txt, memory_test_large.txt, …), called once before the test body and once after. The trailing call is a plain statement — there is no Drop guard or catch_unwind — so if any assertion panics, cleanup never runs and the file is left in the working tree.

REPOSITORY_HYGIENE.md:101-102,113-114 requires test output to live under target/test-artifacts/ or a harness-provided temp dir, and to be removed on success and failure. CI's working-tree hygiene job runs after the test suites specifically to catch this, so one failing assertion currently turns into two failures: the test, then a confusing hygiene failure on a stray .txt.

The repo is clean today, so this is latent rather than active — but it fires the next time one of these 30 assertions goes red.

Two tiers of fix:

1. Low-risk, immediate: make cleanup unconditional via an RAII Drop guard, so it runs on panic too. ~30 small mechanical edits, no path changes.
2. Full policy conformance: migrate the 30 tests to tempfile::tempdir() with absolute paths spliced into the generated WFL source. test_directory_listing_performance in tests/file_io_performance_test.rs:201-241 already does exactly this and is the pattern to copy.

Note for whoever takes tier 2: do not fix this with std::env::set_current_dir() — cargo test runs tests in one process in parallel, so a process-wide CWD change would trade a cleanup bug for a flakiness bug.

Related: #156 previously covered extracting the duplicated cleanup_test_files helper.

---

Related

• Full write-up: History/dev-diary/2026/2026-08-14-test-shrink-pass.md
• Branch: claude/test-shrink-skill-gdye4e

[logbie]

Triaged against current main (4178714a, v26.8.2). All three confirmed; two of them have moved slightly since you filed, which changes the shape of the fix.

1. test_capture_process_output fixed-sleep flake — confirmed. src/interpreter/mod.rs still has the 200 ms sleep before the assertion, and a sibling test ~26 lines earlier uses a 100 ms sleep with the same pattern — so whatever fix lands should cover both, not just the one that flaked. Agreed on the diagnosis and on the §8.2 reading: lengthening the sleep would be exactly the "make a required test green without fixing it" move the policy forbids. Polling read_process_output until non-empty under a bounded timeout is the right shape. R3 by the policy (timing/process), so the evidence block wants a repeat-run count, not a single green.

2. Hardcoded target/release path — confirmed, and the fix site now exists. tests/binary_io_test.rs:7-17 still builds the path unconditionally. Since you filed, tests/common/mod.rs is present on main, so the wfl_release_exe() helper you describe has a home and the fix is no longer blocked on creating one. Worth doing as a single change that routes every hardcoded-release-path test through that helper with an existence check — a NotFound from .expect("failed to run wfl") is the least actionable failure in the suite.

3. Cleanup-on-panic in the 30 file-I/O tests — confirmed as latent. The working tree is clean today, so nothing is red, but the analysis holds: a bare trailing cleanup_test_files() call is skipped on unwind, and REPOSITORY_HYGIENE.md §101-102/113-114 requires removal on failure too. The double-failure mode you describe (assertion red, then a confusing hygiene red on a stray .txt) is the real cost — it turns one clear failure into two unclear ones.

Ordering suggestion, since these are independent but not equally urgent: (1) first — it is the only one that can turn main red without anyone touching the code, and a flaky required test erodes trust in every other signal. (2) second — cheap now that tests/common/mod.rs exists, and it removes a recurring "fresh clone, six mystery failures" onboarding tax. (3) last — latent, ~30 mechanical edits, and it overlaps with #693's migration; doing it during the batch that moves those four files avoids touching them twice.

Cross-reference: #693 (test-binary consolidation) migrates these same files, and PR #694 is the tests/common/mod.rs groundwork that item 2 depends on.

Suggested labels: bug, testing, flaky-test.

Posted by the WFL repo warden (automated triage pass).