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.

Fixes #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>

diff --git a/src/os/os_test.go b/src/os/os_test.go
index 82ca6f987d4ad9715e4020db8a9063331238c173..63427deb6e01cfbd0f3b8051428ffd5f817f2141 100644 (file)
--- 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) {