From 1d2087e2b919eb16840d91a44f52079d7e09e666 Mon Sep 17 00:00:00 2001 From: Marc Di Luzio Date: Sat, 6 Jun 2020 11:52:12 +0100 Subject: [PATCH] Fix test instabilities by refactoring to make address dynamic and readable --- cmd/rove-server/api_test.go | 20 +++++++++++++++---- cmd/rove-server/main.go | 18 ++++++++++++++--- cmd/rove/main_test.go | 21 ++++++++++++++++---- docker-compose.yml | 2 +- pkg/server/server.go | 39 ++++++++++++++++++++++++------------- pkg/server/server_test.go | 10 ++++++---- script/test.sh | 3 +-- 7 files changed, 82 insertions(+), 31 deletions(-) diff --git a/cmd/rove-server/api_test.go b/cmd/rove-server/api_test.go index 79431e7..50008a6 100644 --- a/cmd/rove-server/api_test.go +++ b/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) } diff --git a/cmd/rove-server/main.go b/cmd/rove-server/main.go index b64a919..e3042ae 100644 --- a/cmd/rove-server/main.go +++ b/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 diff --git a/cmd/rove/main_test.go b/cmd/rove/main_test.go index 52bf964..d19759f 100644 --- a/cmd/rove/main_test.go +++ b/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")) diff --git a/docker-compose.yml b/docker-compose.yml index 76f4aa0..bdb69e6 100644 --- a/docker-compose.yml +++ b/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 diff --git a/pkg/server/server.go b/pkg/server/server.go index 3a84b2a..55c6fc9 100644 --- a/pkg/server/server.go +++ b/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) } } diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 619ebbe..5f9b4ba 100644 --- a/pkg/server/server_test.go +++ b/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 { diff --git a/script/test.sh b/script/test.sh index 5b0ff09..a0ffa6a 100755 --- a/script/test.sh +++ b/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