Stop a slow host turning a login-guard test into a segfault (closes #186)
All checks were successful
check / check (push) Successful in 2m53s

CI run 232 failed
TestLoginGuard_SaturatedSemaphoreRefusesRatherThanQueueing and then
took the whole internal/middleware test binary down with a SIGSEGV, on
a commit whose own gates were green. acquire returns (nil, false) on
every refusal path, the assertion on ok was non-fatal, and the next
line called the nil release. Two defects sit behind that, and the
second one is not confined to the test.

acquire selected over a slot send and an already-armed wait timer. Go
picks among ready cases uniformly at random, so a process descheduled
for longer than the wait sheds a request with slots standing free —
under load, which is exactly when shedding a login is least
defensible. A non-blocking preamble now takes a free slot before any
timer is armed, the same shape lifecycle.waitDone uses to settle its
own both-ready race. It cannot let a late arrival barge past a queued
waiter: a waiter can only be parked on a full buffer, and a release
refills that buffer from the head of the send queue under the channel
lock, so the preamble's send fails whenever anyone is waiting. Placing
it ahead of the queue admission also stops a request that never waits
from occupying a waiter's place.

The test's third acquire is therefore settled by construction rather
than by the wait being long enough, and
TestLoginGuard_FreeSlotBeatsAnExpiredWait pins that: 1000 passes with
a wait already elapsed on arrival, the worst case scheduling can
produce. Without the preamble it fails on pass 1.

Assertions whose value is dereferenced afterwards are require, not
assert. A sweep of every test file found one sibling of the same
shape: webhook_db_manager_test.go checked a slice length non-fatally
and indexed it on the next line, so the regression it guards would
have surfaced as an index-out-of-range panic through
internal/database rather than as a failing test.

TestLoginGuard_SemaphoreBoundsConcurrentVerifications used a 10 ms
sleep to make two workers overlap and asserted the observed maximum
was exactly two. A sleep only makes overlap likely; on a host that can
deschedule a goroutine for longer than the sleep the workers serialise
and the maximum comes back as one. The slot holders now rendezvous, so
the overlap the assertion needs is a fact rather than a race won, and
the test no longer sleeps at all.
This commit is contained in:
2026-08-18 01:41:36 +00:00
parent b573959a26
commit 6ea30519dd
3 changed files with 122 additions and 5 deletions

View File

@@ -339,7 +339,12 @@ func TestWebhookDBManager_MultipleWebhooks(t *testing.T) {
var events []database.Event
require.NoError(t, db2.Find(&events).Error)
assert.Len(t, events, 1)
// require, not assert: this is exactly the regression the test
// guards, so the empty slice is the expected failure, and a
// non-fatal length check would index into it on the next line and
// panic the whole package test binary instead of failing here.
require.Len(t, events, 1)
assert.Equal(t, "PUT", events[0].Method)
}