Deduplicate the auto_discover shell bootstraps and poll instead of sleeping

[FAZuH]

Part of the whole-repo test-suite audit (2026-09-30). Cuts 330 lines of copy-paste and 144 seconds of wall clock.

What to build

Every auto_discover integration test bootstrapping its harness through the existing helper, and waiting on a poller instead of a fixed sleep.

Findings

• 18 hand-rolled ~30-line bootstraps. tests/auto_discover/recovery.rs inlines the consul + dummy0 + natmap + cat > /tmp/discovery.yaml + auto-discover block ten times (rg -c "cat > /tmp/discovery.yaml" tests/auto_discover/recovery.rs returns 10; consul agent -dev appears seven times). tests/auto_discover/forwarding.rs inlines the consul + natmap pair seven times, and startup_race.rs:17-49 inlines the whole thing once. new_format_setup_with_defaults_ext at tests/auto_discover/mod.rs:114 and new_format_setup at :110 already emit exactly that block, and 32 tests use them. There is no tests/common/ because mod.rs already is it; the copy-paste bypasses it.
• 160 fixed sleeps, 308 seconds of unconditional wait, across the tests/ tree. 36 of them are sleep 4 waiting for a Docker event to reach Consul. wait_for_consul_service at mod.rs:166 polls for exactly that condition and is used by only three tests. tests/natmap_docker.rs adds 39 copies of for i in $(seq 1 20); do [ -S /tmp/ns ] && break; sleep 0.2; done; extract one wait_for_socket helper.
• A shutdown-flush race. tests/natmap_docker.rs:287,309 send kill -INT $DAEMON_PID and then poll [ -S /tmp/ns ], but the socket already exists, so the loop breaks on its first iteration and the flush assertion races the shutdown. Poll kill -0 $DAEMON_PID instead.
• A duplicate test. tests/auto_discover/recovery.rs:411 remove_all_services_clean_slate and :266 remove_service_from_config_stale_deregistered are the same test: both kill the daemon, write services: {}, run sync, and assert the service is gone. Merge into one.

Acceptance criteria

• No test in tests/auto_discover/ writes a discovery config inline
• rg -c "sleep 4" tests/ finds no unconditional four-second wait for a Consul service
• Total suite wall clock drops by at least two minutes
• The shutdown-flush tests wait on the process, not on the socket file
• No test becomes order-dependent. The Docker suite runs single-threaded today via .cargo/config.toml; do not rely on that for correctness.
• cargo test -p lab-ops --test auto_discover passes

Blocked by

None (can start immediately).

Note: run the auto_discover integration suite deliberately, not as a side effect.

[FAZuH]

Landed on v0.2 as 5ac047f, merged eee8359. Closed by hand: a closing keyword only fires when a commit reaches the default branch, and this work is parked on the v0.2 version branch.

One acceptance criterion is NOT met, and I am not going to pretend otherwise

Total suite wall clock drops by at least two minutes

Actual, measured by stashing the change and re-running rather than estimated:

	wall	auto_discover	total tests
before	101s	95.04s	426
after	45s	34.19s	425

56 seconds, not two minutes. The ticket's own estimate is the reason: it counted 308 seconds of aggregate sleep, but cargo runs tests in parallel threads within a binary, so sleeps in different tests overlap and the saving is bounded by the critical path, not the sum. The sleeps really are gone — rg "sleep 4" tests/ is now 0, and no fixed sleep of 2s or more remains anywhere in tests/ — they just never cost the wall clock their total suggests.

Reaching two minutes would mean waiting less than the conditions actually need, which buys the number with flakiness. #54 is open and deliberately untouched, so that trade was not available to take here.

The rest of the criteria

Criterion	Result
No test in tests/auto_discover/ writes a discovery config inline	cat > /tmp/discovery.yaml now appears only in mod.rs, the single definition. consul agent -dev likewise: 4 files / 18 sites down to mod.rs alone.
No unconditional four-second wait	29 → 0
Shutdown-flush waits on the process	2 sites now poll kill -0 $DAEMON_PID. They were polling the socket file, which already exists, so the loop broke on its first iteration and the assertion raced the shutdown.
No order dependence	Nothing written depends on serialization. Note the ticket's claim that .cargo/config.toml enforces --test-threads=1 is false — it sets only rustflags, and AGENTS.md has been corrected to say so.
auto_discover passes	46 passed, 6 consecutive runs

Three bugs the rewrite surfaced

1. poll_until ran under set -e, so VAR=$(grep -c ...) — non-zero when the condition is false — killed the script on its first miss instead of retrying. Fixed once in the shared helper, so no call site can trip it again.
2. grep -c prints 0 and docker inspect prints false; both are non-empty strings, so a poll on either passes on its first iteration. Switched to grep -o and | grep true.
3. stop_job's trailing kill -0 ... && {...} returned non-zero on the success path, which would have aborted every && chain after it in all five natmap sites.

Test count 47 → 46

recovery.rs's remove_service_from_config_stale_deregistered was the same test as remove_all_services_clean_slate. The latter is kept, with the stronger assertion.

Flake observed, not fixed

registration::concurrent_starts_all_registered failed once in 9 full runs, and passes in isolation. The poll accepts everything the old sleep 10 plus assert accepted, evaluated later, so the diff does not cause it — but it is real and it is #54 territory, so it is reported here rather than chased.

Stale docs found, left for #41

• docs/dev/testing.md:179 — "must start the daemon … with & and sleep 2" is now false everywhere.
• docs/dev/testing.md:178 — "Docker tests hang: Always use --test-threads=1" still is not enforced by anything in the tree.
• tests/auto_discover/preserve_src_ip.rs:206 — # Run sync manually again sits above a command passing a Consul URL where sibling tests pass a config path.