fix: resolve flaky TestAgent_Session_TTY_MOTD_Update (#23375)

The 5ms ServiceBannerRefreshInterval caused excessive DRPC
connection churn (200 calls/s) under the race detector, creating
heavy mutex contention on FakeAgentAPI and significant CPU overhead.
This made the test timing-sensitive in ways that manifested as
session.Wait() hangs, killing the test binary via timeout.

Three changes:
- Increase refresh interval from 5ms to testutil.IntervalFast (25ms),
  reducing DRPC connection churn and mutex contention by 5x.
- Replace bare <-ready receives with testutil.TryReceive so the test
  fails with context expiry instead of hanging indefinitely.
- Add a timeout to session.Wait() in testSessionOutput to prevent any
  SSH session hang from killing the entire test binary.

Fixes coder/internal#1417
This commit is contained in:
Mathias Fredriksson
2026-03-20 19:33:10 +00:00
committed by GitHub
parent f3b91b7f11
commit c60a3568d7
+21 -9
View File
@@ -713,15 +713,15 @@ func TestAgent_Session_TTY_MOTD_Update(t *testing.T) {
},
}
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
setSBInterval := func(_ *agenttest.Client, opts *agent.Options) {
opts.ServiceBannerRefreshInterval = 5 * time.Millisecond
opts.ServiceBannerRefreshInterval = testutil.IntervalFast
}
//nolint:dogsled // Allow the blank identifiers.
conn, client, _, _, _ := setupAgent(t, agentsdk.Manifest{}, 0, setSBInterval)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
//nolint:paralleltest // These tests need to swap the banner func.
for _, port := range sshPorts {
sshClient, err := conn.SSHClientOnPort(ctx, port)
@@ -733,7 +733,10 @@ func TestAgent_Session_TTY_MOTD_Update(t *testing.T) {
for i, test := range tests {
t.Run(fmt.Sprintf("(:%d)/%d", port, i), func(t *testing.T) {
// Set new banner func and wait for the agent to call it to update the
// banner.
// banner. We wait for two calls to ensure the value has been stored:
// the second call can only begin after the first iteration of
// fetchServiceBannerLoop completes (call + store), so after
// receiving two signals at least one store has happened.
ready := make(chan struct{}, 2)
client.SetAnnouncementBannersFunc(func() ([]codersdk.BannerConfig, error) {
select {
@@ -742,8 +745,8 @@ func TestAgent_Session_TTY_MOTD_Update(t *testing.T) {
}
return []codersdk.BannerConfig{test.banner}, nil
})
<-ready
<-ready // Wait for two updates to ensure the value has propagated.
testutil.TryReceive(ctx, t, ready)
testutil.TryReceive(ctx, t, ready)
session, err := sshClient.NewSession()
require.NoError(t, err)
@@ -3550,8 +3553,17 @@ func testSessionOutput(t *testing.T, session *ssh.Session, expected, unexpected
require.NoError(t, err)
ptty.WriteLine("exit 0")
err = session.Wait()
require.NoError(t, err)
waitErr := make(chan error, 1)
go func() {
waitErr <- session.Wait()
}()
select {
case err = <-waitErr:
require.NoError(t, err)
case <-time.After(testutil.WaitLong):
require.Fail(t, "timed out waiting for session to exit")
}
for _, unexpected := range unexpected {
require.NotContains(t, stdout.String(), unexpected, "should not show output")