Skip to content

Commit f416d1e

Browse files
Planckbakaappleboy
andauthored
test(gin): resolve race conditions in integration tests (#4453)
- Implement TestRebuild404Handlers to verify 404 handler chain rebuilding when global middleware is added via Use() - Add waitForServerReady helper with exponential backoff to replace unreliable time.Sleep() calls in integration tests - Fix race conditions in TestRunEmpty, TestRunEmptyWithEnv, and TestRunWithPort by using proper server readiness checks - All tests now pass consistently with -race flag This addresses the empty test function and eliminates flaky test failures caused by insufficient wait times for server startup. Co-authored-by: Bo-Yi Wu <[email protected]>
1 parent 583db59 commit f416d1e

File tree

3 files changed

+65
-10
lines changed

3 files changed

+65
-10
lines changed

gin_integration_test.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,10 @@ func TestRunEmpty(t *testing.T) {
7070
router.GET("/example", func(c *Context) { c.String(http.StatusOK, "it worked") })
7171
assert.NoError(t, router.Run())
7272
}()
73-
// have to wait for the goroutine to start and run the server
74-
// otherwise the main thread will complete
75-
time.Sleep(5 * time.Millisecond)
73+
74+
// Wait for server to be ready with exponential backoff
75+
err := waitForServerReady("http://localhost:8080/example", 10)
76+
require.NoError(t, err, "server should start successfully")
7677

7778
require.Error(t, router.Run(":8080"))
7879
testRequest(t, "http://localhost:8080/example")
@@ -213,9 +214,10 @@ func TestRunEmptyWithEnv(t *testing.T) {
213214
router.GET("/example", func(c *Context) { c.String(http.StatusOK, "it worked") })
214215
assert.NoError(t, router.Run())
215216
}()
216-
// have to wait for the goroutine to start and run the server
217-
// otherwise the main thread will complete
218-
time.Sleep(5 * time.Millisecond)
217+
218+
// Wait for server to be ready with exponential backoff
219+
err := waitForServerReady("http://localhost:3123/example", 10)
220+
require.NoError(t, err, "server should start successfully")
219221

220222
require.Error(t, router.Run(":3123"))
221223
testRequest(t, "http://localhost:3123/example")
@@ -234,9 +236,10 @@ func TestRunWithPort(t *testing.T) {
234236
router.GET("/example", func(c *Context) { c.String(http.StatusOK, "it worked") })
235237
assert.NoError(t, router.Run(":5150"))
236238
}()
237-
// have to wait for the goroutine to start and run the server
238-
// otherwise the main thread will complete
239-
time.Sleep(5 * time.Millisecond)
239+
240+
// Wait for server to be ready with exponential backoff
241+
err := waitForServerReady("http://localhost:5150/example", 10)
242+
require.NoError(t, err, "server should start successfully")
240243

241244
require.Error(t, router.Run(":5150"))
242245
testRequest(t, "http://localhost:5150/example")

gin_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,29 @@ func TestNoMethodWithoutGlobalHandlers(t *testing.T) {
545545
}
546546

547547
func TestRebuild404Handlers(t *testing.T) {
548+
var middleware0 HandlerFunc = func(c *Context) {}
549+
var middleware1 HandlerFunc = func(c *Context) {}
550+
551+
router := New()
552+
553+
// Initially, allNoRoute should be nil
554+
assert.Nil(t, router.allNoRoute)
555+
556+
// Set NoRoute handlers
557+
router.NoRoute(middleware0)
558+
assert.Len(t, router.allNoRoute, 1)
559+
assert.Len(t, router.noRoute, 1)
560+
compareFunc(t, router.allNoRoute[0], middleware0)
561+
562+
// Add Use middleware should trigger rebuild404Handlers
563+
router.Use(middleware1)
564+
assert.Len(t, router.allNoRoute, 2)
565+
assert.Len(t, router.Handlers, 1)
566+
assert.Len(t, router.noRoute, 1)
567+
568+
// Global middleware should come first
569+
compareFunc(t, router.allNoRoute[0], middleware1)
570+
compareFunc(t, router.allNoRoute[1], middleware0)
548571
}
549572

550573
func TestNoMethodWithGlobalHandlers(t *testing.T) {

test_helpers.go

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@
44

55
package gin
66

7-
import "net/http"
7+
import (
8+
"fmt"
9+
"net/http"
10+
"time"
11+
)
812

913
// CreateTestContext returns a fresh Engine and a Context associated with it.
1014
// This is useful for tests that need to set up a new Gin engine instance
@@ -29,3 +33,28 @@ func CreateTestContextOnly(w http.ResponseWriter, r *Engine) (c *Context) {
2933
c.writermem.reset(w)
3034
return
3135
}
36+
37+
// waitForServerReady waits for a server to be ready by making HTTP requests
38+
// with exponential backoff. This is more reliable than time.Sleep() for testing.
39+
func waitForServerReady(url string, maxAttempts int) error {
40+
client := &http.Client{
41+
Timeout: 100 * time.Millisecond,
42+
}
43+
44+
for i := 0; i < maxAttempts; i++ {
45+
resp, err := client.Get(url)
46+
if err == nil {
47+
resp.Body.Close()
48+
return nil
49+
}
50+
51+
// Exponential backoff: 10ms, 20ms, 40ms, 80ms, 160ms...
52+
backoff := time.Duration(10*(1<<uint(i))) * time.Millisecond
53+
if backoff > 500*time.Millisecond {
54+
backoff = 500 * time.Millisecond
55+
}
56+
time.Sleep(backoff)
57+
}
58+
59+
return fmt.Errorf("server at %s did not become ready after %d attempts", url, maxAttempts)
60+
}

0 commit comments

Comments
 (0)