Repository navigation
Conversation
…the faults they assert on They took the first ingested turn as proof that the notify watcher was registered, made one directory, and read the first fault in the whole spool. Under load that directory could land before the watcher existed and never be reported, and a ws_restart from a lost port race could be spooled ahead of it. The tests now make a fresh directory per check until one is reported, filter the spool to ingest_failed faults for the paths they made, assert that no such fault names a vanished path, and wait on the turn rows they assert on instead of a running total.
The three comments the review found longer than the repository's one-line rule are cut to one line each; no code changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was flaky
The three e2e tests added in #49 failed 5 times in 125 runs under CPU load. Two causes, both in the tests:
faults[0]/faults.is_empty()over the whole spool. The harness picks WebSocket ports withfree_port()(bind port 0, release), so daemons started at the same time can collide, and the loser spools aws_restart("Address already in use") ahead of anything the tests look at.Change
Tests only, in
crates/lumen-daemon/tests/e2e_pipeline.rs. No product code, no sleep added.unreadable-N.jsonldirectory on every poll ofwait_until, until a fault for one is spooled.ingest_failed/notifyfault a minute: a probe made before the vanish would take that slot and hide a regression.ingest_failures(spool, named): kindingest_failed, file name matching. The two "is not a fault" tests now assert that no such fault names the vanished path, not that the spool is empty. The "cannot read" test asserts anotifyfault for anunreadable-Ndirectory.has_turn) instead of a running count.Evidence
Same loop before and after: the compiled test binary runs the three tests N times, 16 at a time, beside 16 busy-loop processes (load average 70 to 86 on a 10-core machine), killed afterwards.
The 5 failures on the current tests: 4
ws_restartport collisions read as the first fault (three tests), 1 lost watcher event (timed out waiting for the directory fault). A lighter profile (125 runs) did not fail.Both causes also reproduce on demand with a local, uncommitted daemon change:
ws_restartspooled at startupEach assertion still goes red when the behaviour it guards breaks (local, reverted, not committed):
ErrorKind::NotFoundhandling iningest_fromremoved: the deletion test and the listed-then-gone test fail in 45 of 45 runs (5 direct, 40 under load); the "cannot read" test stays green.Not changed
free_port()still has its bind race, which can fail other e2e tests the same way. Fixing it means handing the daemon a listener or retrying on a bind failure, which is a harness or product change of its own.Checks
The local CI-equivalent gate (fmt, clippy, workspace tests, frontend, e2e, GUI, MSRV, install verify) is green on macOS. The Linux and Windows legs run in CI only; the listed-then-gone test stays
#[cfg(unix)], as before.