Add more test helpers, cleanup existing tests. Add editorconfig and golangci config, comply with golangci rules.

2026-03-31 02:27:08 +09:00 · 2023-10-31 10:46:02 -04:00
parent ccf8e7e6b6
commit 219f7e0db9
13 changed files with 531 additions and 206 deletions
--- a/.editorconfig
+++ b/.editorconfig
@@ -0,0 +1,54 @@
+root = true
+
+# Sane defaults.
+[*]
+# Always use unix end of line.
+end_of_line              = lf
+# Always insert a new line at the end of files.
+insert_final_newline     = true
+# Don't leave trailing whitespaces.
+trim_trailing_whitespace = true
+# Default to utf8 encoding.
+charset                  = utf-8
+# Space > tab for consistent aligns.
+indent_style             = space
+# Default to 2 spaces for indent/tabs.
+indent_size              = 2
+# Flag long lines.
+max_line_length          = 140
+
+# Explicitly define settings for commonly used files.
+
+[*.go]
+indent_style = tab
+indent_size  = 8
+
+[*.feature]
+indent_style = space
+indent_size  = 2
+
+[*.json]
+indent_style = space
+indent_size  = 2
+
+[*.{yml,yaml}]
+indent_style = space
+indent_size  = 2
+
+[*.tf]
+indent_style = space
+indent_size  = 2
+
+[*.md]
+# Don't check line lenghts in files.
+max_line_length = 0
+
+[{Makefile,*.mk}]
+indent_style = tab
+indent_size  = 8
+
+[{Dockerfile,Dockerfile.*}]
+indent_size = 4
+
+[*.sql]
+indent_size = 2
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -0,0 +1,324 @@
+---
+# Reference: https://golangci-lint.run/usage/configuration/
+run:
+  timeout: 5m
+  # modules-download-mode: vendor
+
+  # Include test files.
+  tests: true
+
+  skip-dirs: []
+
+  skip-files: []
+
+output:
+  # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number".
+  format: colored-line-number
+  print-issued-lines: true
+  print-linter-name: true
+
+# Linter specific settings. See below in the `linter.enable` section for details on what each linter is doing.
+linters-settings:
+  dogsled:
+    # Checks assignments with too many blank identifiers. Default is 2.
+    max-blank-identifiers: 2
+
+  dupl:
+    # Tokens count to trigger issue.
+    threshold: 150
+
+  errcheck:
+    # Report about not checking of errors in type assertions: `a := b.(MyStruct)`.
+    # Enabled as this is often overlooked by developers.
+    check-type-assertions: true
+    # Report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`.
+    # Disabled as we consider that if the developer did type `_`, it was on purpose.
+    # Note that while this isn't enforced by the linter, each and every case of ignored error should
+    # be accompanied with a comment explaining why that error is being discarded.
+    check-blank: false
+
+  exhaustive:
+    # Indicates that switch statements are to be considered exhaustive if a
+    # 'default' case is present, even if all enum members aren't listed in the
+    # switch.
+    default-signifies-exhaustive: false
+
+  funlen:
+    # funlen checks the number of lines/statements in a function.
+    # While is is always best to keep functions short for readability, maintainability and testing,
+    # the default are a bit too strict (60 lines / 40 statements), increase it to be more flexible.
+    lines: 160
+    statements: 70
+
+  # NOTE: We don't set `gci` for import order as it supports only one prefix. Use `goimports.local-prefixes` instead.
+
+  gocognit:
+    # Minimal code complexity to report, defaults to 30 in gocognit, defaults 10 in golangci.
+    # Use 15 as it allows for some flexibility while preventing too much complexity.
+    # NOTE: Similar to gocyclo.
+    min-complexity: 35
+
+  nestif:
+    # Minimal complexity of if statements to report.
+    min-complexity: 8
+
+  goconst:
+    # Minimal length of string constant.
+    min-len: 4
+    # Minimal occurrences count to trigger.
+    # Increase the default from 3 to 5 as small number of const usage can reduce readability instead of improving it.
+    min-occurrences: 5
+
+  gocritic:
+    # Which checks should be disabled; can't be combined with 'enabled-checks'.
+    # See https://go-critic.github.io/overview#checks-overview
+    # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run`
+    disabled-checks:
+      - hugeParam  # Very strict check on the size of variables being copied. Too strict for most developer.
+    # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint run` to see all tags and checks.
+    # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags".
+    enabled-tags:
+      - diagnostic
+      - style
+      - opinionated
+      - performance
+    settings:
+      rangeValCopy:
+        sizeThreshold: 1024  # Increase the allowed copied bytes in range.
+
+  cyclop:
+    max-complexity: 35
+
+  gocyclo:
+    # Similar check as gocognit.
+    # NOTE: We might be able to remove this linter as it is redundant with gocyclo. It is in golangci-lint, so we keep it for now.
+    min-complexity: 35
+
+  godot:
+    # Check all top-level comments, not only declarations.
+    check-all: true
+
+  gofmt:
+    # simplify code: gofmt with `-s` option.
+    simplify: true
+
+  # NOTE: the goheader settings are set per-project.
+
+  goimports:
+    # Put imports beginning with prefix after 3rd-party packages.
+    # It's a comma-separated list of prefixes.
+    local-prefixes: "github.com/creack/pty"
+
+  golint:
+    # Minimal confidence for issues, default is 0.8.
+    min-confidence: 0.8
+
+  gosimple:
+    # Select the Go version to target. The default is '1.13'.
+    go: "1.18"
+    # https://staticcheck.io/docs/options#checks
+    checks: ["all"]
+
+  gosec:
+
+  govet:
+    # Enable all available checks from go vet.
+    enable-all: false
+    # Report about shadowed variables.
+    check-shadowing: true
+
+  # NOTE: depguard is disabled as it is very slow and made redundant by gomodguard.
+
+  lll:
+    # Make sure everyone is on the same level, fix the tab width to go's default.
+    tab-width: 8
+    # Increase the default max line length to give more flexibility. Forcing newlines can reduce readability instead of improving it.
+    line-length: 180
+
+  misspell:
+    locale: US
+    ignore-words:
+
+  nakedret:
+    # Make an issue if func has more lines of code than this setting and it has naked returns; default is 30.
+    # NOTE: Consider setting this to 1 to prevent naked returns.
+    max-func-lines: 30
+
+  nolintlint:
+    # Prevent ununsed directive to avoid stale comments.
+    allow-unused: false
+    # Require an explanation of nonzero length after each nolint directive.
+    require-explanation: true
+    # Exclude following linters from requiring an explanation.
+    # NOTE: It is strongly discouraged to put anything in there.
+    allow-no-explanation: []
+    # Enable to require nolint directives to mention the specific linter being suppressed. This ensurce the developer understand the reason being the error.
+    require-specific: true
+
+  prealloc:
+    # NOTE: For most programs usage of prealloc will be a premature optimization.
+    #       Keep thing simple, pre-alloc what is obvious and profile the program for more complex scenarios.
+    #
+    simple: true       # Checkonly on simple loops that have no returns/breaks/continues/gotos in them.
+    range-loops: true  # Check range loops, true by default
+    for-loops: false   # Check suggestions on for loops, false by default
+
+  rowserrcheck:
+    packages: []
+
+  staticcheck:
+    # Select the Go version to target. The default is '1.13'.
+    go: "1.18"
+    # https://staticcheck.io/docs/options#checks
+    checks: ["all"]
+
+  stylecheck:
+    # Select the Go version to target. The default is '1.13'.
+    go: "1.18"
+    # https://staticcheck.io/docs/options#checks
+    checks: ["all"]  # "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022"]
+
+  tagliatelle:
+    # Check the struck tag name case.
+    case:
+      # Use the struct field name to check the name of the struct tag.
+      use-field-name: false
+      rules:
+        # Any struct tag type can be used.
+        # support string case: `camel`, `pascal`, `kebab`, `snake`, `goCamel`, `goPascal`, `goKebab`, `goSnake`, `upper`, `lower`
+        json: snake
+        firestore: camel
+        yaml: camel
+        xml: camel
+        bson: camel
+        avro: snake
+        mapstructure: kebab
+        envconfig: upper
+
+  unparam:
+    # Don't create an error if an exported code have static params being used. It is often expected in libraries.
+    # NOTE: It would be nice if this linter would differentiate between a main package and a lib.
+    check-exported: true
+
+  unused: {}
+
+  whitespace:
+    multi-if: false    # Enforces newlines (or comments) after every multi-line if statement
+    multi-func: false  # Enforces newlines (or comments) after every multi-line function signature
+
+# Run `golangci-lint help linters` to get the full list of linter with their description.
+linters:
+  disable-all: true
+  # NOTE: enable-all is deprecated because too  many people don't pin versions...
+  # We still require explicit documentation on why some linters are disabled.
+  # disable:
+  #   - depguard          # Go linter that checks if package imports are in a list of acceptable packages [fast: true, auto-fix: false]
+  #   - exhaustivestruct  # Checks if all struct's fields are initialized [fast: true, auto-fix: false]
+  #   - forbidigo         # Forbids identifiers [fast: true, auto-fix: false]
+  #   - gci               # Gci control golang package import order and make it always deterministic. [fast: true, auto-fix: true]
+  #   - godox             # Tool for detection of FIXME, TODO and other comment keywords [fast: true, auto-fix: false]
+  #   - goerr113          # Golang linter to check the errors handling expressions [fast: true, auto-fix: false]
+  #   - golint            # Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes [fast: false, auto-fix: false]
+  #   - gomnd             # An analyzer to detect magic numbers. [fast: true, auto-fix: false]
+  #   - gomoddirectives   # Manage the use of 'replace', 'retract', and 'excludes' directives in go.mod. [fast: true, auto-fix: false]
+  #   - interfacer        # Linter that suggests narrower interface types [fast: false, auto-fix: false]
+  #   - maligned          # Tool to detect Go structs that would take less memory if their fields were sorted [fast: false, auto-fix: false]
+  #   - nlreturn          # nlreturn checks for a new line before return and branch statements to increase code clarity [fast: true, auto-fix: false]
+  #   - scopelint         # Scopelint checks for unpinned variables in go programs [fast: true, auto-fix: false]
+  #   - wrapcheck         # Checks that errors returned from external packages are wrapped [fast: false, auto-fix: false]
+  #   - wsl               # Whitespace Linter - Forces you to use empty lines! [fast: true, auto-fix: false]
+
+  # disable-reasons:
+  #   - depguard          # Checks whitelisted/blacklisted import path, but runs way too slow. Not that useful.
+  #   - exhaustivestruct  # Good concept, but not mature enough (errors on not assignable fields like locks) and too noisy when using AWS SDK as most fields are unused.
+  #   - forbidigo         # Great idea, but too strict out of the box. Probably will re-enable soon.
+  #   - gci               # Conflicts with goimports/gofumpt.
+  #   - godox             # Don't fail when finding TODO, FIXME, etc.
+  #   - goerr113          # Too many false positives.
+  #   - golint            # Deprecated (since v1.41.0) due to: The repository of the linter has been archived by the owner.  Replaced by revive.
+  #   - gomnd             # Checks for magic numbers. Disabled due to too many false positives not configurable (03/01/2020 v1.23.7).
+  #   - gomoddirectives   # Doesn't support //nolint to whitelist.
+  #   - interfacer        # Deprecated (since v1.38.0) due to: The repository of the linter has been archived by the owner.
+  #   - maligned          # Deprecated (since v1.38.0) due to: The repository of the linter has been archived by the owner.  Replaced by govet 'fieldalignment'.
+  #   - nlreturn          # Actually reduces readability in most cases.
+  #   - scopelint         # Deprecated (since v1.39.0) due to: The repository of the linter has been deprecated by the owner.  Replaced by exportloopref.
+  #   - wrapcheck         # Good concept, but always warns for http coded errors. Need to re-enable and whitelist our error package.
+  #   - wsl               # Forces to add newlines around blocks. Lots of false positives, not that useful.
+
+  enable:
+    - asciicheck        # Simple linter to check that your code does not contain non-ASCII identifiers [fast: true, auto-fix: false]
+    - bodyclose         # checks whether HTTP response body is closed successfully [fast: false, auto-fix: false]
+    - cyclop            # checks function and package cyclomatic complexity [fast: false, auto-fix: false]
+    - dogsled           # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) [fast: true, auto-fix: false]
+    - dupl              # Tool for code clone detection [fast: true, auto-fix: false]
+    - durationcheck     # check for two durations multiplied together [fast: false, auto-fix: false]
+    - errcheck          # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases [fast: false, auto-fix: false]
+    - errname           # Checks that sentinel errors are prefixed with the `Err` and error types are suffixed with the `Error`. [fast: false, auto-fix: false]
+    - errorlint         # go-errorlint is a source code linter for Go software that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13. [fast: false, auto-fix: false]
+    - exhaustive        # check exhaustiveness of enum switch statements [fast: false, auto-fix: false]
+    - exportloopref     # checks for pointers to enclosing loop variables [fast: false, auto-fix: false]
+    - forcetypeassert   # finds forced type assertions [fast: true, auto-fix: false]
+    - funlen            # Tool for detection of long functions [fast: true, auto-fix: false]
+    - gochecknoglobals  # check that no global variables exist [fast: true, auto-fix: false]
+    - gochecknoinits    # Checks that no init functions are present in Go code [fast: true, auto-fix: false]
+    - gocognit          # Computes and checks the cognitive complexity of functions [fast: true, auto-fix: false]
+    - goconst           # Finds repeated strings that could be replaced by a constant [fast: true, auto-fix: false]
+    - gocritic          # Provides many diagnostics that check for bugs, performance and style issues. [fast: false, auto-fix: false]
+    - gocyclo           # Computes and checks the cyclomatic complexity of functions [fast: true, auto-fix: false]
+    - godot             # Check if comments end in a period [fast: true, auto-fix: true]
+    - gofmt             # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification [fast: true, auto-fix: true]
+    - gofumpt           # Gofumpt checks whether code was gofumpt-ed. [fast: true, auto-fix: true]
+    - goheader          # Checks is file header matches to pattern [fast: true, auto-fix: false]
+    - goimports         # Goimports does everything that gofmt does. Additionally it checks unused imports [fast: true, auto-fix: true]
+    - gomodguard        # Allow and block list linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations. [fast: true, auto-fix: false]
+    - goprintffuncname  # Checks that printf-like functions are named with `f` at the end [fast: true, auto-fix: false]
+    - gosec             # (gas): Inspects source code for security problems [fast: false, auto-fix: false]
+    - gosimple          # (megacheck): Linter for Go source code that specializes in simplifying a code [fast: false, auto-fix: false]
+    - govet             # (vet, vetshadow): Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: false, auto-fix: false]
+    - importas          # Enforces consistent import aliases [fast: false, auto-fix: false]
+    - ineffassign       # Detects when assignments to existing variables are not used [fast: true, auto-fix: false]
+    - lll               # Reports long lines [fast: true, auto-fix: false]
+    - makezero          # Finds slice declarations with non-zero initial length [fast: false, auto-fix: false]
+    - misspell          # Finds commonly misspelled English words in comments [fast: true, auto-fix: true]
+    - nakedret          # Finds naked returns in functions greater than a specified function length [fast: true, auto-fix: false]
+    - nestif            # Reports deeply nested if statements [fast: true, auto-fix: false]
+    - nilerr            # Finds the code that returns nil even if it checks that the error is not nil. [fast: false, auto-fix: false]
+    - noctx             # noctx finds sending http request without context.Context [fast: false, auto-fix: false]
+    - nolintlint        # Reports ill-formed or insufficient nolint directives [fast: true, auto-fix: false]
+    - paralleltest      # paralleltest detects missing usage of t.Parallel() method in your Go test [fast: true, auto-fix: false]
+    - prealloc          # Finds slice declarations that could potentially be preallocated [fast: true, auto-fix: false]
+    - predeclared       # find code that shadows one of Go's predeclared identifiers [fast: true, auto-fix: false]
+    - promlinter        # Check Prometheus metrics naming via promlint [fast: true, auto-fix: false]
+    - revive            # Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint. [fast: false, auto-fix: false]
+    # Disabled due to generic. Work in progress upstream.
+    # - rowserrcheck      # checks whether Err of rows is checked successfully [fast: false, auto-fix: false]
+    # Disabled due to generic. Work in progress upstream.
+    # - sqlclosecheck     # Checks that sql.Rows and sql.Stmt are closed. [fast: false, auto-fix: false]
+    - staticcheck       # (megacheck): Staticcheck is a go vet on steroids, applying a ton of static analysis checks [fast: false, auto-fix: false]
+    - stylecheck        # Stylecheck is a replacement for golint [fast: false, auto-fix: false]
+    # Disabled due to generic. Work in progress upstream.
+    # - tagliatelle       # Checks the struct tags. [fast: true, auto-fix: false]
+    # - testpackage       # linter that makes you use a separate _test package [fast: true, auto-fix: false]
+    - thelper           # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers [fast: false, auto-fix: false]
+    - tparallel         # tparallel detects inappropriate usage of t.Parallel() method in your Go test codes [fast: false, auto-fix: false]
+    - typecheck         # Like the front-end of a Go compiler, parses and type-checks Go code [fast: false, auto-fix: false]
+    - unconvert         # Remove unnecessary type conversions [fast: false, auto-fix: false]
+    - unparam           # Reports unused function parameters [fast: false, auto-fix: false]
+    # Disabled due to way too many false positive in go1.20.
+    # - unused            # (megacheck): Checks Go code for unused constants, variables, functions and types [fast: false, auto-fix: false]
+    # Disabled due to generic. Work in progress upstream.
+    # - wastedassign      # wastedassign finds wasted assignment statements. [fast: false, auto-fix: false]
+    - whitespace        # Tool for detection of leading and trailing whitespace [fast: true, auto-fix: true]
+
+issues:
+  exclude:
+    # Allow shadowing of 'err'.
+    - 'shadow: declaration of "err" shadows declaration'
+    # Allow shadowing of `ctx`.
+    - 'shadow: declaration of "ctx" shadows declaration'
+  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
+  max-per-linter: 10
+  # Disable default excludes. Always be explicit on what we exclude.
+  exclude-use-default: false
+  # Exclude some linters from running on tests files.
+  exclude-rules: []
--- a/doc_test.go
+++ b/doc_test.go
@@ -1,39 +1,10 @@
 package pty

 import (
-	"bytes"
-	"io"
 	"os"
 	"testing"
 )

-// Will fill p from reader r.
-func readBytes(r io.Reader, p []byte) error {
-	_, err := io.ReadFull(r, p)
-	return err
-}
-
-// openCloses opens a pty/tty pair and stages the closing as part of the cleanup.
-func openClose(t *testing.T) (pty, tty *os.File) {
-	t.Helper()
-
-	pty, tty, err := Open()
-	if err != nil {
-		t.Fatalf("Unexpected error from Open: %s.", err)
-	}
-	t.Cleanup(func() {
-		if err := tty.Close(); err != nil {
-			t.Errorf("Unexpected error from tty Close: %s.", err)
-		}
-
-		if err := pty.Close(); err != nil {
-			t.Errorf("Unexpected error from pty Close: %s.", err)
-		}
-	})
-
-	return pty, tty
-}
-
 func TestOpen(t *testing.T) {
 	t.Parallel()

@@ -60,31 +31,22 @@ func TestName(t *testing.T) {
 func TestOpenByName(t *testing.T) {
 	t.Parallel()

-	pty, tty := openClose(t)
+	pty, tty := openClose(t) // Get the pty/tty pair.

+	// Manually open the tty from the exiting name.
 	ttyFile, err := os.OpenFile(tty.Name(), os.O_RDWR, 0o600)
-	if err != nil {
-		t.Fatalf("Failed to open tty file: %s.", err)
-	}
+	noError(t, err, "Failed to open tty file")
 	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.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))
-	}

-	buffer := make([]byte, len(text))
-	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)
-	}
+	n, err := ttyFile.Write(text)                                   // Write the text to the manually open tty.
+	noError(t, err, "Unexpected error from Write")                  // Make sure it didn't fail.
+	assert(t, len(text), n, "Unexpected count returned from Write") // Assert the number of bytes written.
+
+	buffer := readN(t, pty, len(text), "Unexpected error from pty Read")
+	assertBytes(t, text, buffer, "Unexpected result result returned from pty Read")
 }

 func TestGetsize(t *testing.T) {
@@ -93,50 +55,30 @@ func TestGetsize(t *testing.T) {
 	pty, tty := openClose(t)

 	prows, pcols, err := Getsize(pty)
-	if err != nil {
-		t.Errorf("Unexpected error from Getsize: %s.", err)
-	}
+	noError(t, err, "Unexpected error from pty Getsize")

 	trows, tcols, err := Getsize(tty)
-	if err != nil {
-		t.Errorf("Unexpected error from Getsize: %s.", err)
-	}
+	noError(t, err, "Unexpected error from tty Getsize")

-	if 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)
-	}
+	assert(t, prows, trows, "rows from Getsize on pty and tty should match")
+	assert(t, pcols, tcols, "cols from Getsize on pty and tty should match")
 }

-func TestGetsizefull(t *testing.T) {
+func TestGetsizeFull(t *testing.T) {
 	t.Parallel()

 	pty, tty := openClose(t)

 	psize, err := GetsizeFull(pty)
-	if err != nil {
-		t.Fatalf("Unexpected error from GetsizeFull: %s.", err)
-	}
+	noError(t, err, "Unexpected error from pty GetsizeFull")

 	tsize, err := GetsizeFull(tty)
-	if err != nil {
-		t.Fatalf("Unexpected error from GetsizeFull: %s.", err)
-	}
+	noError(t, err, "Unexpected error from tty GetsizeFull")

-	if 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)
-	}
-	if 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)
-	}
+	assert(t, psize.X, tsize.X, "X from GetsizeFull on pty and tty should match")
+	assert(t, psize.Y, tsize.Y, "Y from GetsizeFull on pty and tty should match")
+	assert(t, psize.Rows, tsize.Rows, "rows from GetsizeFull on pty and tty should match")
+	assert(t, psize.Cols, tsize.Cols, "cols from GetsizeFull on pty and tty should match")
 }

 func TestSetsize(t *testing.T) {
@@ -145,36 +87,22 @@ func TestSetsize(t *testing.T) {
 	pty, tty := openClose(t)

 	psize, err := GetsizeFull(pty)
-	if err != nil {
-		t.Fatalf("Unexpected error from GetsizeFull: %s.", err)
-	}
+	noError(t, err, "Unexpected error from pty GetsizeFull")

-	psize.X = psize.X + 1
-	psize.Y = psize.Y + 1
-	psize.Rows = psize.Rows + 1
-	psize.Cols = psize.Cols + 1
+	psize.X++
+	psize.Y++
+	psize.Rows++
+	psize.Cols++

-	if err := Setsize(tty, psize); err != nil {
-		t.Fatalf("Unexpected error from Setsize: %s", err)
-	}
+	noError(t, Setsize(tty, psize), "Unexpected error from Setsize")

 	tsize, err := GetsizeFull(tty)
-	if err != nil {
-		t.Fatalf("Unexpected error from GetsizeFull: %s.", err)
-	}
+	noError(t, err, "Unexpected error from tty GetsizeFull")

-	if 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)
-	}
-	if 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)
-	}
+	assert(t, psize.X, tsize.X, "Unexpected Getsize X result after Setsize")
+	assert(t, psize.Y, tsize.Y, "Unexpected Getsize Y result after Setsize")
+	assert(t, psize.Rows, tsize.Rows, "Unexpected Getsize Rows result after Setsize")
+	assert(t, psize.Cols, tsize.Cols, "Unexpected Getsize Cols result after Setsize")
 }

 func TestReadWriteText(t *testing.T) {
@@ -182,54 +110,35 @@ func TestReadWriteText(t *testing.T) {

 	pty, tty := openClose(t)

-	// Write to tty, read from pty
-	text := []byte("ping")
-	n, err := tty.Write(text)
-	if err != nil {
-		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))
-	}
+	// Write to tty, read from pty.
+	{
+		text := []byte("ping")

-	buffer := make([]byte, 4)
-	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)
+		n, err := tty.Write(text)
+		noError(t, err, "Unexpected error from tty Write")
+		assert(t, n, len(text), "Unexpected count returned from tty Write")
+
+		buffer := readN(t, pty, len(text), "Unexpected error from pty Read")
+		assertBytes(t, text, buffer, "Unexpected result returned from pty Read")
 	}

 	// Write to pty, read from tty.
 	// We need to send a \n otherwise this will block in the terminal driver.
-	text = []byte("pong\n")
-	n, err = pty.Write(text)
-	if err != nil {
-		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))
-	}
+	{
+		text := []byte("pong\n")

-	buffer = make([]byte, 5)
-	err = readBytes(tty, buffer)
-	if err != nil {
-		t.Fatalf("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)
-	}
+		n, err := pty.Write(text)
+		noError(t, err, "Unexpected error from pty Write")
+		assert(t, n, len(text), "Unexpected count returned from pty Write")

-	// Read the echo back from pty
-	buffer = make([]byte, 5)
-	err = readBytes(pty, buffer)
-	if err != nil {
-		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)
+		// Expect the raw text back when reading from tty.
+		buffer := readN(t, tty, len(text), "Unexpected error from tty Read")
+		assertBytes(t, text, buffer, "Unexpected result returned from tty Read")
+
+		// Read the echo back from pty. Expect LF to be CRLF.
+		expect := []byte("pong\r\n")
+		buffer = readN(t, pty, len(expect), "Unexpected error from pty Read")
+		assertBytes(t, expect, buffer, "Unexpected result returned from pty Read")
 	}
 }

@@ -239,52 +148,25 @@ func TestReadWriteControls(t *testing.T) {
 	pty, tty := openClose(t)

 	// Write the start of a line to pty.
-	text := []byte("pind")
-	n, err := pty.Write(text)
-	if err != nil {
-		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))
-	}
+	n, err := pty.WriteString("pind")                                    // Intentional typo.
+	noError(t, err, "Unexpected error from Write initial text")          // Make sure it didn't fail.
+	assert(t, 4, n, "Unexpected count returned from Write initial text") // Assert the number of bytes written.

 	// Backspace that last char.
-	n, err = pty.Write([]byte("\b"))
-	if err != nil {
-		t.Fatalf("Unexpected error from Write: %s.", err)
-	}
-	if n != 1 {
-		t.Errorf("Unexpected count returned from Write, got %d expected %d.", n, 1)
-	}
+	n, err = pty.WriteString("\b")                                    // "Remove" the typo.
+	noError(t, err, "Unexpected error from Write backspace")          // Make sure it didn't fail.
+	assert(t, 1, n, "Unexpected count returned from Write backspace") // Assert the number of bytes written.

-	// Write the correct char and a CR.
-	n, err = pty.Write([]byte("g\n"))
-	if err != nil {
-		t.Fatalf("Unexpected error from Write: %s.", err)
-	}
-	if n != 2 {
-		t.Errorf("Unexpected count returned from Write, got %d expected %d.", n, 2)
-	}
+	// Write the correct char and a LF.
+	n, err = pty.WriteString("g\n")                                    // Fix the typo.
+	noError(t, err, "Unexpected error from Write fixed text")          // Make sure it didn't fail.
+	assert(t, 2, n, "Unexpected count returned from Write fixed text") // Assert the number of bytes written.

-	// Read the line
-	buffer := make([]byte, 7)
-	err = readBytes(tty, buffer)
-	if err != nil {
-		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)
-	}
+	// Read the line.
+	buffer := readN(t, tty, 7, "Unexpected error from tty Read")
+	assertBytes(t, []byte("pind\bg\n"), buffer, "Unexpected result returned from tty Read")

-	// 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)
-	}
-	expect = []byte("pind^Hg")
-	if !bytes.Equal(expect, buffer) {
-		t.Errorf("Unexpected result returned from Read, got %v expected %v.", buffer, expect)
-	}
+	// Read the echo back from pty.
+	buffer = readN(t, pty, 9, "Unexpected error from pty Read")
+	assertBytes(t, []byte("pind^Hg\r\n"), buffer, "Unexpected result returned from pty Read")
 }
--- a/helpers_test.go
+++ b/helpers_test.go
@@ -0,0 +1,62 @@
+package pty
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"os"
+	"testing"
+)
+
+// openCloses opens a pty/tty pair and stages the closing as part of the cleanup.
+func openClose(t *testing.T) (pty, tty *os.File) {
+	t.Helper()
+
+	pty, tty, err := Open()
+	if err != nil {
+		t.Fatalf("Unexpected error from Open: %s.", err)
+	}
+	t.Cleanup(func() {
+		if err := tty.Close(); err != nil {
+			t.Errorf("Unexpected error from tty Close: %s.", err)
+		}
+
+		if err := pty.Close(); err != nil {
+			t.Errorf("Unexpected error from pty Close: %s.", err)
+		}
+	})
+
+	return pty, tty
+}
+
+func noError(t *testing.T, err error, msg string) {
+	t.Helper()
+	if err != nil {
+		t.Fatalf("%s: %s.", msg, err)
+	}
+}
+
+func assert[T comparable](t *testing.T, a, b T, msg string) {
+	t.Helper()
+	if a != b {
+		t.Errorf("%s: %v != %v.", msg, a, b)
+	}
+}
+
+// When asserting bytes, we want to display the diff, which may contain control characters.
+func assertBytes(t *testing.T, a, b []byte, msg string) {
+	t.Helper()
+	if !bytes.Equal(a, b) {
+		t.Errorf("%s: %v != %v.", msg, a, b)
+	}
+}
+
+// Read a specific number of bytes from r, assert it didn't fail and return the bytes.
+func readN(t *testing.T, r io.Reader, n int, msg string) []byte {
+	t.Helper()
+
+	buf := make([]byte, n)
+	_, err := io.ReadFull(r, buf)
+	noError(t, err, fmt.Sprintf("%s: %s", msg, err))
+	return buf
+}
--- a/io_test.go
+++ b/io_test.go
@@ -18,12 +18,15 @@ const (
 	timeout        = time.Second
 )

+//nolint:gochecknoglobals // Expected global lock to avodi potential race on (*os.File).Fd().
 var glTestFdLock sync.Mutex

 // Check that SetDeadline() works for ptmx.
 // Outstanding Read() calls must be interrupted by deadline.
 //
 // https://github.com/creack/pty/issues/162
+//
+//nolint:paralleltest // Potential in (*os.File).Fd().
 func TestReadDeadline(t *testing.T) {
 	ptmx, success := prepare(t)

@@ -51,6 +54,8 @@ func TestReadDeadline(t *testing.T) {
 //
 // https://github.com/creack/pty/issues/114
 // https://github.com/creack/pty/issues/88
+//
+//nolint:paralleltest // Potential in (*os.File).Fd().
 func TestReadClose(t *testing.T) {
 	ptmx, success := prepare(t)

@@ -73,7 +78,7 @@ func TestReadClose(t *testing.T) {
 	}
 }

-// Open pty and setup watchdogs for graceful and not so graceful failure modes
+// 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()

@@ -104,18 +109,18 @@ func prepare(t *testing.T) (ptmx *os.File, done func()) {
 		case <-ctx.Done():
 			// ptmx.Read() did not block forever, yay!
 		case <-time.After(timeout):
-			if _, err := pts.Write([]byte{errMarker}); err != nil { // unblock ptmx.Read()
+			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.")
-			done() // cancel panic()
+			done() // Cancel panic().
 		}
 	}()
 	go func() {
 		select {
 		case <-ctx.Done():
 			// Test has either failed or succeeded; it definitely did not hang.
-		case <-time.After(timeout * 10 / 9): // timeout +11%
+		case <-time.After(timeout * 10 / 9): // Timeout + 11%.
 			panic("ptmx.Read() was not unblocked; avoid hanging forever.") // Just in case.
 		}
 	}()
--- a/ioctl.go
+++ b/ioctl.go
@@ -8,13 +8,13 @@ import "os"
 func ioctl(f *os.File, cmd, ptr uintptr) error {
 	sc, e := f.SyscallConn()
 	if e != nil {
-		return ioctl_inner(f.Fd(), cmd, ptr) // fall back to blocking io (old behavior)
+		return ioctlInner(f.Fd(), cmd, ptr) // Fall back to blocking io (old behavior).
 	}

 	ch := make(chan error, 1)
 	defer close(ch)

-	e = sc.Control(func(fd uintptr) { ch <- ioctl_inner(fd, cmd, ptr) })
+	e = sc.Control(func(fd uintptr) { ch <- ioctlInner(fd, cmd, ptr) })
 	if e != nil {
 		return e
 	}
--- a/ioctl_inner.go
+++ b/ioctl_inner.go
@@ -5,12 +5,13 @@ package pty

 import "syscall"

+// Local syscall const values.
 const (
 	TIOCGWINSZ = syscall.TIOCGWINSZ
 	TIOCSWINSZ = syscall.TIOCSWINSZ
 )

-func ioctl_inner(fd, cmd, ptr uintptr) error {
+func ioctlInner(fd, cmd, ptr uintptr) error {
 	_, _, e := syscall.Syscall(syscall.SYS_IOCTL, fd, cmd, ptr)
 	if e != 0 {
 		return e
--- a/ioctl_legacy.go
+++ b/ioctl_legacy.go
@@ -6,5 +6,5 @@ package pty
 import "os"

 func ioctl(f *os.File, cmd, ptr uintptr) error {
-	return ioctl_inner(f.Fd(), cmd, ptr) // fall back to blocking io (old behavior)
+	return ioctlInner(f.Fd(), cmd, ptr) // fall back to blocking io (old behavior)
 }
--- a/ioctl_solaris.go
+++ b/ioctl_solaris.go
@@ -40,7 +40,7 @@ type strioctl struct {
 // Defined in asm_solaris_amd64.s.
 func sysvicall6(trap, nargs, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2 uintptr, err syscall.Errno)

-func ioctl_inner(fd, cmd, ptr uintptr) error {
+func ioctlInner(fd, cmd, ptr uintptr) error {
 	if _, _, errno := sysvicall6(uintptr(unsafe.Pointer(&procioctl)), 3, fd, cmd, ptr, 0, 0, 0); errno != 0 {
 		return errno
 	}
--- a/ioctl_unsupported.go
+++ b/ioctl_unsupported.go
@@ -8,6 +8,6 @@ const (
 	TIOCSWINSZ = 0
 )

-func ioctl_inner(fd, cmd, ptr uintptr) error {
+func ioctlInner(fd, cmd, ptr uintptr) error {
 	return ErrUnsupported
 }
--- a/pty_linux.go
+++ b/pty_linux.go
@@ -49,6 +49,6 @@ func ptsname(f *os.File) (string, error) {

 func unlockpt(f *os.File) error {
 	var u _C_int
-	// use TIOCSPTLCK with a pointer to zero to clear the lock
+	// use TIOCSPTLCK with a pointer to zero to clear the lock.
 	return ioctl(f, syscall.TIOCSPTLCK, uintptr(unsafe.Pointer(&u))) //nolint:gosec // Expected unsafe pointer for Syscall call.
 }
--- a/winsize.go
+++ b/winsize.go
@@ -10,10 +10,7 @@ func InheritSize(pty, tty *os.File) error {
 	if err != nil {
 		return err
 	}
-	if err := Setsize(tty, size); err != nil {
-		return err
-	}
-	return nil
+	return Setsize(tty, size)
 }

 // Getsize returns the number of rows (lines) and cols (positions
--- a/winsize_unix.go
+++ b/winsize_unix.go
@@ -11,10 +11,10 @@ import (

 // Winsize describes the terminal size.
 type Winsize struct {
-	Rows uint16 // ws_row: Number of rows (in cells)
-	Cols uint16 // ws_col: Number of columns (in cells)
-	X    uint16 // ws_xpixel: Width in pixels
-	Y    uint16 // ws_ypixel: Height in pixels
+	Rows uint16 // ws_row: Number of rows (in cells).
+	Cols uint16 // ws_col: Number of columns (in cells).
+	X    uint16 // ws_xpixel: Width in pixels.
+	Y    uint16 // ws_ypixel: Height in pixels.
 }

 // Setsize resizes t to s.