Repository navigation
test(broker): wait for the foreign-tap worker before counting entries - #80
Merged
Merged
Conversation
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.
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
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 helperwaitGated). So exactlytapQueuetaps fit in the queue, and the rest are dropped. Then it waits for every entry, checks the dropped count in the log, callsClose(it waits for the worker), and only then checks the upper limit. Each limit has its own failure message.ForeignTapruns in a goroutine with a 5 s deadline, and the gate opens on every way out. Before, aForeignTapthat blocked made the test hang until the test timeout.Appendwaits at the gate. ThewaitForfailure message now says "want at least".Tests
-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.ForeignTapfail: a blocking send, a synchronous write, and a queue that is too big.TestPanicBeforeTheCorealso uses the changed gate helper: 0 failures in 2,880 runs.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 weakenTestPanicBeforeTheCore. 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