Skip to content

Commit a814579

Browse files
go/mysql: stop TestStaticConfigHUP panicking inside EventuallyWithT
The previous deflake commit (e3cd779) ported the EventuallyWithT callback verbatim from upstream's PR #19388, which uses `require.X(c, ...)`. That works on upstream's testify v1.11+, but this branch is pinned to testify v1.9, where `CollectT.FailNow` is implemented as `panic("Assertion failed")` and `EventuallyWithT` doesn't recover from it — so the first failed poll crashes the goroutine. The job log showed exactly that: panic: Assertion failed testify/assert.(*CollectT).FailNow ... EventuallyWithT.func1 ... FAIL vitess.io/vitess/go/mysql Replace `require.X(c, ...)` with `assert.X(c, ...)` (which just flags the CollectT instead of panicking) and guard the `entries[0]` indexing on `assert.NotEmpty`, otherwise a `nil[0]` slice access escapes the same way. Hoisted the polling loop into a `waitForReload` helper since both hupTest and hupTestWithRotation now use the same body. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Arthur Schreiber <arthur@planetscale.com> (cherry picked from commit 248182c)
1 parent 196e56e commit a814579

1 file changed

Lines changed: 21 additions & 6 deletions

File tree

go/mysql/auth_server_static_test.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,7 @@ func hupTest(t *testing.T, aStatic *AuthServerStatic, tmpFile *os.File, oldStr,
160160
syscall.Kill(syscall.Getpid(), syscall.SIGHUP)
161161

162162
// wait for signal handler
163-
require.EventuallyWithT(t, func(c *assert.CollectT) {
164-
require.Nil(c, aStatic.getEntries()[oldStr], "Should not have old %s after config reload", oldStr)
165-
require.Equal(c, newStr, aStatic.getEntries()[newStr][0].Password, "%s's Password should be '%s'", newStr, newStr)
166-
}, 30*time.Second, 10*time.Millisecond, "config should be reloaded with new file after rotation")
163+
waitForReload(t, aStatic, oldStr, newStr)
167164
}
168165

169166
func hupTestWithRotation(t *testing.T, aStatic *AuthServerStatic, tmpFile *os.File, oldStr, newStr string) {
@@ -172,9 +169,27 @@ func hupTestWithRotation(t *testing.T, aStatic *AuthServerStatic, tmpFile *os.Fi
172169
t.Fatalf("couldn't overwrite temp file: %v", err)
173170
}
174171

172+
waitForReload(t, aStatic, oldStr, newStr)
173+
}
174+
175+
// waitForReload polls aStatic until the auth file reload has dropped oldStr
176+
// and installed newStr.
177+
//
178+
// We use `assert.X(c, ...)` (not `require.X`) inside the callback because
179+
// testify v1.9 on this branch implements `CollectT.FailNow` as
180+
// `panic("Assertion failed")`, and `EventuallyWithT` doesn't recover from
181+
// that — a failed poll would crash the test instead of being retried. We
182+
// also gate the `[0]` indexing on `NotEmpty` for the same reason: a panic
183+
// inside the callback (e.g. `nil[0]`) escapes the goroutine.
184+
func waitForReload(t *testing.T, aStatic *AuthServerStatic, oldStr, newStr string) {
185+
t.Helper()
175186
require.EventuallyWithT(t, func(c *assert.CollectT) {
176-
require.Nil(c, aStatic.getEntries()[oldStr], "Should not have old %s after config reload", oldStr)
177-
require.Equal(c, newStr, aStatic.getEntries()[newStr][0].Password, "%s's Password should be '%s'", newStr, newStr)
187+
assert.Nil(c, aStatic.getEntries()[oldStr], "Should not have old %s after config reload", oldStr)
188+
entries := aStatic.getEntries()[newStr]
189+
if !assert.NotEmpty(c, entries, "Should have new %s entries after config reload", newStr) {
190+
return
191+
}
192+
assert.Equal(c, newStr, entries[0].Password, "%s's Password should be '%s'", newStr, newStr)
178193
}, 30*time.Second, 10*time.Millisecond, "config should be reloaded with new file after rotation")
179194
}
180195

0 commit comments

Comments
 (0)