Fix/flaky e2e tests

test/e2e/daemon_test.go

@@ -65,9 +65,12 @@ func testDaemonLaunch(t *testing.T, env []string) string {
 		t.Fatalf("failed to kill daemon: %v", err)
 	}
 
-	// Finally check for graceful termination
-	time.Sleep(50 * time.Millisecond)
-
+	// Finally check for graceful termination. Poll instead of sleeping a
+	// fixed amount, since shutdown is slower under the race detector.
+	deadline := time.Now().Add(2 * time.Second)
+	for pidRunning(pid) && time.Now().Before(deadline) {
+		time.Sleep(10 * time.Millisecond)
+	}
 	if pidRunning(pid) {
 		t.Fatalf("pid %d running", pid)
 	}

test/e2e/helpers.go

@@ -20,6 +20,10 @@ const (
 	binary      = "../../boring.test"
 	cliTimeout  = 10 * time.Second
 	connTimeout = 5 * time.Second
+	// Generous on purpose: the first run of a freshly linked binary on
+	// macOS, or a binary built with -race, can take well over 500ms to
+	// start up.
+	daemonStartTimeout = 5 * time.Second
 )
 
 var testMsg = []byte("hello through tunnel")
@@ -111,25 +115,30 @@ func daemonWithCancel(env []string) (context.CancelFunc, error) {
 		return nil, err
 	}
 
-	// Prevent zombie processes
+	// Prevent zombie processes. Wait must only be called once, so
+	// cancel waits on the channel instead of calling it again.
+	exited := make(chan struct{})
 	go func() {
 		cmd.Wait()
+		close(exited)
 	}()
 
 	cancel := func() {
 		cmd.Process.Signal(syscall.SIGTERM)
-		cmd.Wait()
+		<-exited
 	}
 
 	// Wait for daemon to start
 	wait := time.NewTimer(0.)
 	waitTime := 2 * time.Millisecond
-	timeout := time.After(500 * time.Millisecond)
+	timeout := time.After(daemonStartTimeout)
 	sock := getEnv(env, "BORING_SOCK")
 
 	for {
 		select {
 		case <-timeout:
+			// Don't leave the process behind when giving up on it
+			cancel()
 			return nil, fmt.Errorf("daemon not responsive after timeout")
 		case <-wait.C:
 			if conn, err := net.Dial("unix", sock); err == nil {

test/e2e/ssh_server.go

@@ -314,6 +314,12 @@ func (s *sshServer) resetKeepAlives() {
 	s.keepAlives = 0
 }
 
+func (s *sshServer) getKeepAlives() int {
+	s.keepAliveMu.Lock()
+	defer s.keepAliveMu.Unlock()
+	return s.keepAlives
+}
+
 func (s *sshServer) incrementKeepAlives() {
 	s.keepAliveMu.Lock()
 	defer s.keepAliveMu.Unlock()

test/e2e/tunnels_test.go

@@ -537,6 +537,32 @@ func TestCloseGroup(t *testing.T) {
 	}
 }
 
+var openStatus = regexp.MustCompile(`^\d{2}m\d{2}s$`)
+
+// waitForStatus polls 'list' until the first tunnel's status satisfies ok.
+func waitForStatus(t *testing.T, env []string, desc string, ok func(string) bool) {
+	t.Helper()
+	deadline := time.Now().Add(10 * time.Second)
+	for {
+		c, out, err := cliCommand(env, "list")
+		if err != nil {
+			t.Fatalf("failed to run CLI command: %v", err)
+		}
+		if c != 0 {
+			t.Fatalf("exit code %d: %s", c, out)
+		}
+		lines := strings.Split(strings.TrimSpace(stripANSI(out)), "\n")
+		s := strings.Fields(lines[1])[0]
+		if ok(s) {
+			return
+		}
+		if time.Now().After(deadline) {
+			t.Fatalf("tunnel not %s, status is %q", desc, s)
+		}
+		time.Sleep(20 * time.Millisecond)
+	}
+}
+
 func makeListener(addr string) (net.Listener, error) {
 	l, err := net.Listen("tcp", addr)
 	if err != nil {
@@ -747,23 +773,13 @@ func TestTunnelReconnect(t *testing.T) {
 	server.pause()
 	server.closeAll()
 
-	// verify tunnel is in Reconn state
-	c, out, err = cliCommand(env, "list")
-	if err != nil {
-		t.Fatalf("failed to run CLI command: %v", err)
-	}
-	if c != 0 {
-		t.Fatalf("exit code %d: %s", c, out)
-	}
-	lines := strings.Split(strings.TrimSpace(stripANSI(out)), "\n")
-
-	if strings.Fields(lines[1])[0] != "reconn" {
-		t.Errorf("test tunnel not reconnecting in list: %s", out)
-	}
+	// The daemon notices the dropped connection asynchronously, so poll
+	// for the state changes instead of sleeping a fixed amount.
+	waitForStatus(t, env, "reconn", func(s string) bool { return s == "reconn" })
 
 	// Reconnect the server
 	server.resume()
-	time.Sleep(500 * time.Millisecond) // Plenty of time for reconnection
+	waitForStatus(t, env, "open", openStatus.MatchString)
 
 	testTunnel(t, "localhost:49711", "localhost:49712")
 }
@@ -932,8 +948,8 @@ func TestTunnelKeepAlive(t *testing.T) {
 	// keep-alive should be sent within a second for this tunnel
 	time.Sleep(1100 * time.Millisecond)
 
-	if server.keepAlives != 1 {
-		t.Fatalf("expected 1 keep-alive, got %d", server.keepAlives)
+	if n := server.getKeepAlives(); n != 1 {
+		t.Fatalf("expected 1 keep-alive, got %d", n)
 	}
 }