From 52775ef1722b40cdc0a2c6cbf45f08abf63e40f2 Mon Sep 17 00:00:00 2001 From: George K Date: Wed, 8 Jul 2026 13:01:29 -0700 Subject: [PATCH] test(coderd/externalauth): fix RevokeTokenRFC_Timeout flake (#27082) Under CI load the request's 10ms revoke timeout could expire before the request reached the FakeIDP revoke handler. The handler never ran, so the test's wait for it to finish blocked until the 25s test context expired instead of passing quickly. Raise `RevokeTimeout` to 100ms so the request has ~10x more headroom to reach the handler under load. After RevokeToken returns, check a `handlerStarted` signal before asserting: this anchors the `DeadlineExceeded` assertion to a request that was actually in flight, and turns any residual scheduling race into a fast, labeled failure instead of a hang. Unblock the handler on the early-exit path with a `t.Cleanup`. It must be registered after the FakeIDP setup so LIFO runs it before the server's `Close()`; otherwise a handler that dispatched late would block `Close()` and hang teardown until the test timeout. Drop the previous `time.Sleep` watchdog and the handler-done channel, since the FakeIDP server's `Close()` already joins the in-flight handler. Refs: https://linear.app/codercom/issue/PLAT-317 --- coderd/externalauth/externalauth_test.go | 44 +++++++++++++----------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/coderd/externalauth/externalauth_test.go b/coderd/externalauth/externalauth_test.go index 4221e73309..548d2a33b6 100644 --- a/coderd/externalauth/externalauth_test.go +++ b/coderd/externalauth/externalauth_test.go @@ -1083,41 +1083,45 @@ func TestRevokeToken(t *testing.T) { t.Run("RevokeTokenRFC_Timeout", func(t *testing.T) { t.Parallel() + handlerStarted := make(chan bool, 1) revokeExited := make(chan bool, 1) - testTimeout := make(chan bool, 1) - handlerDone := make(chan bool) - - go func() { - time.Sleep(5 * time.Second) - testTimeout <- true - }() fake, config, link := setupOauth2Test(t, testConfig{ FakeIDPOpts: []oidctest.FakeIDPOpt{ oidctest.WithRevokeTokenRFC(func() (int, error) { - defer func() { - handlerDone <- true - }() - - select { - case <-testTimeout: - t.Error("test timeout reached before context timeout") - return http.StatusOK, nil - case <-revokeExited: - return http.StatusOK, nil - } + handlerStarted <- true + <-revokeExited + return http.StatusOK, nil }), oidctest.WithServing(), }, }) + // Always unblock the handler so it can return. Must be + // registered after setupOauth2Test so LIFO runs it first. + t.Cleanup(func() { + select { + case revokeExited <- true: + default: + } + }) + ctx := oidc.ClientContext(testutil.Context(t, testutil.WaitLong), fake.HTTPClient(nil)) - config.RevokeTimeout = time.Millisecond * 10 + // A short timeout forces the request's deadline to fire while + // the handler is blocked in-flight, exercising the revoke + // timeout path. + config.RevokeTimeout = 100 * time.Millisecond revoked, err := config.RevokeToken(ctx, link) + // Make sure request has reached the handler before asserting. + // NOTE: if this flakes again, increase config.RevokeTimeout. + select { + case <-handlerStarted: + default: + t.Fatal("RevokeToken returned before revoke handler started") + } revokeExited <- true require.ErrorIs(t, err, context.DeadlineExceeded) require.False(t, revoked) - _ = testutil.RequireReceive(ctx, t, handlerDone) }) t.Run("RevokeTokenGitHub_OK", func(t *testing.T) {