From: Bryan C. Mills <bcmills@google.com>
Date: Wed, 16 Feb 2022 15:24:42 +0000 (-0500)
Subject: [release-branch.go1.18] os: eliminate arbitrary sleep in Kill tests
X-Git-Tag: go1.18rc1~1
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=e70ee9591464a41ea203ab9e71ba367ec9ea7ac3;p=gostls13.git

[release-branch.go1.18] os: eliminate arbitrary sleep in Kill tests

The test spawned a subprocess that arbitrarily slept for one second.
However, on some platforms, longer than one second may elapse between
starting the subprocess and sending the termination signal.

Instead, the subprocess now closes stdout and reads stdin until EOF,
eliminating the need for an arbitrary duration. (If the parent test
times out, the stdin pipe will break, so the subprocess still won't
leak forever.)

This also makes the test much faster in the typical case: since it
uses synchronization instead of sleeping, it can run as quickly as the
host OS can start and kill the process.

Updates #44131.

Change-Id: I9753571438380dc14fc3531efdaea84578a47fae
Reviewed-on: https://go-review.googlesource.com/c/go/+/386174
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit eaf040502b763a6f00dced35e4173c2ce90eb52f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/386196
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
---

diff --git a/src/os/os_test.go b/src/os/os_test.go
index 82ca6f987d..63427deb6e 100644
--- a/src/os/os_test.go
+++ b/src/os/os_test.go
@@ -28,6 +28,16 @@ import (
 	"time"
 )
 
+func TestMain(m *testing.M) {
+	if Getenv("GO_OS_TEST_DRAIN_STDIN") == "1" {
+		os.Stdout.Close()
+		io.Copy(io.Discard, os.Stdin)
+		Exit(0)
+	}
+
+	Exit(m.Run())
+}
+
 var dot = []string{
 	"dir_unix.go",
 	"env.go",
@@ -2259,9 +2269,18 @@ func testKillProcess(t *testing.T, processKiller func(p *Process)) {
 	testenv.MustHaveExec(t)
 	t.Parallel()
 
-	// Re-exec the test binary itself to emulate "sleep 1".
-	cmd := osexec.Command(Args[0], "-test.run", "TestSleep")
-	err := cmd.Start()
+	// Re-exec the test binary to start a process that hangs until stdin is closed.
+	cmd := osexec.Command(Args[0])
+	cmd.Env = append(os.Environ(), "GO_OS_TEST_DRAIN_STDIN=1")
+	stdout, err := cmd.StdoutPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	stdin, err := cmd.StdinPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = cmd.Start()
 	if err != nil {
 		t.Fatalf("Failed to start test process: %v", err)
 	}
@@ -2270,19 +2289,14 @@ func testKillProcess(t *testing.T, processKiller func(p *Process)) {
 		if err := cmd.Wait(); err == nil {
 			t.Errorf("Test process succeeded, but expected to fail")
 		}
+		stdin.Close() // Keep stdin alive until the process has finished dying.
 	}()
 
-	time.Sleep(100 * time.Millisecond)
-	processKiller(cmd.Process)
-}
+	// Wait for the process to be started.
+	// (It will close its stdout when it reaches TestMain.)
+	io.Copy(io.Discard, stdout)
 
-// TestSleep emulates "sleep 1". It is a helper for testKillProcess, so we
-// don't have to rely on an external "sleep" command being available.
-func TestSleep(t *testing.T) {
-	if testing.Short() {
-		t.Skip("Skipping in short mode")
-	}
-	time.Sleep(time.Second)
+	processKiller(cmd.Process)
 }
 
 func TestKillStartProcess(t *testing.T) {