Merge pull request #224 from spiral/feature/reuse-ports-and-test-improvements

Test improvements

8 files changed, 56 insertions, 40 deletions

diff --git a/.travis.yml b/.travis.yml
index e98795fb..f3955eb9 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -29,17 +29,19 @@ script:
   - go test ./service/health -race -v -coverprofile=health.txt -covermode=atomic
 
 after_success:
-  - bash <(curl -s https://codecov.io/bash) -f lib.txt
-  - bash <(curl -s https://codecov.io/bash) -f util.txt
-  - bash <(curl -s https://codecov.io/bash) -f service.txt
-  - bash <(curl -s https://codecov.io/bash) -f env.txt
-  - bash <(curl -s https://codecov.io/bash) -f rpc.txt
-  - bash <(curl -s https://codecov.io/bash) -f http.txt
-  - bash <(curl -s https://codecov.io/bash) -f static.txt
-  - bash <(curl -s https://codecov.io/bash) -f limit.txt
-  - bash <(curl -s https://codecov.io/bash) -f headers.txt
-  - bash <(curl -s https://codecov.io/bash) -f metrics.txt
-  - bash <(curl -s https://codecov.io/bash) -f health.txt
+  - curl https://codecov.io/bash -o codecov-bash
+  - chmod +x codecov-bash
+  - ./codecov-bash -f lib.txt
+  - ./codecov-bash -f util.txt
+  - ./codecov-bash -f service.txt
+  - ./codecov-bash -f env.txt
+  - ./codecov-bash -f rpc.txt
+  - ./codecov-bash -f http.txt
+  - ./codecov-bash -f static.txt
+  - ./codecov-bash -f limit.txt
+  - ./codecov-bash -f headers.txt
+  - ./codecov-bash -f metrics.txt
+  - ./codecov-bash -f health.txt
 
 jobs:
   include:
diff --git a/server_config.go b/server_config.go
index 641c1866..5403ff01 100644
--- a/server_config.go
+++ b/server_config.go
@@ -127,7 +127,7 @@ func (cfg *ServerConfig) makeFactory() (Factory, error) {
 		return nil, errors.New("invalid relay DSN (pipes, tcp://:6001, unix://rr.sock)")
 	}
 
-	if dsn[0] == "unix" {
+	if dsn[0] == "unix" && fileExists(dsn[1]) {
 		err := syscall.Unlink(dsn[1])
 		if err != nil {
 			return nil, err
@@ -141,3 +141,13 @@ func (cfg *ServerConfig) makeFactory() (Factory, error) {
 
 	return NewSocketFactory(ln, cfg.RelayTimeout), nil
 }
+
+// fileExists checks if a file exists and is not a directory before we
+// try using it to prevent further errors.
+func fileExists(filename string) bool {
+	info, err := os.Stat(filename)
+	if os.IsNotExist(err) {
+		return false
+	}
+	return !info.IsDir()
+}
diff --git a/server_config_test.go b/server_config_test.go
index 4f26d6ab..303eba90 100644
--- a/server_config_test.go
+++ b/server_config_test.go
@@ -71,6 +71,10 @@ func Test_ServerConfig_UnixSocketFactory(t *testing.T) {
 
 	cfg := &ServerConfig{Relay: "unix://unix.sock"}
 	f, err := cfg.makeFactory()
+	if err != nil {
+		t.Error(err)
+	}
+
 	defer func() {
 		err := f.Close()
 		if err != nil {
diff --git a/service/health/service_test.go b/service/health/service_test.go
index d346d92b..e8685548 100644
--- a/service/health/service_test.go
+++ b/service/health/service_test.go
@@ -90,10 +90,10 @@ func TestService_Serve_DeadWorker(t *testing.T) {
 
 	assert.NoError(t, c.Init(&testCfg{
 		healthCfg: `{
-			"address": "localhost:2116"
+			"address": "localhost:2117"
 		}`,
 		httpCfg: `{
-			"address": "localhost:2115",
+			"address": "localhost:2118",
 			"workers":{
 				"command": "php ../../tests/http/slow-client.php echo pipes 1000",
 				"pool": {"numWorkers": 1}
@@ -126,7 +126,7 @@ func TestService_Serve_DeadWorker(t *testing.T) {
 	}
 
 	// Check health check
-	_, res, err := get("http://localhost:2116/")
+	_, res, err := get("http://localhost:2117/")
 	assert.NoError(t, err)
 	assert.Equal(t, http.StatusInternalServerError, res.StatusCode)
 }
@@ -141,10 +141,10 @@ func TestService_Serve_DeadWorkerStillHealthy(t *testing.T) {
 
 	assert.NoError(t, c.Init(&testCfg{
 		healthCfg: `{
-			"address": "localhost:2116"
+			"address": "localhost:2119"
 		}`,
 		httpCfg: `{
-			"address": "localhost:2115",
+			"address": "localhost:2120",
 			"workers":{
 				"command": "php ../../tests/http/client.php echo pipes",
 				"pool": {"numWorkers": 2}
@@ -177,7 +177,7 @@ func TestService_Serve_DeadWorkerStillHealthy(t *testing.T) {
 	}
 
 	// Check health check
-	_, res, err := get("http://localhost:2116/")
+	_, res, err := get("http://localhost:2119/")
 	assert.NoError(t, err)
 	assert.Equal(t, http.StatusOK, res.StatusCode)
 }
@@ -191,7 +191,7 @@ func TestService_Serve_NoHTTPService(t *testing.T) {
 
 	assert.NoError(t, c.Init(&testCfg{
 		healthCfg: `{
-			"address": "localhost:2116"
+			"address": "localhost:2121"
 		}`,
 	}))
 
@@ -212,10 +212,10 @@ func TestService_Serve_NoServer(t *testing.T) {
 
 	assert.NoError(t, c.Init(&testCfg{
 		healthCfg: `{
-			"address": "localhost:2116"
+			"address": "localhost:2122"
 		}`,
 		httpCfg: `{
-			"address": "localhost:2115",
+			"address": "localhost:2123",
 			"workers":{
 				"command": "php ../../tests/http/client.php echo pipes",
 				"pool": {"numWorkers": 1}
@@ -243,7 +243,7 @@ func TestService_Serve_NoServer(t *testing.T) {
 	// Set the httpService to nil
 	healthSvc.httpService = nil
 
-	_, res, err := get("http://localhost:2116/")
+	_, res, err := get("http://localhost:2122/")
 	assert.NoError(t, err)
 	assert.Equal(t, http.StatusInternalServerError, res.StatusCode)
 }
@@ -260,10 +260,10 @@ func TestService_Serve_NoPool(t *testing.T) {
 
 	assert.NoError(t, c.Init(&testCfg{
 		healthCfg: `{
-			"address": "localhost:2116"
+			"address": "localhost:2124"
 		}`,
 		httpCfg: `{
-			"address": "localhost:2115",
+			"address": "localhost:2125",
 			"workers":{
 				"command": "php ../../tests/http/client.php echo pipes",
 				"pool": {"numWorkers": 1}
@@ -291,7 +291,7 @@ func TestService_Serve_NoPool(t *testing.T) {
 	// Stop the pool
 	httpSvc.Server().Stop()
 
-	_, res, err := get("http://localhost:2116/")
+	_, res, err := get("http://localhost:2124/")
 	assert.NoError(t, err)
 	assert.Equal(t, http.StatusInternalServerError, res.StatusCode)
 }
diff --git a/service/rpc/config_test.go b/service/rpc/config_test.go
index 6791128b..c65e7415 100644
--- a/service/rpc/config_test.go
+++ b/service/rpc/config_test.go
@@ -81,7 +81,7 @@ func Test_Config_Error(t *testing.T) {
 	ln, err := cfg.Listener()
 	assert.Nil(t, ln)
 	assert.Error(t, err)
-	assert.Equal(t, "Invalid DSN (tcp://:6001, unix://file.sock)", err.Error())
+	assert.Equal(t, "invalid DSN (tcp://:6001, unix://file.sock)", err.Error())
 }
 
 func Test_Config_ErrorMethod(t *testing.T) {
diff --git a/socket_factory_test.go b/socket_factory_test.go
index 9f74cf8c..8beb3fc6 100644
--- a/socket_factory_test.go
+++ b/socket_factory_test.go
@@ -370,15 +370,13 @@ func Test_Unix_Broken(t *testing.T) {
 	w, _ := NewSocketFactory(ls, time.Minute).SpawnWorker(cmd)
 	go func() {
 		err := w.Wait()
-
 		assert.Error(t, err)
 		assert.Contains(t, err.Error(), "undefined_function()")
 	}()
+
 	defer func() {
 		err = w.Stop()
-		if err != nil {
-			t.Errorf("error stopping the worker: error %v", err)
-		}
+		assert.Error(t, err)
 	}()
 
 	res, err := w.Exec(&Payload{Body: []byte("hello")})
diff --git a/util/network.go b/util/network.go
index 3f533023..b9066de7 100644
--- a/util/network.go
+++ b/util/network.go
@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"net"
+	"os"
 	"strings"
 	"syscall"
 )
@@ -19,7 +20,7 @@ func CreateListener(address string) (net.Listener, error) {
 		return nil, errors.New("invalid Protocol (tcp://:6001, unix://file.sock)")
 	}
 
-	if dsn[0] == "unix" {
+	if dsn[0] == "unix" && fileExists(dsn[1]) {
 		err := syscall.Unlink(dsn[1])
 		if err != nil {
 			return nil, fmt.Errorf("error during the unlink syscall: error %v", err)
@@ -28,3 +29,13 @@ func CreateListener(address string) (net.Listener, error) {
 
 	return net.Listen(dsn[0], dsn[1])
 }
+
+// fileExists checks if a file exists and is not a directory before we
+// try using it to prevent further errors.
+func fileExists(filename string) bool {
+	info, err := os.Stat(filename)
+	if os.IsNotExist(err) {
+		return false
+	}
+	return !info.IsDir()
+}
diff --git a/worker.go b/worker.go
index 32f63554..33b9d496 100644
--- a/worker.go
+++ b/worker.go
@@ -6,7 +6,6 @@ import (
 	"github.com/spiral/goridge"
 	"os"
 	"os/exec"
-	"runtime"
 	"strconv"
 	"strings"
 	"sync"
@@ -102,14 +101,6 @@ func (w *Worker) Wait() error {
 	w.mu.Lock()
 	defer w.mu.Unlock()
 
-	if runtime.GOOS != "windows" {
-		// windows handles processes and close pipes differently,
-		// we can ignore wait here as process.Wait() already being handled above
-		if err := w.cmd.Wait(); err != nil {
-			return err
-		}
-	}
-
 	if w.endState.Success() {
 		w.state.set(StateStopped)
 		return nil