Skip to content

Commit 40b7277

Browse files
JAORMXclaude
andauthored
Fix flaky vMCP health endpoint tests (#2415)
The health endpoint tests were failing intermittently in CI due to port conflicts when running parallel tests. All test instances tried to bind to the default port 4483, causing "address already in use" errors. Changes: - Add Ready() channel to vMCP server to signal when listener is created - Use networking.FindAvailable() to assign unique random ports to tests - Improve test synchronization by waiting on Ready() channel - Add error channel to catch and report startup failures The tests now run reliably with 20 parallel iterations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
1 parent 05b00e5 commit 40b7277

File tree

2 files changed

+43
-10
lines changed

2 files changed

+43
-10
lines changed

pkg/vmcp/server/health_test.go

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/stretchr/testify/require"
1313
"go.uber.org/mock/gomock"
1414

15+
"github.com/stacklok/toolhive/pkg/networking"
1516
"github.com/stacklok/toolhive/pkg/vmcp"
1617
"github.com/stacklok/toolhive/pkg/vmcp/aggregator"
1718
"github.com/stacklok/toolhive/pkg/vmcp/mocks"
@@ -30,11 +31,15 @@ func createTestServer(t *testing.T) *server.Server {
3031
mockBackendClient := mocks.NewMockBackendClient(ctrl)
3132
rt := router.NewDefaultRouter()
3233

34+
// Find an available port for parallel test execution
35+
port := networking.FindAvailable()
36+
require.NotZero(t, port, "Failed to find available port")
37+
3338
srv := server.New(&server.Config{
3439
Name: "test-vmcp",
3540
Version: "1.0.0",
3641
Host: "127.0.0.1",
37-
Port: 0, // Random port for parallel tests
42+
Port: port,
3843
}, rt, mockBackendClient)
3944

4045
// Register minimal capabilities
@@ -54,17 +59,28 @@ func createTestServer(t *testing.T) *server.Server {
5459
require.NoError(t, err)
5560

5661
// Start server in background
57-
ctx, cancel := context.WithCancel(context.Background())
58-
go func() { _ = srv.Start(ctx) }()
62+
ctx, cancel := context.WithCancel(t.Context())
63+
t.Cleanup(cancel)
5964

60-
// Wait for server to start
61-
time.Sleep(100 * time.Millisecond)
65+
errCh := make(chan error, 1)
66+
go func() {
67+
if err := srv.Start(ctx); err != nil {
68+
errCh <- err
69+
}
70+
}()
71+
72+
// Wait for server to be ready (with timeout)
73+
select {
74+
case <-srv.Ready():
75+
// Server is ready to accept connections
76+
case err := <-errCh:
77+
t.Fatalf("Server failed to start: %v", err)
78+
case <-time.After(5 * time.Second):
79+
t.Fatalf("Server did not become ready within 5s (address: %s)", srv.Address())
80+
}
6281

63-
// Cleanup
64-
t.Cleanup(func() {
65-
cancel()
66-
time.Sleep(50 * time.Millisecond)
67-
})
82+
// Give the HTTP server a moment to start accepting connections
83+
time.Sleep(10 * time.Millisecond)
6884

6985
return srv
7086
}

pkg/vmcp/server/server.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,11 @@ type Server struct {
103103
// The mark3labs SDK calls our sessionIDAdapter, which delegates to this manager.
104104
// The SDK does NOT manage sessions itself - it only provides the interface.
105105
sessionManager *session.Manager
106+
107+
// Ready channel signals when the server is ready to accept connections.
108+
// Closed once the listener is created and serving.
109+
ready chan struct{}
110+
readyOnce sync.Once
106111
}
107112

108113
// New creates a new Virtual MCP Server instance.
@@ -148,6 +153,7 @@ func New(
148153
router: rt,
149154
backendClient: backendClient,
150155
sessionManager: sessionManager,
156+
ready: make(chan struct{}),
151157
}
152158
}
153159

@@ -262,6 +268,11 @@ func (s *Server) Start(ctx context.Context) error {
262268
}
263269
}()
264270

271+
// Signal that the server is ready (listener created and serving started)
272+
s.readyOnce.Do(func() {
273+
close(s.ready)
274+
})
275+
265276
// Wait for either context cancellation or server error
266277
select {
267278
case <-ctx.Done():
@@ -600,3 +611,9 @@ func (*Server) handleHealth(w http.ResponseWriter, _ *http.Request) {
600611
func (s *Server) SessionManager() *session.Manager {
601612
return s.sessionManager
602613
}
614+
615+
// Ready returns a channel that is closed when the server is ready to accept connections.
616+
// This is useful for testing and synchronization.
617+
func (s *Server) Ready() <-chan struct{} {
618+
return s.ready
619+
}

0 commit comments

Comments
 (0)