From d2e155bb1c89da8cfce2be13800ad615bafcd753 Mon Sep 17 00:00:00 2001
From: Milan Pavlik <milan@gitpod.io>
Date: Wed, 20 Apr 2022 17:48:41 +0000
Subject: [PATCH] [baseserver] Helper for starting server in tests

---
 .../common-go/baseserver/server_test.go       | 26 +++----------------
 components/common-go/baseserver/testing.go    | 15 ++++++++++-
 .../public-api-server/integration_test.go     |  9 ++-----
 3 files changed, 20 insertions(+), 30 deletions(-)

diff --git a/components/common-go/baseserver/server_test.go b/components/common-go/baseserver/server_test.go
index 0e2dcc11edb99b..5ab2bcd485532d 100644
--- a/components/common-go/baseserver/server_test.go
+++ b/components/common-go/baseserver/server_test.go
@@ -11,19 +11,14 @@ import (
 	"github.com/stretchr/testify/require"
 	"net/http"
 	"testing"
-	"time"
 )
 
 func TestServer_StartStop(t *testing.T) {
 	// We don't use the helper NewForTests, because we want to control stopping ourselves.
 	srv, err := baseserver.New("server_test", baseserver.WithHTTPPort(8765), baseserver.WithGRPCPort(8766))
 	require.NoError(t, err)
+	baseserver.StartServerForTests(t, srv)
 
-	go func() {
-		require.NoError(t, srv.ListenAndServe())
-	}()
-
-	baseserver.WaitForServerToBeReachable(t, srv, 3*time.Second)
 	require.Equal(t, "http://localhost:8765", srv.HTTPAddress())
 	require.Equal(t, "localhost:8766", srv.GRPCAddress())
 	require.NoError(t, srv.Close())
@@ -31,12 +26,7 @@ func TestServer_StartStop(t *testing.T) {
 
 func TestServer_ServesReady(t *testing.T) {
 	srv := baseserver.NewForTests(t)
-
-	go func(t *testing.T) {
-		require.NoError(t, srv.ListenAndServe())
-	}(t)
-
-	baseserver.WaitForServerToBeReachable(t, srv, 3*time.Second)
+	baseserver.StartServerForTests(t, srv)
 
 	readyURL := fmt.Sprintf("%s/ready", srv.HTTPAddress())
 	resp, err := http.Get(readyURL)
@@ -47,11 +37,7 @@ func TestServer_ServesReady(t *testing.T) {
 func TestServer_ServesMetricsEndpointWithDefaultConfig(t *testing.T) {
 	srv := baseserver.NewForTests(t)
 
-	go func(t *testing.T) {
-		require.NoError(t, srv.ListenAndServe())
-	}(t)
-
-	baseserver.WaitForServerToBeReachable(t, srv, 3*time.Second)
+	baseserver.StartServerForTests(t, srv)
 
 	readyUR := fmt.Sprintf("%s/metrics", srv.HTTPAddress())
 	resp, err := http.Get(readyUR)
@@ -65,11 +51,7 @@ func TestServer_ServesMetricsEndpointWithCustomMetricsConfig(t *testing.T) {
 		baseserver.WithMetricsRegistry(registry),
 	)
 
-	go func(t *testing.T) {
-		require.NoError(t, srv.ListenAndServe())
-	}(t)
-
-	baseserver.WaitForServerToBeReachable(t, srv, 3*time.Second)
+	baseserver.StartServerForTests(t, srv)
 
 	readyUR := fmt.Sprintf("%s/metrics", srv.HTTPAddress())
 	resp, err := http.Get(readyUR)
diff --git a/components/common-go/baseserver/testing.go b/components/common-go/baseserver/testing.go
index 96a3dd1c6dad78..54d04598f5ca3f 100644
--- a/components/common-go/baseserver/testing.go
+++ b/components/common-go/baseserver/testing.go
@@ -34,7 +34,20 @@ func NewForTests(t *testing.T, opts ...Option) *Server {
 	return srv
 }
 
-func WaitForServerToBeReachable(t *testing.T, srv *Server, timeout time.Duration) {
+// StartServerForTests starts the server for test purposes.
+// This is a helper which also ensures the server is reachable before returning.
+func StartServerForTests(t *testing.T, srv *Server) {
+	t.Helper()
+
+	go func() {
+		err := srv.ListenAndServe()
+		require.NoError(t, err)
+	}()
+
+	waitForServerToBeReachable(t, srv, 3*time.Second)
+}
+
+func waitForServerToBeReachable(t *testing.T, srv *Server, timeout time.Duration) {
 	ctx, cancel := context.WithTimeout(context.Background(), timeout)
 	defer cancel()
 
diff --git a/components/public-api-server/integration_test.go b/components/public-api-server/integration_test.go
index b144a093a23d32..507dc8c55e11f3 100644
--- a/components/public-api-server/integration_test.go
+++ b/components/public-api-server/integration_test.go
@@ -14,19 +14,14 @@ import (
 	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/status"
 	"testing"
-	"time"
 )
 
 func TestPublicAPIServer(t *testing.T) {
 	ctx := context.Background()
 	srv := baseserver.NewForTests(t)
-	require.NoError(t, register(srv))
-
-	go func() {
-		require.NoError(t, srv.ListenAndServe())
-	}()
 
-	baseserver.WaitForServerToBeReachable(t, srv, 1*time.Second)
+	require.NoError(t, register(srv))
+	baseserver.StartServerForTests(t, srv)
 
 	conn, err := grpc.Dial(srv.GRPCAddress(), grpc.WithTransportCredentials(insecure.NewCredentials()))
 	require.NoError(t, err)