Skip to content

test(broker): wait for the foreign-tap worker before counting entries - #80

Merged
groupsky merged 1 commit into
mainfrom
test/foreign-tap-flaky
Oct 15, 2026
Merged

groupsky merged 1 commit into
mainfrom
test/foreign-tap-flaky

Conversation

@groupsky

Copy link
Copy Markdown
Contributor

What

Fixes the flaky TestForeignTap (#79). Only tests change; the broker was right.

  • TestForeignTap: it waits until the worker holds one tap at the gate (new helper waitGated). So exactly tapQueue taps fit in the queue, and the rest are dropped. Then it waits for every entry, checks the dropped count in the log, calls Close (it waits for the worker), and only then checks the upper limit. Each limit has its own failure message.
  • No hang on a bug: ForeignTap runs in a goroutine with a 5 s deadline, and the gate opens on every way out. Before, a ForeignTap that blocked made the test hang until the test timeout.
  • Test helpers: the fake audit log signals when an Append waits at the gate. The waitFor failure message now says "want at least".
  • The other broker tests are fine: counts checked right after an answer are safe, because the handler writes those entries and log lines before it returns. Work in the background is already waited for.

Tests

  • Stress (6 processes at once, -count=40 -cpu 1,2,4): the old test failed 48 of 5,040 runs pinned to 4 CPUs, and 20 of 11,520 without pinning. The new one: 0 of 5,760 pinned to 4 CPUs, also 0 on Go 1.26.
  • Broken copies of ForeignTap fail: a blocking send, a synchronous write, and a queue that is too big.
  • TestPanicBeforeTheCore also uses the changed gate helper: 0 failures in 2,880 runs.
  • A stress run of the whole suite (90 runs of every test, 30 of them pinned to 4 CPUs, as on a CI runner) found no other flaky test.

Review

One round with the security-reviewer agent: "ready to commit". The reviewer showed that the exact numbers (64 queued, 128 dropped, 66 entries) follow from the order of events in the code, not from timing, so they also hold at -cpu 1. Its own broken copies all failed with clear messages, and none hung. It also checked that the gate-helper change does not weaken TestPanicBeforeTheCore. Under load, the old test failed 23 of 1,200 runs, and the new one passed 3,600 of 3,600 on Go 1.27 and 1.26.

Closes #79

TestForeignTap counted the audit entries right after the "dropped
foreign_tap entries" log line, before the worker had written the queued
taps. So it failed now and then (48 of 5,040 runs on 4 CPUs).

It now waits until the worker holds one tap at the gate, so exactly
tapQueue taps are queued and the rest are dropped. It waits for every
entry, checks the dropped count in the log, closes the service, and only
then checks the upper limit. ForeignTap runs with a deadline, so a
ForeignTap that blocks fails the test instead of hanging it.
@groupsky
groupsky merged commit c010095 into main Oct 15, 2026
7 checks passed
@groupsky
groupsky deleted the test/foreign-tap-flaky branch October 15, 2026 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

broker: TestForeignTap is flaky; it counts the entries before the worker writes them

1 participant