fixing tests

4 files changed, 57 insertions, 44 deletions

diff --git a/server.go b/server.go
index 55ca0771..fb6927d6 100644
--- a/server.go
+++ b/server.go
@@ -2,8 +2,8 @@ package roadrunner
 
 import (
 	"fmt"
-	"os/exec"
 	"sync"
+	"github.com/pkg/errors"
 )
 
 const (
@@ -28,9 +28,6 @@ type Server struct {
 	// configures server, pool, cmd creation and factory.
 	cfg *ServerConfig
 
-	// worker command creator
-	cmd func() *exec.Cmd
-
 	// observes pool events (can be attached to multiple pools at the same time)
 	observer func(event int, ctx interface{})
 
@@ -48,8 +45,8 @@ type Server struct {
 }
 
 // NewServer creates new router. Make sure to call configure before the usage.
-func NewServer(cmd func() *exec.Cmd, cfg *ServerConfig) *Server {
-	return &Server{cmd: cmd, cfg: cfg}
+func NewServer(cfg *ServerConfig) *Server {
+	return &Server{cfg: cfg}
 }
 
 // Observe attaches server event watcher.
@@ -68,11 +65,15 @@ func (srv *Server) Reconfigure(cfg *ServerConfig) error {
 	}
 	srv.mu.Unlock()
 
+	if srv.cfg.Differs(cfg) {
+		return errors.New("unable to reconfigure server (cmd and pool changes are allowed)")
+	}
+
 	srv.mu.Lock()
 	previous := srv.pool
 	srv.mu.Unlock()
 
-	pool, err := NewPool(srv.cmd, srv.factory, cfg.Pool)
+	pool, err := NewPool(cfg.makeCommand(), srv.factory, cfg.Pool)
 	if err != nil {
 		return err
 	}
@@ -108,7 +109,7 @@ func (srv *Server) Start() (err error) {
 		return err
 	}
 
-	if srv.pool, err = NewPool(srv.cmd, srv.factory, srv.cfg.Pool); err != nil {
+	if srv.pool, err = NewPool(srv.cfg.makeCommand(), srv.factory, srv.cfg.Pool); err != nil {
 		return err
 	}
 
@@ -132,7 +133,6 @@ func (srv *Server) Stop() error {
 	srv.pool.Destroy()
 	srv.factory.Close()
 
-	srv.cmd = nil
 	srv.factory = nil
 	srv.pool = nil
 	srv.started = false
diff --git a/server_config.go b/server_config.go
index d7c27c4a..4a03f44d 100644
--- a/server_config.go
+++ b/server_config.go
@@ -10,8 +10,7 @@ import (
 
 // Server config combines factory, pool and cmd configurations.
 type ServerConfig struct {
-	// Command includes command strings with all the parameters, example: "php worker.php pipes". This config section
-	// must not change on re-configuration.
+	// Command includes command strings with all the parameters, example: "php worker.php pipes".
 	Command string
 
 	// Relay defines connection method and factory to be used to connect to workers:
@@ -28,17 +27,17 @@ type ServerConfig struct {
 	Pool Config
 }
 
-// Differs returns true if configuration has changed but ignores pool changes.
+// Differs returns true if configuration has changed but ignores pool or cmd changes.
 func (cfg *ServerConfig) Differs(new *ServerConfig) bool {
-	return cfg.Command != new.Command || cfg.Relay != new.Relay || cfg.RelayTimeout != new.RelayTimeout
+	return cfg.Relay != new.Relay || cfg.RelayTimeout != new.RelayTimeout
 }
 
 // makeCommands returns new command provider based on configured options.
-func (cfg *ServerConfig) makeCommand() (func() *exec.Cmd, error) {
+func (cfg *ServerConfig) makeCommand() func() *exec.Cmd {
 	var cmd = strings.Split(cfg.Command, " ")
 	return func() *exec.Cmd {
 		return exec.Command(cmd[0], cmd[1:]...)
-	}, nil
+	}
 }
 
 // makeFactory creates and connects new factory instance based on given parameters.
diff --git a/server_config_test.go b/server_config_test.go
index 74878782..1831ae95 100644
--- a/server_config_test.go
+++ b/server_config_test.go
@@ -81,3 +81,12 @@ func Test_ServerConfig_ErrorMethod(t *testing.T) {
 	assert.Nil(t, f)
 	assert.Error(t, err)
 }
+
+func Test_ServerConfig_Cmd(t *testing.T) {
+	cfg := &ServerConfig{
+		Command: "php php-src/tests/client.php pipes",
+	}
+
+	cmd := cfg.makeCommand()
+	assert.NotNil(t, cmd)
+}
diff --git a/server_test.go b/server_test.go
index 64b240d1..3a19a5c5 100644
--- a/server_test.go
+++ b/server_test.go
@@ -2,17 +2,17 @@ package roadrunner
 
 import (
 	"github.com/stretchr/testify/assert"
-	"os/exec"
 	"runtime"
 	"testing"
 	"time"
+	"os/exec"
 )
 
 func TestServer_PipesEcho(t *testing.T) {
 	srv := NewServer(
-		func() *exec.Cmd { return exec.Command("php", "php-src/tests/client.php", "echo", "pipes") },
 		&ServerConfig{
-			Relay: "pipes",
+			Command: "php php-src/tests/client.php echo pipes",
+			Relay:   "pipes",
 			Pool: Config{
 				NumWorkers:      uint64(runtime.NumCPU()),
 				AllocateTimeout: time.Second,
@@ -35,8 +35,8 @@ func TestServer_PipesEcho(t *testing.T) {
 
 func TestServer_SocketEcho(t *testing.T) {
 	srv := NewServer(
-		func() *exec.Cmd { return exec.Command("php", "php-src/tests/client.php", "echo", "tcp") },
 		&ServerConfig{
+			Command:      "php php-src/tests/client.php echo tcp",
 			Relay:        "tcp://:9007",
 			RelayTimeout: 10 * time.Second,
 			Pool: Config{
@@ -61,9 +61,9 @@ func TestServer_SocketEcho(t *testing.T) {
 
 func TestServer_Configure_BeforeStart(t *testing.T) {
 	srv := NewServer(
-		func() *exec.Cmd { return exec.Command("php", "php-src/tests/client.php", "echo", "pipes") },
 		&ServerConfig{
-			Relay: "pipes",
+			Command: "php php-src/tests/client.php echo pipes",
+			Relay:   "pipes",
 			Pool: Config{
 				NumWorkers:      uint64(runtime.NumCPU()),
 				AllocateTimeout: time.Second,
@@ -73,7 +73,8 @@ func TestServer_Configure_BeforeStart(t *testing.T) {
 	defer srv.Stop()
 
 	err := srv.Reconfigure(&ServerConfig{
-		Relay: "pipes",
+		Command: "php php-src/tests/client.php echo pipes",
+		Relay:   "pipes",
 		Pool: Config{
 			NumWorkers:      2,
 			AllocateTimeout: time.Second,
@@ -95,11 +96,11 @@ func TestServer_Configure_BeforeStart(t *testing.T) {
 	assert.Len(t, srv.Workers(), 2)
 }
 
-func TestServer_StopUnstarted(t *testing.T) {
+func TestServer_Stop_NotStarted(t *testing.T) {
 	srv := NewServer(
-		func() *exec.Cmd { return exec.Command("php", "php-src/tests/client.php", "echo", "pipes") },
 		&ServerConfig{
-			Relay: "pipes",
+			Command: "php php-src/tests/client.php echo pipes",
+			Relay:   "pipes",
 			Pool: Config{
 				NumWorkers:      uint64(runtime.NumCPU()),
 				AllocateTimeout: time.Second,
@@ -112,9 +113,9 @@ func TestServer_StopUnstarted(t *testing.T) {
 
 func TestServer_Reconfigure(t *testing.T) {
 	srv := NewServer(
-		func() *exec.Cmd { return exec.Command("php", "php-src/tests/client.php", "echo", "pipes") },
 		&ServerConfig{
-			Relay: "pipes",
+			Command: "php php-src/tests/client.php echo pipes",
+			Relay:   "pipes",
 			Pool: Config{
 				NumWorkers:      1,
 				AllocateTimeout: time.Second,
@@ -127,7 +128,8 @@ func TestServer_Reconfigure(t *testing.T) {
 	assert.Len(t, srv.Workers(), 1)
 
 	err := srv.Reconfigure(&ServerConfig{
-		Relay: "pipes",
+		Command: "php php-src/tests/client.php echo pipes",
+		Relay:   "pipes",
 		Pool: Config{
 			NumWorkers:      2,
 			AllocateTimeout: time.Second,
@@ -141,9 +143,9 @@ func TestServer_Reconfigure(t *testing.T) {
 
 func TestServer_Reset(t *testing.T) {
 	srv := NewServer(
-		func() *exec.Cmd { return exec.Command("php", "php-src/tests/client.php", "echo", "pipes") },
 		&ServerConfig{
-			Relay: "pipes",
+			Command: "php php-src/tests/client.php echo pipes",
+			Relay:   "pipes",
 			Pool: Config{
 				NumWorkers:      1,
 				AllocateTimeout: time.Second,
@@ -163,9 +165,9 @@ func TestServer_Reset(t *testing.T) {
 
 func TestServer_ReplacePool(t *testing.T) {
 	srv := NewServer(
-		func() *exec.Cmd { return exec.Command("php", "php-src/tests/client.php", "echo", "pipes") },
 		&ServerConfig{
-			Relay: "pipes",
+			Command: "php php-src/tests/client.php echo pipes",
+			Relay:   "pipes",
 			Pool: Config{
 				NumWorkers:      1,
 				AllocateTimeout: time.Second,
@@ -192,17 +194,15 @@ func TestServer_ReplacePool(t *testing.T) {
 }
 
 func TestServer_ServerFailure(t *testing.T) {
-	mode := "pipes"
-	srv := NewServer(
-		func() *exec.Cmd { return exec.Command("php", "php-src/tests/client.php", "echo", mode) },
-		&ServerConfig{
-			Relay: "pipes",
-			Pool: Config{
-				NumWorkers:      1,
-				AllocateTimeout: time.Second,
-				DestroyTimeout:  time.Second,
-			},
-		})
+	srv := NewServer(&ServerConfig{
+		Command: "php php-src/tests/client.php echo pipes",
+		Relay:   "pipes",
+		Pool: Config{
+			NumWorkers:      1,
+			AllocateTimeout: time.Second,
+			DestroyTimeout:  time.Second,
+		},
+	})
 	defer srv.Stop()
 
 	assert.NoError(t, srv.Start())
@@ -214,8 +214,13 @@ func TestServer_ServerFailure(t *testing.T) {
 		}
 	})
 
+	// emulating potential server failure
+	srv.cfg.Command = "php php-src/tests/client.php echo broken-connection"
+	srv.pool.(*StaticPool).cmd = func() *exec.Cmd {
+		return exec.Command("php", "php-src/tests/client.php", "echo", "broken-connection")
+	}
+
 	// killing random worker and expecting pool to replace it
-	mode = "suddenly-broken"
 	srv.Workers()[0].cmd.Process.Kill()
 
 	<-failure