Merge pull request #2 from edio/refactoring

Code re-factoring and cleanup
ncbi · Dec 29, 2017 · 1617c4f · 1617c4f
2 parents 79d965c + b6b0475
commit 1617c4f
Show file tree

Hide file tree

Showing 7 changed files with 285 additions and 151 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Changed
+
+- `--timeout` option now affects both dialing to server and RPC call (before that dialing had hard-coded 1s timeout)
+
 ## 1.0.0 - 2017-12-13
 
 ### Added

diff --git a/README.md b/README.md
@@ -33,10 +33,16 @@ Get help
 gprobe -h
 ```
 
-## Building
+## Building from source
 
 Valid _go_ environment is required to build `gprobe` (`go` is in `PATH`, `GOPATH` is set, etc.).
 
+Clone code into valid `GOPATH` location
+
+```bash
+git clone git@github.com:ncbi/gprobe.git $GOPATH/src/github.com/ncbi/gprobe
+```
+
 Build distributable tarballs for all OSes
 
 ```bash
@@ -56,7 +62,7 @@ This project follows git-flow branching model. All development is done off of th
 
 To contribute:
 
-1. Create a feature branch from the latest `develop`, commit your work there
+1. Fork or create a feature branch from the latest `develop`, commit your work there
     ```bash
     git checkout develop
     git pull

diff --git a/acctest/main_test.go b/acctest/main_test.go
@@ -22,105 +22,132 @@ package acctest
 
 import (
 	"bytes"
+	"flag"
 	"fmt"
 	"github.com/stretchr/testify/assert"
-	"google.golang.org/grpc"
-	"google.golang.org/grpc/health"
-	hv1 "google.golang.org/grpc/health/grpc_health_v1"
 	"io"
 	"log"
-	"net"
 	"os"
 	"os/exec"
 	"syscall"
 	"testing"
-	"flag"
+
+	hv1 "google.golang.org/grpc/health/grpc_health_v1"
 )
 
 var (
-	port int
-	listenAddr string
-	bin string
+	port        int
+	cert        string
+	key         string
+	bin         string
+	stubSrvAddr string
 )
 
-func startServer() net.Listener {
-	listener, err := net.Listen("tcp", fmt.Sprintf(":%d", port))
-	if err != nil {
-		log.Fatalf("failed to listen: %v", err)
-	}
-
-	grpcServer := grpc.NewServer()
-	server := health.NewServer()
-	hv1.RegisterHealthServer(grpcServer, server)
-
-	server.SetServingStatus("foo", hv1.HealthCheckResponse_SERVING)
-	server.SetServingStatus("bar", hv1.HealthCheckResponse_NOT_SERVING)
-	go grpcServer.Serve(listener)
-	return listener
-}
-
 func init() {
 	flag.IntVar(&port, "stub-port", 54321, "port for the stub server")
-	flag.StringVar(&bin, "gprobe", "", "path to the gprobe binary")
+	flag.StringVar(&bin, "gprobe", "../gprobe", "path to the gprobe binary")
 }
 
 func TestMain(m *testing.M) {
 	flag.Parse()
-	listenAddr = fmt.Sprintf("%s:%d", "localhost", port)
-
-	lis := startServer()
-	result := 0
-	defer func() {
-		lis.Close()
-		os.Exit(result)
-	}()
-
-	result = m.Run()
+	stubSrvAddr = fmt.Sprintf("%s:%d", "localhost", port)
+	os.Exit(m.Run())
 }
 
 func TestShouldReturnServingForRunningServer(t *testing.T) {
-	stdout, stderr, exitcode := runBin(t, listenAddr)
+	// given
+	srv, _, err := StartInsecureServer(port)
+	if err != nil {
+		log.Fatalf("can't start stub server: %v", err)
+	}
+	defer srv.GracefulStop()
+
+	// when
+	stdout, stderr, exitcode := runBin(t, stubSrvAddr)
 
 	assert.Equal(t, 0, exitcode)
 	assert.Equal(t, "SERVING\n", stdout)
 	assert.Empty(t, stderr)
 }
 
 func TestShouldFailIfServerIsNotListening(t *testing.T) {
-	stdout, stderr, exitcode := runBin(t, "nosuchhost:1234")
+	// given no server
 
+	// when
+	stdout, stderr, exitcode := runBin(t, stubSrvAddr)
+
+	// then
 	assert.Equal(t, 127, exitcode)
 	assert.Empty(t, stdout)
-	assert.Contains(t, stderr, "error", "should print status to STDOUT")
+	assert.Contains(t, stderr, "error")
 }
 
 func TestShouldReturnServingForHealthyService(t *testing.T) {
-	stdout, stderr, exitcode := runBin(t, listenAddr, "foo")
+	// given
+	srv, svc, err := StartInsecureServer(port)
+	if err != nil {
+		log.Fatalf("can't start stub server: %v", err)
+	}
+	defer srv.GracefulStop()
+	svc.SetServingStatus("foo", hv1.HealthCheckResponse_SERVING)
+
+	// when
+	stdout, stderr, exitcode := runBin(t, stubSrvAddr, "foo")
 
+	// then
 	assert.Equal(t, 0, exitcode)
 	assert.Equal(t, "SERVING\n", stdout)
 	assert.Empty(t, stderr)
 }
 
 func TestShouldReturnNotServingForUnhealthyService(t *testing.T) {
-	stdout, stderr, exitcode := runBin(t, listenAddr, "bar")
+	// given
+	srv, svc, err := StartInsecureServer(port)
+	if err != nil {
+		log.Fatalf("can't start stub server: %v", err)
+	}
+	defer srv.GracefulStop()
+	svc.SetServingStatus("foo", hv1.HealthCheckResponse_NOT_SERVING)
 
+	// when
+	stdout, stderr, exitcode := runBin(t, stubSrvAddr, "foo")
+
+	// then
 	assert.Equal(t, 2, exitcode)
 	assert.Equal(t, "NOT_SERVING\n", stdout)
 	assert.Contains(t, stderr, "health-check failed")
 }
 
 func TestShouldNotFailForUnhealthyServiceIfNoFailIsSet(t *testing.T) {
-	stdout, stderr, exitcode := runBin(t, "--no-fail", listenAddr, "bar")
+	// given
+	srv, svc, err := StartInsecureServer(port)
+	if err != nil {
+		log.Fatalf("can't start stub server: %v", err)
+	}
+	defer srv.GracefulStop()
+	svc.SetServingStatus("foo", hv1.HealthCheckResponse_NOT_SERVING)
+
+	// when
+	stdout, stderr, exitcode := runBin(t, "--no-fail", stubSrvAddr, "foo")
 
+	// then
 	assert.Equal(t, 0, exitcode)
 	assert.Equal(t, "NOT_SERVING\n", stdout)
 	assert.Empty(t, stderr)
 }
 
 func TestShouldFailIfServiceHealthCheckIsNotRegistered(t *testing.T) {
-	stdout, stderr, exitcode := runBin(t, listenAddr, "non_registered_service")
+	// given
+	srv, _, err := StartInsecureServer(port)
+	if err != nil {
+		log.Fatalf("can't start stub server: %v", err)
+	}
+	defer srv.GracefulStop()
 
+	// when
+	stdout, stderr, exitcode := runBin(t, stubSrvAddr, "foo")
+
+	// then
 	assert.Equal(t, 127, exitcode)
 	assert.Empty(t, stdout)
 	assert.Contains(t, stderr, "NotFound")
@@ -166,5 +193,3 @@ func waitForExitCode(t *testing.T, cmd *exec.Cmd) (exitcode int) {
 	}
 	return
 }
-
-
diff --git a/acctest/stubserver.go b/acctest/stubserver.go
@@ -0,0 +1,28 @@
+package acctest
+
+import (
+	"fmt"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/health"
+	hv1 "google.golang.org/grpc/health/grpc_health_v1"
+	"net"
+)
+
+// StartInsecureServer starts new gRPC application with simple health service.
+// It is callers responsibility to Stop the server
+func StartInsecureServer(port int) (*grpc.Server, *health.Server, error) {
+	return doStart(port)
+}
+
+func doStart(port int, options ...grpc.ServerOption) (server *grpc.Server, service *health.Server, err error) {
+	listener, err := net.Listen("tcp", fmt.Sprintf(":%d", port))
+	if err != nil {
+		return
+	}
+	server = grpc.NewServer(options...)
+	service = health.NewServer()
+	hv1.RegisterHealthServer(server, service)
+
+	go server.Serve(listener)
+	return server, service, nil
+}