test: assert Serve error synchronously after shutdown in TestServe (#1687)

TestServe started the server in a goroutine that captured Serve's
return value, slept two seconds, then called assert.NoError on it. The
test body returned immediately after Shutdown without ever joining that
goroutine, so the assertion ran after the test had already been recorded
as passed and could never affect its result (a dead assertion). It was
also wrong: closing the listener in Shutdown makes Serve return a
net.ErrClosed error, not nil.

Capture Serve's error over a buffered channel and assert it synchronously
in the test body after Shutdown, checking it wraps net.ErrClosed so the
check actually runs within the test's lifetime and catches an unexpected
Serve failure.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Co-authored-by: Alex Jones <1235925+AlexsJones@users.noreply.github.com>

pkg/server/server_test.go

@@ -31,10 +31,9 @@ func TestServe(t *testing.T) {
 		EnableHttp: false,
 	}
 
+	serveErr := make(chan error, 1)
 	go func() {
-		err := s.Serve()
-		time.Sleep(time.Second * 2)
-		assert.NoError(t, err, "Serve should not return an error")
+		serveErr <- s.Serve()
 	}()
 
 	// Wait until the server is ready to accept connections
@@ -60,6 +59,16 @@ func TestServe(t *testing.T) {
 	// Cleanup
 	err = s.Shutdown()
 	assert.NoError(t, err, "Shutdown should not return an error")
+
+	// Shutdown closes the listener, which unblocks Serve. Assert synchronously
+	// that Serve returned only because of that shutdown and not for another
+	// reason.
+	select {
+	case err := <-serveErr:
+		assert.ErrorIs(t, err, net.ErrClosed, "Serve should return only due to the listener being closed on shutdown")
+	case <-time.After(5 * time.Second):
+		t.Error("Serve did not return after shutdown")
+	}
 }
 
 // TestMCPServerCreation tests the creation of an MCP server