Uodate docs and more comments for future

cluster_test.go

@@ -1,7 +1,6 @@
 package zk
 
 import (
-	"os"
 	"sync"
 	"testing"
 	"time"
@@ -18,7 +17,7 @@ func (lw logWriter) Write(b []byte) (int, error) {
 }
 
 func TestBasicCluster(t *testing.T) {
-	ts, err := StartTestCluster(t, 3, os.Stdout, logWriter{t: t, p: "[ZKERR] "})
+	ts, err := StartTestCluster(t, 3, nil, logWriter{t: t, p: "[ZKERR] "})
 	if err != nil {
 		t.Fatal(err)
 	}

conn.go

@@ -1281,8 +1281,11 @@ func (c *Conn) Multi(ops ...interface{}) ([]MultiResponse, error) {
 }
 
 // IncrementalReconfig is the zookeeper reconfiguration api that allows adding and removing servers
-// by lists of members.
-// Return the new configuration stats.
+// by lists of members. For more info refer to the ZK documentation.
+//
+// An optional version allows for conditional reconfigurations, -1 ignores the condition.
+//
+// Returns the new configuration znode stat.
 func (c *Conn) IncrementalReconfig(joining, leaving []string, version int64) (*Stat, error) {
 	// TODO: validate the shape of the member string to give early feedback.
 	request := &reconfigRequest{
@@ -1294,9 +1297,12 @@ func (c *Conn) IncrementalReconfig(joining, leaving []string, version int64) (*S
 	return c.internalReconfig(request)
 }
 
-// Reconfig is the non-incremental update functionality for Zookeeper where the list preovided
-// is the entire new member list.
-// the optional version allows for conditional reconfigurations, -1 ignores the condition.
+// Reconfig is the non-incremental update functionality for Zookeeper where the list provided
+// is the entire new member list. For more info refer to the ZK documentation.
+//
+// An optional version allows for conditional reconfigurations, -1 ignores the condition.
+//
+// Returns the new configuration znode stat.
 func (c *Conn) Reconfig(members []string, version int64) (*Stat, error) {
 	request := &reconfigRequest{
 		NewMembers:  []byte(strings.Join(members, ",")),

conn_test.go

@@ -1,6 +1,7 @@
 package zk
 
 import (
+	"context"
 	"io/ioutil"
 	"testing"
 	"time"
@@ -32,14 +33,12 @@ func TestRecurringReAuthHang(t *testing.T) {
 	if err != nil {
 		panic(err)
 	}
-	for conn.State() != StateHasSession {
-		time.Sleep(50 * time.Millisecond)
-	}
+	defer conn.Close()
 
-	go func() {
-		for range evtC {
-		}
-	}()
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
+	defer cancel()
+
+	waitForSession(ctx, evtC)
 
 	// Add auth.
 	conn.AddAuth("digest", []byte("test:test"))
@@ -49,6 +48,9 @@ func TestRecurringReAuthHang(t *testing.T) {
 	conn.debugReauthDone = make(chan struct{})
 	zkC.StopServer(currentServer)
 	// wait connect to new zookeeper.
+	ctx, cancel = context.WithTimeout(context.Background(), time.Second*5)
+	defer cancel()
+
 	for conn.Server() == currentServer && conn.State() != StateHasSession {
 		time.Sleep(100 * time.Millisecond)
 	}

server_help.go

@@ -34,10 +34,24 @@ type TestCluster struct {
 	Servers []TestServer
 }
 
+// TODO: pull this into its own package to allow for better isolation of integration tests vs. unit
+// testing. This should be used on CI systems and local only when needed whereas unit tests should remain
+// fast and not rely on external dependecies.
 func StartTestCluster(t *testing.T, size int, stdout, stderr io.Writer) (*TestCluster, error) {
 	if testing.Short() {
 		t.Skip("ZK cluster tests skipped in short case.")
 	}
+
+	if testing.Verbose() {
+		// if testing verbose we just spam the server logs as many issues with tests will have the ZK server
+		// logs have the cause of the failure in it.
+		if stdout == nil {
+			stdout = os.Stderr
+		} else {
+			stdout = io.MultiWriter(stdout, os.Stderr)
+		}
+	}
+
 	tmpPath, err := ioutil.TempDir("", "gozk")
 	requireNoError(t, err, "failed to create tmp dir for test server setup")
 
@@ -53,9 +67,7 @@ func StartTestCluster(t *testing.T, size int, stdout, stderr io.Writer) (*TestCl
 
 	for serverN := 0; serverN < size; serverN++ {
 		srvPath := filepath.Join(tmpPath, fmt.Sprintf("srv%d", serverN+1))
-		if err := os.Mkdir(srvPath, 0700); err != nil {
-			requireNoError(t, err, "failed to make server path")
-		}
+		requireNoError(t, os.Mkdir(srvPath, 0700), "failed to make server path")
 
 		port := startPort + serverN*3
 		cfg := ServerConfig{

server_java.go

@@ -65,8 +65,8 @@ func (srv *server) Start() error {
 	srv.cmd = exec.CommandContext(ctx, srv.cmdString, srv.cmdArgs...)
 	srv.cmd.Stdout = srv.stdout
 	srv.cmd.Stderr = srv.stderr
-
 	srv.cmd.Env = srv.cmdEnv
+
 	return srv.cmd.Start()
 }