You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
relay: a close-aborts-read test stays green under the delay it exists to catch #982
a_close_aborts_an_in_flight_read_without_a_frame (crates/nostream-relay/tests/handlers.rs) stays green under the same 200 ms Unsubscribe delay that makes a_close_during_the_invite_mint_leaves_the_event_but_not_the_read fail — but not because it is exercising the abort.
Why it can pass for the wrong reason
Recon for #963 measured it: a one-line delay injected into the Unsubscribe dispatch leaves this test passing. Its two accepted frames can be the second REQ's own read, because that REQ's begin_read aborts the previous read (connection.rs:265-270), and its only CLOSE-sensitive assertion runs while the read is still gated.
So the test proves something — that a re-REQ supersedes a read — but it does not prove what its name says when the CLOSE is delayed, and the injected delay is exactly the case that matters.
What a fix needs
An assertion that can only hold if the CLOSE's abort ran: gate the first read, send the CLOSE, wait until the read is observably aborted (!read_in_flight), then release and assert silence — the shape #963 adopted for its own test, which removes the sleep instead of bounding it.
Two related gaps in the same file, from the same recon: the sibling also carries two 50 ms sleeps, and the census has no entry for either test, so renaming or moving them breaks no parity obligation.
Measured on rewrite/main @ 2b2287f while fixing #963 (PR #980 branch factory/014-close-mint-sync). Not a live defect — a test that cannot fail for its stated reason.
Additional hypothesis, and a warning about the premise. Both came up while closing #963; recorded here rather than folded into it.
1. The premise predates the code it tests.#978 made CLOSE a prefix: handlers::begin runs the whole handler on the connection loop before the next frame is read. This test was last measured before that change, and the path it drives is not the path it drove then — so the first step is to re-measure it at the current head, not to fix it.
2. The test may pass for the wrong reason, not merely under the wrong timing. Its two accepted frames can be the second REQ's own read rather than the first subscription's: if the assertion counts frames without attributing them to a subscription id and a read generation, a replacement subscription's frames satisfy it. A test that can pass for the wrong reason is the class #963 belongs to — the failure is invisible because the test is green for a reason nobody intended.
What would settle it (the same recipe #963 used): inject a delay into the CLOSE dispatch and require the test to go red. If it stays green under an injection that provably delays the abort, it is not testing the abort — and the fix is to attribute each frame to the subscription whose read it came from, not to add a longer sleep.
Activity
chappie-daemon commentedon Oct 25, 2026
Additional hypothesis, and a warning about the premise. Both came up while closing #963; recorded here rather than folded into it.
1. The premise predates the code it tests.
#978madeCLOSEa prefix:handlers::beginruns the whole handler on the connection loop before the next frame is read. This test was last measured before that change, and the path it drives is not the path it drove then — so the first step is to re-measure it at the current head, not to fix it.2. The test may pass for the wrong reason, not merely under the wrong timing. Its two accepted frames can be the second
REQ's own read rather than the first subscription's: if the assertion counts frames without attributing them to a subscription id and a read generation, a replacement subscription's frames satisfy it. A test that can pass for the wrong reason is the class #963 belongs to — the failure is invisible because the test is green for a reason nobody intended.What would settle it (the same recipe #963 used): inject a delay into the
CLOSEdispatch and require the test to go red. If it stays green under an injection that provably delays the abort, it is not testing the abort — and the fix is to attribute each frame to the subscription whose read it came from, not to add a longer sleep.