From 25f65e8bec73770beecc3e5f03eb0c6e98a9d2b3 Mon Sep 17 00:00:00 2001 From: Dariusz Koryto Date: Mon, 28 Sep 2026 13:48:43 +0200 Subject: [PATCH 1/3] Fix flaky e2e helpers and races in test code The daemon helper called cmd.Wait twice (once in the reaper goroutine, once in cancel), which the race detector flags. Cancel now waits on a channel closed by the reaper instead. The 500ms daemon startup timeout was too tight for the first run of a freshly linked binary on macOS and for -race builds, so bump it to 5s. Graceful termination is now polled instead of checked after a fixed 50ms sleep, and the keep-alive counter is read under its mutex. --- test/e2e/daemon_test.go | 9 ++++++--- test/e2e/helpers.go | 13 ++++++++++--- test/e2e/ssh_server.go | 6 ++++++ test/e2e/tunnels_test.go | 4 ++-- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/test/e2e/daemon_test.go b/test/e2e/daemon_test.go index 7c4efae..aa09e11 100644 --- a/test/e2e/daemon_test.go +++ b/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) } diff --git a/test/e2e/helpers.go b/test/e2e/helpers.go index 905ce22..a4dc60a 100644 --- a/test/e2e/helpers.go +++ b/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,20 +115,23 @@ 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 { diff --git a/test/e2e/ssh_server.go b/test/e2e/ssh_server.go index e9d09a0..09605b1 100644 --- a/test/e2e/ssh_server.go +++ b/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() diff --git a/test/e2e/tunnels_test.go b/test/e2e/tunnels_test.go index ca4008c..5357dc4 100644 --- a/test/e2e/tunnels_test.go +++ b/test/e2e/tunnels_test.go @@ -932,8 +932,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) } } From a251c4d645765ecb98c0472eb96bf3f6d9708175 Mon Sep 17 00:00:00 2001 From: Dariusz Koryto Date: Mon, 28 Sep 2026 14:18:16 +0200 Subject: [PATCH 2/3] Kill the daemon when the e2e helper gives up waiting for it daemonWithCancel returned an error on timeout but left the started process running, so a slow start leaked a daemon that outlived the test run. --- test/e2e/helpers.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/e2e/helpers.go b/test/e2e/helpers.go index a4dc60a..88afecd 100644 --- a/test/e2e/helpers.go +++ b/test/e2e/helpers.go @@ -137,6 +137,8 @@ func daemonWithCancel(env []string) (context.CancelFunc, error) { 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 { From 99e80844a114948aaee03b5ad1512019ac307fa2 Mon Sep 17 00:00:00 2001 From: Dariusz Koryto Date: Mon, 28 Sep 2026 15:12:49 +0200 Subject: [PATCH 3/3] Poll for tunnel state in the reconnect test instead of sleeping TestTunnelReconnect checked for the reconnecting state right after dropping the server's connections and slept a fixed 500ms before expecting the tunnel to be back. Both depend on how fast the daemon notices, and the test failed on slower runs such as a -race build on Linux. It now polls 'list' for each state with a 10s deadline. --- test/e2e/tunnels_test.go | 44 +++++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/test/e2e/tunnels_test.go b/test/e2e/tunnels_test.go index 5357dc4..1f7d857 100644 --- a/test/e2e/tunnels_test.go +++ b/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") }