Make agent tests more robust (#632)

* Make agent tests more robust
* Add String() to test logger

cmd/agent/app/agent.go

@@ -18,6 +18,7 @@ import (
 	"io"
 	"net"
 	"net/http"
+	"sync/atomic"
 
 	"go.uber.org/zap"
 
@@ -28,6 +29,7 @@ import (
 type Agent struct {
 	processors []processors.Processor
 	httpServer *http.Server
+	httpAddr   atomic.Value // string, set once agent starts listening
 	logger     *zap.Logger
 	closer     io.Closer
 }
@@ -38,11 +40,13 @@ func NewAgent(
 	httpServer *http.Server,
 	logger *zap.Logger,
 ) *Agent {
-	return &Agent{
+	a := &Agent{
 		processors: processors,
 		httpServer: httpServer,
 		logger:     logger,
 	}
+	a.httpAddr.Store("")
+	return a
 }
 
 // Run runs all of agent UDP and HTTP servers in separate go-routines.
@@ -53,18 +57,25 @@ func (a *Agent) Run() error {
 	if err != nil {
 		return err
 	}
+	a.httpAddr.Store(listener.Addr().String())
 	a.closer = listener
 	go func() {
 		if err := a.httpServer.Serve(listener); err != nil {
 			a.logger.Error("http server failure", zap.Error(err))
 		}
+		a.logger.Info("agent's http server exiting")
 	}()
 	for _, processor := range a.processors {
 		go processor.Serve()
 	}
 	return nil
 }
 
+// HTTPAddr returns the address that HTTP server is listening on
+func (a *Agent) HTTPAddr() string {
+	return a.httpAddr.Load().(string)
+}
+
 // Stop forces all agent go routines to exit.
 func (a *Agent) Stop() {
 	for _, processor := range a.processors {

cmd/agent/app/agent_test.go

@@ -18,12 +18,15 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
+
+	"github.com/jaegertracing/jaeger/pkg/testutils"
 )
 
 func TestAgentStartError(t *testing.T) {
@@ -45,8 +48,12 @@ func TestAgentStartStop(t *testing.T) {
 				},
 			},
 		},
+		HTTPServer: HTTPServerConfiguration{
+			HostPort: ":0",
+		},
 	}
-	agent, err := cfg.CreateAgent(zap.NewNop())
+	logger, logBuf := testutils.NewLogger()
+	agent, err := cfg.CreateAgent(logger)
 	require.NoError(t, err)
 	ch := make(chan error, 2)
 	go func() {
@@ -57,7 +64,14 @@ func TestAgentStartStop(t *testing.T) {
 		close(ch)
 	}()
 
-	url := fmt.Sprintf("http://%s/sampling?service=abc", agent.httpServer.Addr)
+	for i := 0; i < 1000; i++ {
+		if agent.HTTPAddr() != "" {
+			break
+		}
+		time.Sleep(time.Millisecond)
+	}
+
+	url := fmt.Sprintf("http://%s/sampling?service=abc", agent.HTTPAddr())
 	httpClient := &http.Client{
 		Timeout: 100 * time.Millisecond,
 	}
@@ -93,4 +107,12 @@ func TestAgentStartStop(t *testing.T) {
 
 	agent.Stop()
 	assert.NoError(t, <-ch)
+
+	for i := 0; i < 1000; i++ {
+		if strings.Contains(logBuf.String(), "agent's http server exiting") {
+			return
+		}
+		time.Sleep(time.Millisecond)
+	}
+	t.Fatal("Expecting server exist log")
 }

pkg/testutils/logger.go

@@ -75,6 +75,13 @@ func (b *Buffer) Stripped() string {
 	return b.Buffer.Stripped()
 }
 
+// String overwrites zaptest.Buffer.String() to make it thread safe
+func (b *Buffer) String() string {
+	b.RLock()
+	defer b.RUnlock()
+	return b.Buffer.String()
+}
+
 // Write overwrites zaptest.Buffer.bytes.Buffer.Write() to make it thread safe
 func (b *Buffer) Write(p []byte) (int, error) {
 	b.Lock()

pkg/testutils/logger_test.go

@@ -59,6 +59,7 @@ func TestRaceCondition(t *testing.T) {
 		_ = <-start
 		buffer.Lines()
 		buffer.Stripped()
+		_ = buffer.String()
 		finish.Done()
 	}()