Fix test instabilities by refactoring to make address dynamic and readable

cmd/rove-server/api_test.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"fmt"
 	"os"
 	"testing"
 
@@ -10,16 +11,27 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-var serv rove.Server = "localhost:8080"
+// To be set by the main function
+var serv rove.Server
 
 func TestMain(m *testing.M) {
-	s := server.NewServer(server.OptionPort(8080))
-	s.Initialise()
+	s := server.NewServer()
+	if err := s.Initialise(); err != nil {
+		fmt.Println(err)
+		os.Exit(1)
+	}
+
+	serv = rove.Server(s.Addr())
+
 	go s.Run()
 
+	fmt.Printf("Test server hosted on %s", serv)
 	code := m.Run()
 
-	s.Close()
+	if err := s.Close(); err != nil {
+		fmt.Println(err)
+		os.Exit(1)
+	}
 
 	os.Exit(code)
 }

cmd/rove-server/main.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"os/signal"
 	"syscall"
+	"time"
 
 	"github.com/mdiluz/rove/pkg/persistence"
 	"github.com/mdiluz/rove/pkg/server"
@@ -13,8 +14,9 @@ import (
 )
 
 var ver = flag.Bool("version", false, "Display version number")
-var port = flag.Int("port", 8080, "The port to host on")
+var port = flag.String("address", ":8080", "The address to host on")
 var data = flag.String("data", os.TempDir(), "Directory to store persistant data")
+var quit = flag.Int("quit", 0, "Quit after n seconds, useful for testing")
 
 func main() {
 	flag.Parse()
@@ -32,7 +34,7 @@ func main() {
 
 	// Create the server data
 	s := server.NewServer(
-		server.OptionPort(*port),
+		server.OptionAddress(*port),
 		server.OptionPersistentData())
 
 	// Initialise the server
@@ -52,9 +54,19 @@ func main() {
 		os.Exit(0)
 	}()
 
-	fmt.Println("Running...")
+	// Quit after a time if requested
+	if *quit != 0 {
+		go func() {
+			time.Sleep(time.Duration(*quit) * time.Second)
+			if err := s.Close(); err != nil {
+				panic(err)
+			}
+			os.Exit(0)
+		}()
+	}
 
 	// Run the server
+	fmt.Printf("Serving HTTP on %s\n", s.Addr())
 	s.Run()
 
 	// Close the server

cmd/rove/main_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"flag"
+	"fmt"
 	"os"
 	"path"
 	"testing"
@@ -11,14 +12,26 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+var address string
+
 func TestMain(m *testing.M) {
-	s := server.NewServer(server.OptionPort(8080))
-	s.Initialise()
+	s := server.NewServer()
+	if err := s.Initialise(); err != nil {
+		fmt.Println(err)
+		os.Exit(1)
+	}
+
+	address = s.Addr()
+
 	go s.Run()
 
+	fmt.Printf("Test server hosted on %s", address)
 	code := m.Run()
 
-	s.Close()
+	if err := s.Close(); err != nil {
+		fmt.Println(err)
+		os.Exit(1)
+	}
 
 	os.Exit(code)
 }
@@ -31,7 +44,7 @@ func Test_InnerMain(t *testing.T) {
 	assert.Error(t, InnerMain("status"))
 
 	// Now set the host
-	flag.Set("host", "localhost:8080")
+	flag.Set("host", address)
 
 	// No error now as we have a host
 	assert.NoError(t, InnerMain("status"))

docker-compose.yml

@@ -11,6 +11,6 @@ services:
     image: rove-server:latest
     ports:
       - "80:80"
-    command: ./rove-server --port 80 --data=/mnt/rove-server
+    command: ./rove-server --address ":80" --data=/mnt/rove-server ${ROVE_ARGS}
     volumes:
       - persistent-data:/mnt/rove-server:rw

pkg/server/server.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log"
+	"net"
 	"net/http"
 	"sync"
 	"time"
@@ -27,12 +28,14 @@ const (
 
 // Server contains the relevant data to run a game server
 type Server struct {
-	port int
+	address string
 
 	accountant *accounts.Accountant
 	world      *game.World
 
-	server *http.Server
+	listener net.Listener
+	server   *http.Server
+
 	router *mux.Router
 
 	persistence int
@@ -43,10 +46,10 @@ type Server struct {
 // ServerOption defines a server creation option
 type ServerOption func(s *Server)
 
-// OptionPort sets the server port for hosting
-func OptionPort(port int) ServerOption {
+// OptionAddress sets the server address for hosting
+func OptionAddress(address string) ServerOption {
 	return func(s *Server) {
-		s.port = port
+		s.address = address
 	}
 }
 
@@ -64,7 +67,7 @@ func NewServer(opts ...ServerOption) *Server {
 
 	// Set up the default server
 	s := &Server{
-		port:        8080,
+		address:     "",
 		persistence: EphemeralData,
 		router:      router,
 	}
@@ -75,7 +78,7 @@ func NewServer(opts ...ServerOption) *Server {
 	}
 
 	// Set up the server object
-	s.server = &http.Server{Addr: fmt.Sprintf(":%d", s.port), Handler: router}
+	s.server = &http.Server{Addr: s.address, Handler: s.router}
 
 	// Create the accountant
 	s.accountant = accounts.NewAccountant()
@@ -85,7 +88,10 @@ func NewServer(opts ...ServerOption) *Server {
 }
 
 // Initialise sets up internal state ready to serve
-func (s *Server) Initialise() error {
+func (s *Server) Initialise() (err error) {
+
+	// Add to our sync
+	s.sync.Add(1)
 
 	// Load the accounts if requested
 	if s.persistence == PersistentData {
@@ -99,19 +105,26 @@ func (s *Server) Initialise() error {
 		s.router.HandleFunc(route.path, s.wrapHandler(route.method, route.handler))
 	}
 
-	// Add to our sync
-	s.sync.Add(1)
+	// Start the listen
+	if s.listener, err = net.Listen("tcp", s.server.Addr); err != nil {
+		return err
+	}
 
+	s.address = s.listener.Addr().String()
 	return nil
 }
 
+// Addr will return the server address set after the listen
+func (s Server) Addr() string {
+	return s.address
+}
+
 // Run executes the server
 func (s *Server) Run() {
 	defer s.sync.Done()
 
-	// Listen and serve the http requests
-	fmt.Printf("Serving HTTP on port %d\n", s.port)
-	if err := s.server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
+	// Serve the http requests
+	if err := s.server.Serve(s.listener); err != nil && err != http.ErrServerClosed {
 		log.Fatal(err)
 	}
 }

pkg/server/server_test.go

@@ -11,12 +11,12 @@ func TestNewServer(t *testing.T) {
 	}
 }
 
-func TestNewServer_OptionPort(t *testing.T) {
-	server := NewServer(OptionPort(1234))
+func TestNewServer_OptionAddress(t *testing.T) {
+	server := NewServer(OptionAddress(":1234"))
 	if server == nil {
 		t.Error("Failed to create server")
-	} else if server.port != 1234 {
-		t.Error("Failed to set server port")
+	} else if server.address != ":1234" {
+		t.Error("Failed to set server address")
 	}
 }
 
@@ -36,6 +36,7 @@ func TestServer_Run(t *testing.T) {
 	} else if err := server.Initialise(); err != nil {
 		t.Error(err)
 	}
+
 	go server.Run()
 
 	if err := server.Close(); err != nil {
@@ -50,6 +51,7 @@ func TestServer_RunPersistentData(t *testing.T) {
 	} else if err := server.Initialise(); err != nil {
 		t.Error(err)
 	}
+
 	go server.Run()
 
 	if err := server.Close(); err != nil {

script/test.sh

@@ -9,8 +9,7 @@ go mod download
 go build ./...
 
 # Run the server and shut it down again to ensure our docker-compose works
-docker-compose up --detach --build
-docker-compose down
+ROVE_ARGS="--quit 1" docker-compose up --build --exit-code-from=rove-server --abort-on-container-exit
 
 # Run tests with coverage
 go test -v ./... -cover -coverprofile=/tmp/c.out