cmd/go: make testterminal18153 a normal test

Currently, cmd/go's testterminal18153 is implemented as a special test
that doesn't run as part of cmd/go's regular tests. Because the test
requires stdout and stderr to be a terminal, it is currently run
directly by "dist test" so it can inherit the terminal of all.bash.

This has a few problems. It's yet another special case in dist test.
dist test also has to be careful to not apply its own buffering to
this test, so it can't run in parallel and it limits dist test's own
scheduler. It doesn't run as part of regular "go test", which means it
usually only gets coverage from running all.bash. And since we have to
skip it if all.bash wasn't run at a terminal, I'm sure it often gets
skipped even when running all.bash.

Fix all of this by rewriting this test to create its own PTY and
re-exec "go test" to check that PTY passes through go test. This makes
the test self-contained, so it can be a regular cmd/go test, we can
drop it from dist test, and it's not sensitive to the environment of
all.bash.

Preparation for #37486.
Updates #18153.

Change-Id: I6493dbb0143348e299718f6e311ac8a63f5d69c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/449503
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/cmd/dist/test.go

@@ -453,10 +453,6 @@ func (t *tester) registerRaceBenchTest(pkg string) {
 	})
 }
 
-// stdOutErrAreTerminals is defined in test_linux.go, to report
-// whether stdout & stderr are terminals.
-var stdOutErrAreTerminals func() bool
-
 func (t *tester) registerTests() {
 	// Fast path to avoid the ~1 second of `go list std cmd` when
 	// the caller lists specific tests to run. (as the continuous
@@ -591,29 +587,6 @@ func (t *tester) registerTests() {
 		}
 	}
 
-	// This test needs its stdout/stderr to be terminals, so we don't run it from cmd/go's tests.
-	// See issue 18153.
-	if goos == "linux" {
-		t.tests = append(t.tests, distTest{
-			name:    "cmd_go_test_terminal",
-			heading: "cmd/go terminal test",
-			fn: func(dt *distTest) error {
-				t.runPending(dt)
-				timelog("start", dt.name)
-				defer timelog("end", dt.name)
-				if !stdOutErrAreTerminals() {
-					fmt.Println("skipping terminal test; stdout/stderr not terminals")
-					return nil
-				}
-				cmd := exec.Command(gorootBinGo, "test")
-				setDir(cmd, filepath.Join(os.Getenv("GOROOT"), "src/cmd/go/testdata/testterminal18153"))
-				cmd.Stdout = os.Stdout
-				cmd.Stderr = os.Stderr
-				return cmd.Run()
-			},
-		})
-	}
-
 	// On the builders only, test that a moved GOROOT still works.
 	// Fails on iOS because CC_FOR_TARGET refers to clangwrap.sh
 	// in the unmoved GOROOT.

src/cmd/dist/test_linux.go

@@ -1,28 +0,0 @@
-// Copyright 2016 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-//go:build linux
-// +build linux
-
-package main
-
-import (
-	"syscall"
-	"unsafe"
-)
-
-const ioctlReadTermios = syscall.TCGETS
-
-// isTerminal reports whether fd is a terminal.
-func isTerminal(fd uintptr) bool {
-	var termios syscall.Termios
-	_, _, err := syscall.Syscall6(syscall.SYS_IOCTL, fd, ioctlReadTermios, uintptr(unsafe.Pointer(&termios)), 0, 0, 0)
-	return err == 0
-}
-
-func init() {
-	stdOutErrAreTerminals = func() bool {
-		return isTerminal(1) && isTerminal(2)
-	}
-}

src/cmd/go/terminal_test.go

@@ -0,0 +1,119 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main_test
+
+import (
+	"errors"
+	"internal/testenv"
+	"internal/testpty"
+	"io"
+	"os"
+	"testing"
+
+	"golang.org/x/term"
+)
+
+func TestTerminalPassthrough(t *testing.T) {
+	// Check that if 'go test' is run with a terminal connected to stdin/stdout,
+	// then the go command passes that terminal down to the test binary
+	// invocation (rather than, e.g., putting a pipe in the way).
+	//
+	// See issue 18153.
+	testenv.MustHaveGoBuild(t)
+
+	// Start with a "self test" to make sure that if we *don't* pass in a
+	// terminal, the test can correctly detect that. (cmd/go doesn't guarantee
+	// that it won't add a terminal in the middle, but that would be pretty weird.)
+	t.Run("pipe", func(t *testing.T) {
+		r, w, err := os.Pipe()
+		if err != nil {
+			t.Fatalf("pipe failed: %s", err)
+		}
+		defer r.Close()
+		defer w.Close()
+		stdout, stderr := runTerminalPassthrough(t, r, w)
+		if stdout {
+			t.Errorf("stdout is unexpectedly a terminal")
+		}
+		if stderr {
+			t.Errorf("stderr is unexpectedly a terminal")
+		}
+	})
+
+	// Now test with a read PTY.
+	t.Run("pty", func(t *testing.T) {
+		r, processTTY, err := testpty.Open()
+		if errors.Is(err, testpty.ErrNotSupported) {
+			t.Skipf("%s", err)
+		} else if err != nil {
+			t.Fatalf("failed to open test PTY: %s", err)
+		}
+		defer r.Close()
+		w, err := os.OpenFile(processTTY, os.O_RDWR, 0)
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer w.Close()
+		stdout, stderr := runTerminalPassthrough(t, r, w)
+		if !stdout {
+			t.Errorf("stdout is not a terminal")
+		}
+		if !stderr {
+			t.Errorf("stderr is not a terminal")
+		}
+	})
+}
+
+func runTerminalPassthrough(t *testing.T, r, w *os.File) (stdout, stderr bool) {
+	cmd := testenv.Command(t, testGo, "test", "-run=^$")
+	cmd.Env = append(cmd.Environ(), "GO_TEST_TERMINAL_PASSTHROUGH=1")
+	cmd.Stdout = w
+	cmd.Stderr = w
+	t.Logf("running %s", cmd)
+	err := cmd.Start()
+	if err != nil {
+		t.Fatalf("starting subprocess: %s", err)
+	}
+	w.Close()
+	// Read the subprocess output. The behavior of reading from a PTY after the
+	// child closes it is very strange (e.g., on Linux, read returns EIO), so we
+	// ignore errors as long as we get everything we need. We still try to read
+	// all of the output so we can report it in case of failure.
+	buf, err := io.ReadAll(r)
+	if len(buf) != 2 || !(buf[0] == '1' || buf[0] == 'X') || !(buf[1] == '2' || buf[1] == 'X') {
+		t.Errorf("expected exactly 2 bytes matching [1X][2X]")
+		if err != nil {
+			// An EIO here might be expected depending on OS.
+			t.Errorf("error reading from subprocess: %s", err)
+		}
+	}
+	err = cmd.Wait()
+	if err != nil {
+		t.Errorf("suprocess failed with: %s", err)
+	}
+	if t.Failed() {
+		t.Logf("subprocess output:\n%s", string(buf))
+		t.FailNow()
+	}
+	return buf[0] == '1', buf[1] == '2'
+}
+
+func init() {
+	if os.Getenv("GO_TEST_TERMINAL_PASSTHROUGH") == "" {
+		return
+	}
+
+	if term.IsTerminal(1) {
+		print("1")
+	} else {
+		print("X")
+	}
+	if term.IsTerminal(2) {
+		print("2")
+	} else {
+		print("X")
+	}
+	os.Exit(0)
+}

src/cmd/go/testdata/testterminal18153/terminal_test.go

@@ -1,40 +0,0 @@
-// Copyright 2016 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-//go:build linux
-// +build linux
-
-// This test is run by src/cmd/dist/test.go (cmd_go_test_terminal),
-// and not by cmd/go's tests. This is because this test requires
-// that it be called with its stdout and stderr being a terminal.
-// dist doesn't run `cmd/go test` against this test directory if
-// dist's stdout/stderr aren't terminals.
-//
-// See issue 18153.
-
-package p
-
-import (
-	"syscall"
-	"testing"
-	"unsafe"
-)
-
-const ioctlReadTermios = syscall.TCGETS
-
-// isTerminal reports whether fd is a terminal.
-func isTerminal(fd uintptr) bool {
-	var termios syscall.Termios
-	_, _, err := syscall.Syscall6(syscall.SYS_IOCTL, fd, ioctlReadTermios, uintptr(unsafe.Pointer(&termios)), 0, 0, 0)
-	return err == 0
-}
-
-func TestIsTerminal(t *testing.T) {
-	if !isTerminal(1) {
-		t.Errorf("stdout is not a terminal")
-	}
-	if !isTerminal(2) {
-		t.Errorf("stderr is not a terminal")
-	}
-}

src/internal/testpty/pty.go

@@ -2,26 +2,24 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-//go:build ((aix || dragonfly || freebsd || (linux && !android) || netbsd || openbsd) && cgo) || darwin
-
 // Package testpty is a simple pseudo-terminal package for Unix systems,
 // implemented by calling C functions via cgo.
 package testpty
 
 import (
+	"errors"
 	"fmt"
 	"os"
-	"syscall"
 )
 
 type PtyError struct {
 	FuncName    string
 	ErrorString string
-	Errno       syscall.Errno
+	Errno       error
 }
 
 func ptyError(name string, err error) *PtyError {
-	return &PtyError{name, err.Error(), err.(syscall.Errno)}
+	return &PtyError{name, err.Error(), err}
 }
 
 func (e *PtyError) Error() string {
@@ -30,7 +28,11 @@ func (e *PtyError) Error() string {
 
 func (e *PtyError) Unwrap() error { return e.Errno }
 
+var ErrNotSupported = errors.New("testpty.Open not implemented on this platform")
+
 // Open returns a control pty and the name of the linked process tty.
+//
+// If Open is not implemented on this platform, it returns ErrNotSupported.
 func Open() (pty *os.File, processTTY string, err error) {
 	return open()
 }

src/internal/testpty/pty_none.go

@@ -0,0 +1,13 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build !(cgo && (aix || dragonfly || freebsd || (linux && !android) || netbsd || openbsd)) && !darwin
+
+package testpty
+
+import "os"
+
+func open() (pty *os.File, processTTY string, err error) {
+	return nil, "", ErrNotSupported
+}