Merge pull request #4945 from heyitsanthony/fix-exit-status

e2e, pkg/expect: distinguish between Stop and Close

e2e/etcd_test.go

@@ -403,7 +403,7 @@ func (epc *etcdProcessCluster) Close() (err error) {
 			continue
 		}
 		os.RemoveAll(p.cfg.dataDirPath)
-		if curErr := p.proc.Close(); curErr != nil {
+		if curErr := p.proc.Stop(); curErr != nil {
 			if err != nil {
 				err = fmt.Errorf("%v; %v", err, curErr)
 			} else {

e2e/etcdctlv3_test.go

@@ -15,7 +15,6 @@
 package e2e
 
 import (
-	"fmt"
 	"os"
 	"strconv"
 	"strings"
@@ -346,35 +345,36 @@ func ctlV3Get(cx ctlCtx, key string, kvs ...kv) error {
 }
 
 func ctlV3Watch(cx ctlCtx, key, value string) error {
-	cmdArgs := append(ctlV3PrefixArgs(cx.epc, cx.dialTimeout), "watch")
-	if !cx.interactive {
-		if cx.watchRevision > 0 {
-			cmdArgs = append(cmdArgs, "--rev", strconv.Itoa(cx.watchRevision))
-		}
-		cmdArgs = append(cmdArgs, key)
-		return spawnWithExpects(cmdArgs, key, value)
+	watchCmd := []string{"watch", key}
+	if cx.watchRevision > 0 {
+		watchCmd = append(watchCmd, "--rev", strconv.Itoa(cx.watchRevision))
 	}
-	cmdArgs = append(cmdArgs, "--interactive")
+
+	cmdArgs := ctlV3PrefixArgs(cx.epc, cx.dialTimeout)
+	if cx.interactive {
+		cmdArgs = append(cmdArgs, "watch", "--interactive")
+	} else {
+		cmdArgs = append(cmdArgs, watchCmd...)
+	}
+
 	proc, err := spawnCmd(cmdArgs)
 	if err != nil {
 		return err
 	}
-	watchLine := fmt.Sprintf("watch %s", key)
-	if cx.watchRevision > 0 {
-		watchLine = fmt.Sprintf("watch %s --rev %d", key, cx.watchRevision)
+
+	if cx.interactive {
+		if err = proc.Send(strings.Join(watchCmd, " ") + "\r"); err != nil {
+			return err
+		}
 	}
-	if err = proc.Send(watchLine + "\r"); err != nil {
+
+	if _, err = proc.Expect(key); err != nil {
 		return err
 	}
-	_, err = proc.Expect(key)
-	if err != nil {
+	if _, err = proc.Expect(value); err != nil {
 		return err
 	}
-	_, err = proc.Expect(value)
-	if err != nil {
-		return err
-	}
-	return proc.Close()
+	return proc.Stop()
 }
 
 type txnRequests struct {

pkg/expect/expect.go

@@ -95,20 +95,34 @@ func (ep *ExpectProcess) Expect(s string) (string, error) {
 	return "", ep.err
 }
 
-// Close waits for the expect process to close
-func (ep *ExpectProcess) Close() error {
+// Stop kills the expect process and waits for it to exit.
+func (ep *ExpectProcess) Stop() error { return ep.close(true) }
+
+// Close waits for the expect process to exit.
+func (ep *ExpectProcess) Close() error { return ep.close(false) }
+
+func (ep *ExpectProcess) close(kill bool) error {
 	if ep.cmd == nil {
-		return nil
+		return ep.err
 	}
-	ep.cmd.Process.Kill()
+	if kill {
+		ep.cmd.Process.Kill()
+	}
+
+	err := ep.cmd.Wait()
 	ep.ptyMu.Lock()
 	ep.fpty.Close()
 	ep.ptyMu.Unlock()
-	err := ep.cmd.Wait()
 	ep.wg.Wait()
-	if err != nil && strings.Contains(err.Error(), "signal:") {
-		// ignore signal errors; expected from pty
-		err = nil
+
+	if err != nil {
+		ep.err = err
+		if !kill && strings.Contains(err.Error(), "exit status") {
+			// non-zero exit code
+			err = nil
+		} else if kill && strings.Contains(err.Error(), "signal:") {
+			err = nil
+		}
 	}
 	ep.cmd = nil
 	return err

pkg/expect/expect_test.go

@@ -44,12 +44,13 @@ func TestSend(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer ep.Close()
 	if err := ep.Send("a\r"); err != nil {
 		t.Fatal(err)
 	}
-	_, eerr := ep.Expect("b")
-	if eerr != nil {
-		t.Fatal(eerr)
+	if _, err := ep.Expect("b"); err != nil {
+		t.Fatal(err)
+	}
+	if err := ep.Stop(); err != nil {
+		t.Fatal(err)
 	}
 }