Test improvements (#828)

* stabilize http tests by waiting for mock server to start
drand · Oct 13, 2021 · 6fe4c2f · 6fe4c2f
1 parent 136181d
commit 6fe4c2f
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 21 deletions.
diff --git a/client/http/http.go b/client/http/http.go
@@ -24,6 +24,10 @@ import (
 var errClientClosed = fmt.Errorf("client closed")
 
 const defaultClientExec = "unknown"
+const defaultHTTTPTimeout = 60 * time.Second
+
+const httpWaitMaxCounter = 10
+const httpWaitInterval = 2 * time.Second
 
 // New creates a new client pointing to an HTTP endpoint
 func New(url string, chainHash []byte, transport nhttp.RoundTripper) (client.Client, error) {
@@ -45,7 +49,11 @@ func New(url string, chainHash []byte, transport nhttp.RoundTripper) (client.Cli
 		Agent:  agent,
 		done:   make(chan struct{}),
 	}
-	chainInfo, err := c.FetchChainInfo(chainHash)
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+	defer cancel()
+
+	chainInfo, err := c.FetchChainInfo(ctx, chainHash)
 	if err != nil {
 		return nil, err
 	}
@@ -110,10 +118,31 @@ func ForURLs(urls []string, chainHash []byte) []client.Client {
 	return clients
 }
 
+func Ping(ctx context.Context, root string) error {
+	url := fmt.Sprintf("%s/health", root)
+
+	ctx, cancel := context.WithCancel(ctx)
+	defer cancel()
+
+	req, err := nhttp.NewRequestWithContext(ctx, "GET", url, nil)
+	if err != nil {
+		return fmt.Errorf("creating request: %w", err)
+	}
+
+	response, err := nhttp.DefaultClient.Do(req)
+	if err != nil {
+		return fmt.Errorf("creating request: %w", err)
+	}
+
+	defer response.Body.Close()
+
+	return nil
+}
+
 // Instruments an HTTP client around a transport
 func instrumentClient(url string, transport nhttp.RoundTripper) *nhttp.Client {
 	hc := nhttp.Client{}
-	hc.Timeout = nhttp.DefaultClient.Timeout
+	hc.Timeout = defaultHTTTPTimeout
 	hc.Jar = nhttp.DefaultClient.Jar
 	hc.CheckRedirect = nhttp.DefaultClient.CheckRedirect
 	urlLabel := prometheus.Labels{"url": url}
@@ -144,6 +173,27 @@ func instrumentClient(url string, transport nhttp.RoundTripper) *nhttp.Client {
 	return &hc
 }
 
+func IsServerReady(addr string) error {
+	counter := 0
+
+	for {
+		ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+		defer cancel()
+
+		err := Ping(ctx, "http://"+addr)
+		if err == nil {
+			return nil
+		}
+
+		counter++
+		if counter == httpWaitMaxCounter {
+			return fmt.Errorf("timeout waiting http server to be ready")
+		}
+
+		time.Sleep(httpWaitInterval)
+	}
+}
+
 // httpClient implements Client through http requests to a Drand relay.
 type httpClient struct {
 	root      string
@@ -182,13 +232,13 @@ type httpInfoResponse struct {
 // FetchGroupInfo attempts to initialize an httpClient when
 // it does not know the full group parameters for a drand group. The chain hash
 // is the hash of the chain info.
-func (h *httpClient) FetchChainInfo(chainHash []byte) (*chain.Info, error) {
+func (h *httpClient) FetchChainInfo(ctx context.Context, chainHash []byte) (*chain.Info, error) {
 	if h.chainInfo != nil {
 		return h.chainInfo, nil
 	}
 
 	resC := make(chan httpInfoResponse, 1)
-	ctx, cancel := context.WithCancel(context.Background())
+	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
 	go func() {

diff --git a/client/http/http_test.go b/client/http/http_test.go
@@ -15,6 +15,11 @@ func TestHTTPClient(t *testing.T) {
 	addr, chainInfo, cancel, _ := mock.NewMockHTTPPublicServer(t, true)
 	defer cancel()
 
+	err := IsServerReady(addr)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	httpClient, err := New("http://"+addr, chainInfo.Hash(), http.DefaultTransport)
 	if err != nil {
 		t.Fatal(err)
@@ -47,6 +52,11 @@ func TestHTTPGetLatest(t *testing.T) {
 	addr, chainInfo, cancel, _ := mock.NewMockHTTPPublicServer(t, false)
 	defer cancel()
 
+	err := IsServerReady(addr)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	httpClient, err := New("http://"+addr, chainInfo.Hash(), http.DefaultTransport)
 	if err != nil {
 		t.Fatal(err)
@@ -76,6 +86,11 @@ func TestForURLsCreation(t *testing.T) {
 	addr, chainInfo, cancel, _ := mock.NewMockHTTPPublicServer(t, false)
 	defer cancel()
 
+	err := IsServerReady(addr)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	clients := ForURLs([]string{"http://invalid.domain/", "http://" + addr}, chainInfo.Hash())
 	if len(clients) != 2 {
 		t.Fatal("expect both urls returned")
@@ -88,6 +103,11 @@ func TestHTTPWatch(t *testing.T) {
 	addr, chainInfo, cancel, _ := mock.NewMockHTTPPublicServer(t, false)
 	defer cancel()
 
+	err := IsServerReady(addr)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	httpClient, err := New("http://"+addr, chainInfo.Hash(), http.DefaultTransport)
 	if err != nil {
 		t.Fatal(err)
@@ -112,6 +132,11 @@ func TestHTTPClientClose(t *testing.T) {
 	addr, chainInfo, cancel, _ := mock.NewMockHTTPPublicServer(t, false)
 	defer cancel()
 
+	err := IsServerReady(addr)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	httpClient, err := New("http://"+addr, chainInfo.Hash(), http.DefaultTransport)
 	if err != nil {
 		t.Fatal(err)

diff --git a/client/test/http/mock/httpserver.go b/client/test/http/mock/httpserver.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"net"
 	"net/http"
-	"sync"
 	"testing"
 	"time"
 
@@ -46,24 +45,15 @@ func NewMockHTTPPublicServer(t *testing.T, badSecondRound bool) (string, *chain.
 		t.Fatal("could not use server after 3 attempts.")
 	}
 
-	wg := sync.WaitGroup{}
-	wg.Add(1)
-	addr := ""
+	listener, err := net.Listen("tcp", ":0")
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	httpServer := http.Server{Handler: handler}
-	go func() {
-		listener, err := net.Listen("tcp", ":0")
-		if err != nil {
-			t.Fatal(err)
-		}
-		addr = listener.Addr().String()
-		wg.Done()
-
-		httpServer.Serve(listener)
-	}()
-	wg.Wait()
+	go httpServer.Serve(listener)
 
-	return addr, chainInfo, func() {
+	return listener.Addr().String(), chainInfo, func() {
 		httpServer.Shutdown(context.Background())
 		cancel()
 	}, server.(mock.MockService).EmitRand

diff --git a/core/drand_test.go b/core/drand_test.go
@@ -810,10 +810,12 @@ func TestDrandPublicStreamProxy(t *testing.T) {
 	defer cancel()
 
 	// get last round first
+	t.Logf("Getting round %d", 0)
 	resp, err := client.Get(ctx, 0)
 	require.NoError(t, err)
 
 	//  run streaming and expect responses
+	t.Log("Watching new rounds generated")
 	rc := client.Watch(ctx)
 
 	// expect first round now since node already has it
@@ -822,6 +824,8 @@ func TestDrandPublicStreamProxy(t *testing.T) {
 	if !ok {
 		panic("expected beacon")
 	}
+
+	t.Logf("Round received %d", beacon.Round())
 	require.Equal(t, beacon.Round(), resp.Round()+1)
 
 	nTry := 4
@@ -834,6 +838,8 @@ func TestDrandPublicStreamProxy(t *testing.T) {
 		beacon, ok = <-rc
 
 		require.True(t, ok)
+
+		t.Logf("Round received %d", beacon.Round())
 		require.Equal(t, round, beacon.Round())
 	}
 }
diff --git a/http/server_test.go b/http/server_test.go
@@ -8,6 +8,8 @@ import (
 	"testing"
 	"time"
 
+	nhttp "github.com/drand/drand/client/http"
+
 	"github.com/drand/drand/client"
 	"github.com/drand/drand/client/grpc"
 	"github.com/drand/drand/protobuf/drand"
@@ -46,7 +48,8 @@ func TestHTTPRelay(t *testing.T) {
 	server := http.Server{Handler: handler}
 	go func() { _ = server.Serve(listener) }()
 	defer func() { _ = server.Shutdown(ctx) }()
-	time.Sleep(100 * time.Millisecond)
+
+	nhttp.IsServerReady(listener.Addr().String())
 
 	getChain := fmt.Sprintf("http://%s/info", listener.Addr().String())
 	resp, err := http.Get(getChain)
@@ -124,6 +127,8 @@ func TestHTTPWaiting(t *testing.T) {
 	go func() { _ = server.Serve(listener) }()
 	defer func() { _ = server.Shutdown(ctx) }()
 
+	nhttp.IsServerReady(listener.Addr().String())
+
 	// The first request will trigger background watch. 1 get (1969)
 	next, err := http.Get(fmt.Sprintf("http://%s/public/0", listener.Addr().String()))
 	if err != nil {
@@ -184,6 +189,8 @@ func TestHTTPWatchFuture(t *testing.T) {
 	go func() { _ = server.Serve(listener) }()
 	defer func() { _ = server.Shutdown(ctx) }()
 
+	nhttp.IsServerReady(listener.Addr().String())
+
 	// watching sets latest round, future rounds should become inaccessible.
 	u := fmt.Sprintf("http://%s/public/2000", listener.Addr().String())
 	resp, err := http.Get(u)
@@ -214,6 +221,8 @@ func TestHTTPHealth(t *testing.T) {
 	go func() { _ = server.Serve(listener) }()
 	defer func() { _ = server.Shutdown(ctx) }()
 
+	nhttp.IsServerReady(listener.Addr().String())
+
 	resp, _ := http.Get(fmt.Sprintf("http://%s/health", listener.Addr().String()))
 	defer func() { _ = resp.Body.Close() }()
 	if resp.StatusCode == http.StatusOK {