Minor cosmetic cleanups in the tests. Add missing error checks. Use Fatal instead of Error to avoid panics upon failure.

doc_test.go

@@ -7,17 +7,10 @@ import (
 	"testing"
 )
 
-// Will fill p from reader r
+// Will fill p from reader r.
 func readBytes(r io.Reader, p []byte) error {
-	bytesRead := 0
-	for bytesRead < len(p) {
-		n, err := r.Read(p[bytesRead:])
-		if err != nil {
-			return err
-		}
-		bytesRead = bytesRead + n
-	}
-	return nil
+	_, err := io.ReadFull(r, p)
+	return err
 }
 
 func TestOpen(t *testing.T) {
@@ -25,17 +18,15 @@ func TestOpen(t *testing.T) {
 
 	pty, tty, err := Open()
 	if err != nil {
-		t.Errorf("Unexpected error from Open: %s", err)
+		t.Fatalf("Unexpected error from Open: %s.", err)
 	}
 
-	err = tty.Close()
-	if err != nil {
-		t.Errorf("Unexpected error from tty Close: %s", err)
+	if err := tty.Close(); err != nil {
+		t.Errorf("Unexpected error from tty Close: %s.", err)
 	}
 
-	err = pty.Close()
-	if err != nil {
-		t.Errorf("Unexpected error from pty Close: %s", err)
+	if err := pty.Close(); err != nil {
+		t.Errorf("Unexpected error from pty Close: %s.", err)
 	}
 }
 
@@ -44,24 +35,22 @@ func TestName(t *testing.T) {
 
 	pty, tty, err := Open()
 	if err != nil {
-		t.Errorf("Unexpected error from Open: %s", err)
+		t.Fatalf("Unexpected error from Open: %s.", err)
 	}
 
 	// Check name isn't empty. There's variation on what exactly the OS calls these files.
 	if pty.Name() == "" {
-		t.Error("pty name was empty")
+		t.Error("Pty name was empty.")
 	}
 	if tty.Name() == "" {
-		t.Error("tty name was empty")
+		t.Error("Tty name was empty.")
 	}
 
-	err = tty.Close()
-	if err != nil {
+	if err := tty.Close(); err != nil {
 		t.Errorf("Unexpected error from tty Close: %s", err)
 	}
 
-	err = pty.Close()
-	if err != nil {
+	if err := pty.Close(); err != nil {
 		t.Errorf("Unexpected error from pty Close: %s", err)
 	}
 }
@@ -74,30 +63,30 @@ func TestOpenByName(t *testing.T) {
 
 	pty, tty, err := Open()
 	if err != nil {
-		t.Fatal(err)
+		t.Fatalf("Error: open: %s.", err)
 	}
-	defer pty.Close()
-	defer tty.Close()
+	defer func() { _ = pty.Close() }()
+	defer func() { _ = tty.Close() }()
 
-	ttyFile, err := os.OpenFile(tty.Name(), os.O_RDWR, 0600)
+	ttyFile, err := os.OpenFile(tty.Name(), os.O_RDWR, 0o600)
 	if err != nil {
-		t.Fatalf("Failed to open tty file: %v", err)
+		t.Fatalf("Failed to open tty file: %s.", err)
 	}
-	defer ttyFile.Close()
+	defer func() { _ = ttyFile.Close() }()
 
 	// Ensure we can write to the newly opened tty file and read on the pty.
 	text := []byte("ping")
 	n, err := ttyFile.Write(text)
 	if err != nil {
-		t.Errorf("Unexpected error from Write: %s", err)
+		t.Fatalf("Unexpected error from Write: %s.", err)
 	}
 	if n != len(text) {
-		t.Errorf("Unexpected count returned from Write, got %d expected %d", n, len(text))
+		t.Errorf("Unexpected count returned from Write, got %d expected %d.", n, len(text))
 	}
 
 	buffer := make([]byte, len(text))
 	if err := readBytes(pty, buffer); err != nil {
-		t.Errorf("Unexpected error from readBytes: %s", err)
+		t.Fatalf("Unexpected error from readBytes: %s.", err)
 	}
 	if !bytes.Equal(text, buffer) {
 		t.Errorf("Unexpected result returned from Read, got %v expected %v", buffer, text)
@@ -109,34 +98,32 @@ func TestGetsize(t *testing.T) {
 
 	pty, tty, err := Open()
 	if err != nil {
-		t.Errorf("Unexpected error from Open: %s", err)
+		t.Fatalf("Unexpected error from Open: %s.", err)
 	}
 
 	prows, pcols, err := Getsize(pty)
 	if err != nil {
-		t.Errorf("Unexpected error from Getsize: %s", err)
+		t.Errorf("Unexpected error from Getsize: %s.", err)
 	}
 
 	trows, tcols, err := Getsize(tty)
 	if err != nil {
-		t.Errorf("Unexpected error from Getsize: %s", err)
+		t.Errorf("Unexpected error from Getsize: %s.", err)
 	}
 
 	if prows != trows {
-		t.Errorf("pty rows != tty rows: %d != %d", prows, trows)
+		t.Errorf("pty rows != tty rows: %d != %d.", prows, trows)
 	}
 	if prows != trows {
-		t.Errorf("pty cols != tty cols: %d != %d", pcols, tcols)
+		t.Errorf("pty cols != tty cols: %d != %d.", pcols, tcols)
 	}
 
-	err = tty.Close()
-	if err != nil {
-		t.Errorf("Unexpected error from tty Close: %s", err)
+	if err := tty.Close(); err != nil {
+		t.Errorf("Unexpected error from tty Close: %s.", err)
 	}
 
-	err = pty.Close()
-	if err != nil {
-		t.Errorf("Unexpected error from pty Close: %s", err)
+	if err := pty.Close(); err != nil {
+		t.Errorf("Unexpected error from pty Close: %s.", err)
 	}
 }
 
@@ -145,40 +132,38 @@ func TestGetsizefull(t *testing.T) {
 
 	pty, tty, err := Open()
 	if err != nil {
-		t.Errorf("Unexpected error from Open: %s", err)
+		t.Fatalf("Unexpected error from Open: %s.", err)
 	}
 
 	psize, err := GetsizeFull(pty)
 	if err != nil {
-		t.Errorf("Unexpected error from GetsizeFull: %s", err)
+		t.Fatalf("Unexpected error from GetsizeFull: %s.", err)
 	}
 
 	tsize, err := GetsizeFull(tty)
 	if err != nil {
-		t.Errorf("Unexpected error from GetsizeFull: %s", err)
+		t.Fatalf("Unexpected error from GetsizeFull: %s.", err)
 	}
 
 	if psize.X != tsize.X {
-		t.Errorf("pty x != tty x: %d != %d", psize.X, tsize.X)
+		t.Errorf("pty x != tty x: %d != %d.", psize.X, tsize.X)
 	}
 	if psize.Y != tsize.Y {
-		t.Errorf("pty y != tty y: %d != %d", psize.Y, tsize.Y)
+		t.Errorf("pty y != tty y: %d != %d.", psize.Y, tsize.Y)
 	}
 	if psize.Rows != tsize.Rows {
-		t.Errorf("pty rows != tty rows: %d != %d", psize.Rows, tsize.Rows)
+		t.Errorf("pty rows != tty rows: %d != %d.", psize.Rows, tsize.Rows)
 	}
 	if psize.Cols != tsize.Cols {
-		t.Errorf("pty cols != tty cols: %d != %d", psize.Cols, tsize.Cols)
+		t.Errorf("pty cols != tty cols: %d != %d.", psize.Cols, tsize.Cols)
 	}
 
-	err = tty.Close()
-	if err != nil {
-		t.Errorf("Unexpected error from tty Close: %s", err)
+	if err := tty.Close(); err != nil {
+		t.Errorf("Unexpected error from tty Close: %s.", err)
 	}
 
-	err = pty.Close()
-	if err != nil {
-		t.Errorf("Unexpected error from pty Close: %s", err)
+	if err := pty.Close(); err != nil {
+		t.Errorf("Unexpected error from pty Close: %s.", err)
 	}
 }
 
@@ -187,12 +172,12 @@ func TestSetsize(t *testing.T) {
 
 	pty, tty, err := Open()
 	if err != nil {
-		t.Errorf("Unexpected error from Open: %s", err)
+		t.Fatalf("Unexpected error from Open: %s.", err)
 	}
 
 	psize, err := GetsizeFull(pty)
 	if err != nil {
-		t.Errorf("Unexpected error from GetsizeFull: %s", err)
+		t.Fatalf("Unexpected error from GetsizeFull: %s.", err)
 	}
 
 	psize.X = psize.X + 1
@@ -200,37 +185,34 @@ func TestSetsize(t *testing.T) {
 	psize.Rows = psize.Rows + 1
 	psize.Cols = psize.Cols + 1
 
-	err = Setsize(tty, psize)
-	if err != nil {
-		t.Errorf("Unexpected error from Setsize: %s", err)
+	if err := Setsize(tty, psize); err != nil {
+		t.Fatalf("Unexpected error from Setsize: %s", err)
 	}
 
 	tsize, err := GetsizeFull(tty)
 	if err != nil {
-		t.Errorf("Unexpected error from GetsizeFull: %s", err)
+		t.Fatalf("Unexpected error from GetsizeFull: %s.", err)
 	}
 
 	if psize.X != tsize.X {
-		t.Errorf("pty x != tty x: %d != %d", psize.X, tsize.X)
+		t.Errorf("pty x != tty x: %d != %d.", psize.X, tsize.X)
 	}
 	if psize.Y != tsize.Y {
-		t.Errorf("pty y != tty y: %d != %d", psize.Y, tsize.Y)
+		t.Errorf("pty y != tty y: %d != %d.", psize.Y, tsize.Y)
 	}
 	if psize.Rows != tsize.Rows {
-		t.Errorf("pty rows != tty rows: %d != %d", psize.Rows, tsize.Rows)
+		t.Errorf("pty rows != tty rows: %d != %d.", psize.Rows, tsize.Rows)
 	}
 	if psize.Cols != tsize.Cols {
-		t.Errorf("pty cols != tty cols: %d != %d", psize.Cols, tsize.Cols)
+		t.Errorf("pty cols != tty cols: %d != %d.", psize.Cols, tsize.Cols)
 	}
 
-	err = tty.Close()
-	if err != nil {
-		t.Errorf("Unexpected error from tty Close: %s", err)
+	if err := tty.Close(); err != nil {
+		t.Errorf("Unexpected error from tty Close: %s.", err)
 	}
 
-	err = pty.Close()
-	if err != nil {
-		t.Errorf("Unexpected error from pty Close: %s", err)
+	if err := pty.Close(); err != nil {
+		t.Errorf("Unexpected error from pty Close: %s.", err)
 	}
 }
 
@@ -239,7 +221,7 @@ func TestReadWriteText(t *testing.T) {
 
 	pty, tty, err := Open()
 	if err != nil {
-		t.Errorf("Unexpected error from Open: %s", err)
+		t.Fatalf("Unexpected error from Open: %s.", err)
 	}
 
 	// Write to tty, read from pty
@@ -249,16 +231,15 @@ func TestReadWriteText(t *testing.T) {
 		t.Errorf("Unexpected error from Write: %s", err)
 	}
 	if n != len(text) {
-		t.Errorf("Unexpected count returned from Write, got %d expected %d", n, len(text))
+		t.Errorf("Unexpected count returned from Write, got %d expected %d.", n, len(text))
 	}
 
 	buffer := make([]byte, 4)
-	err = readBytes(pty, buffer)
-	if err != nil {
-		t.Errorf("Unexpected error from readBytes: %s", err)
+	if err := readBytes(pty, buffer); err != nil {
+		t.Fatalf("Unexpected error from readBytes: %s.", err)
 	}
 	if !bytes.Equal(text, buffer) {
-		t.Errorf("Unexpected result returned from Read, got %v expected %v", buffer, text)
+		t.Errorf("Unexpected result returned from Read, got %v expected %v.", buffer, text)
 	}
 
 	// Write to pty, read from tty.
@@ -266,41 +247,41 @@ func TestReadWriteText(t *testing.T) {
 	text = []byte("pong\n")
 	n, err = pty.Write(text)
 	if err != nil {
-		t.Errorf("Unexpected error from Write: %s", err)
+		t.Fatalf("Unexpected error from Write: %s.", err)
 	}
 	if n != len(text) {
-		t.Errorf("Unexpected count returned from Write, got %d expected %d", n, len(text))
+		t.Errorf("Unexpected count returned from Write, got %d expected %d.", n, len(text))
 	}
 
 	buffer = make([]byte, 5)
 	err = readBytes(tty, buffer)
 	if err != nil {
-		t.Errorf("Unexpected error from readBytes: %s", err)
+		t.Errorf("Unexpected error from readBytes: %s.", err)
 	}
 	expect := []byte("pong\n")
 	if !bytes.Equal(expect, buffer) {
-		t.Errorf("Unexpected result returned from Read, got %v expected %v", buffer, expect)
+		t.Errorf("Unexpected result returned from Read, got %v expected %v.", buffer, expect)
 	}
 
 	// Read the echo back from pty
 	buffer = make([]byte, 5)
 	err = readBytes(pty, buffer)
 	if err != nil {
-		t.Errorf("Unexpected error from readBytes: %s", err)
+		t.Fatalf("Unexpected error from readBytes: %s.", err)
 	}
 	expect = []byte("pong\r")
 	if !bytes.Equal(expect, buffer) {
-		t.Errorf("Unexpected result returned from Read, got %v expected %v", buffer, expect)
+		t.Errorf("Unexpected result returned from Read, got %v expected %v.", buffer, expect)
 	}
 
 	err = tty.Close()
 	if err != nil {
-		t.Errorf("Unexpected error from tty Close: %s", err)
+		t.Errorf("Unexpected error from tty Close: %s.", err)
 	}
 
 	err = pty.Close()
 	if err != nil {
-		t.Errorf("Unexpected error from pty Close: %s", err)
+		t.Errorf("Unexpected error from pty Close: %s.", err)
 	}
 }
 
@@ -309,66 +290,64 @@ func TestReadWriteControls(t *testing.T) {
 
 	pty, tty, err := Open()
 	if err != nil {
-		t.Errorf("Unexpected error from Open: %s", err)
+		t.Fatalf("Unexpected error from Open: %s.", err)
 	}
 
-	// Write the start of a line to pty
+	// Write the start of a line to pty.
 	text := []byte("pind")
 	n, err := pty.Write(text)
 	if err != nil {
-		t.Errorf("Unexpected error from Write: %s", err)
+		t.Fatalf("Unexpected error from Write: %s.", err)
 	}
 	if n != len(text) {
-		t.Errorf("Unexpected count returned from Write, got %d expected %d", n, len(text))
+		t.Errorf("Unexpected count returned from Write, got %d expected %d.", n, len(text))
 	}
 
-	// Backspace that last char
+	// Backspace that last char.
 	n, err = pty.Write([]byte("\b"))
 	if err != nil {
-		t.Errorf("Unexpected error from Write: %s", err)
+		t.Fatalf("Unexpected error from Write: %s.", err)
 	}
 	if n != 1 {
-		t.Errorf("Unexpected count returned from Write, got %d expected %d", n, 1)
+		t.Errorf("Unexpected count returned from Write, got %d expected %d.", n, 1)
 	}
 
-	// Write the correct char and a CR
+	// Write the correct char and a CR.
 	n, err = pty.Write([]byte("g\n"))
 	if err != nil {
-		t.Errorf("Unexpected error from Write: %s", err)
+		t.Fatalf("Unexpected error from Write: %s.", err)
 	}
 	if n != 2 {
-		t.Errorf("Unexpected count returned from Write, got %d expected %d", n, 2)
+		t.Errorf("Unexpected count returned from Write, got %d expected %d.", n, 2)
 	}
 
 	// Read the line
 	buffer := make([]byte, 7)
 	err = readBytes(tty, buffer)
 	if err != nil {
-		t.Errorf("Unexpected error from readBytes: %s", err)
+		t.Errorf("Unexpected error from readBytes: %s.", err)
 	}
 	expect := []byte("pind\bg\n")
 	if !bytes.Equal(expect, buffer) {
-		t.Errorf("Unexpected result returned from Read, got %v expected %v", buffer, expect)
+		t.Errorf("Unexpected result returned from Read, got %v expected %v.", buffer, expect)
 	}
 
 	// Read the echo back from pty
 	buffer = make([]byte, 7)
 	err = readBytes(pty, buffer)
 	if err != nil {
-		t.Errorf("Unexpected error from readBytes: %s", err)
+		t.Errorf("Unexpected error from readBytes: %s.", err)
 	}
 	expect = []byte("pind^Hg")
 	if !bytes.Equal(expect, buffer) {
-		t.Errorf("Unexpected result returned from Read, got %v expected %v", buffer, expect)
+		t.Errorf("Unexpected result returned from Read, got %v expected %v.", buffer, expect)
 	}
 
-	err = tty.Close()
-	if err != nil {
-		t.Errorf("Unexpected error from tty Close: %s", err)
+	if err := tty.Close(); err != nil {
+		t.Errorf("Unexpected error from tty Close: %s.", err)
 	}
 
-	err = pty.Close()
-	if err != nil {
-		t.Errorf("Unexpected error from pty Close: %s", err)
+	if err := pty.Close(); err != nil {
+		t.Errorf("Unexpected error from pty Close: %s.", err)
 	}
 }

io_test.go

@@ -4,13 +4,12 @@
 package pty
 
 import (
-	"testing"
-
 	"context"
 	"errors"
 	"os"
 	"runtime"
 	"sync"
+	"testing"
 	"time"
 )
 
@@ -19,7 +18,7 @@ const (
 	timeout        = time.Second
 )
 
-var mu sync.Mutex
+var glTestFdLock sync.Mutex
 
 // Check that SetDeadline() works for ptmx.
 // Outstanding Read() calls must be interrupted by deadline.
@@ -28,25 +27,27 @@ var mu sync.Mutex
 func TestReadDeadline(t *testing.T) {
 	ptmx, success := prepare(t)
 
-	err := ptmx.SetDeadline(time.Now().Add(timeout / 10))
-	if err != nil {
+	if err := ptmx.SetDeadline(time.Now().Add(timeout / 10)); err != nil {
 		if errors.Is(err, os.ErrNoDeadline) {
-			t.Skipf("deadline is not supported on %s/%s", runtime.GOOS, runtime.GOARCH)
+			t.Skipf("Deadline is not supported on %s/%s.", runtime.GOOS, runtime.GOARCH)
 		} else {
-			t.Fatalf("error: set deadline: %v\n", err)
+			t.Fatalf("Error: set deadline: %s.", err)
 		}
 	}
 
-	var buf = make([]byte, 1)
+	buf := make([]byte, 1)
 	n, err := ptmx.Read(buf)
 	success()
+	if err != nil && !errors.Is(err, os.ErrDeadlineExceeded) {
+		t.Fatalf("Unexpected read error: %s.", err)
+	}
 
 	if n != 0 && buf[0] != errMarker {
-		t.Errorf("received unexpected data from pmtx (%d bytes): 0x%X; err=%v", n, buf, err)
+		t.Errorf("Received unexpected data from pmtx (%d bytes): 0x%X; err=%v.", n, buf, err)
 	}
 }
 
-// Check that ptmx.Close() interrupts outstanding ptmx.Read() calls
+// Check that ptmx.Close() interrupts outstanding ptmx.Read() calls.
 //
 // https://github.com/creack/pty/issues/114
 // https://github.com/creack/pty/issues/88
@@ -55,23 +56,27 @@ func TestReadClose(t *testing.T) {
 
 	go func() {
 		time.Sleep(timeout / 10)
-		err := ptmx.Close()
-		if err != nil {
-			t.Errorf("failed to close ptmx: %v", err)
+		if err := ptmx.Close(); err != nil {
+			t.Errorf("Failed to close ptmx: %s.", err)
 		}
 	}()
 
-	var buf = make([]byte, 1)
+	buf := make([]byte, 1)
 	n, err := ptmx.Read(buf)
 	success()
+	if err != nil && !errors.Is(err, os.ErrClosed) {
+		t.Fatalf("Unexpected read error: %s.", err)
+	}
 
 	if n != 0 && buf[0] != errMarker {
-		t.Errorf("received unexpected data from pmtx (%d bytes): 0x%X; err=%v", n, buf, err)
+		t.Errorf("Received unexpected data from pmtx (%d bytes): 0x%X; err=%v.", n, buf, err)
 	}
 }
 
 // Open pty and setup watchdogs for graceful and not so graceful failure modes
 func prepare(t *testing.T) (ptmx *os.File, done func()) {
+	t.Helper()
+
 	if runtime.GOOS == "darwin" {
 		t.Log("creack/pty uses blocking i/o on darwin intentionally:")
 		t.Log("> https://github.com/creack/pty/issues/52")
@@ -81,13 +86,13 @@ func prepare(t *testing.T) (ptmx *os.File, done func()) {
 	}
 
 	// Due to data race potential in (*os.File).Fd()
-	// we should never run these two tests in parallel
-	mu.Lock()
-	t.Cleanup(mu.Unlock)
+	// we should never run these two tests in parallel.
+	glTestFdLock.Lock()
+	t.Cleanup(glTestFdLock.Unlock)
 
 	ptmx, pts, err := Open()
 	if err != nil {
-		t.Fatalf("error: open: %v\n", err)
+		t.Fatalf("Error: open: %s.\n", err)
 	}
 	t.Cleanup(func() { _ = ptmx.Close() })
 	t.Cleanup(func() { _ = pts.Close() })
@@ -99,21 +104,21 @@ func prepare(t *testing.T) (ptmx *os.File, done func()) {
 		case <-ctx.Done():
 			// ptmx.Read() did not block forever, yay!
 		case <-time.After(timeout):
-			_, err := pts.Write([]byte{errMarker}) // unblock ptmx.Read()
-			if err != nil {
-				t.Errorf("failed to write to pts: %v", err)
+			if _, err := pts.Write([]byte{errMarker}); err != nil { // unblock ptmx.Read()
+				t.Errorf("Failed to write to pts: %s.", err)
 			}
-			t.Error("ptmx.Read() was not unblocked")
+			t.Error("ptmx.Read() was not unblocked.")
 			done() // cancel panic()
 		}
 	}()
 	go func() {
 		select {
 		case <-ctx.Done():
-			// Test has either failed or succeeded; it definitely did not hang
+			// Test has either failed or succeeded; it definitely did not hang.
 		case <-time.After(timeout * 10 / 9): // timeout +11%
-			panic("ptmx.Read() was not unblocked; avoid hanging forever") // just in case
+			panic("ptmx.Read() was not unblocked; avoid hanging forever.") // Just in case.
 		}
 	}()
+
 	return ptmx, done
 }