Skip to content

Commit 87964ac

Browse files
authored
Reduce internal tty API surface to the only production-used helper (#6942)
The `golang.org/x/term` review found that `internal/tty` exposed two helpers with no production callers: `IsStdoutTerminal` and `StderrTerminalWidth`. This change trims the package to the single stderr TTY check actually used by the logger and aligns tests with that smaller surface. - **API cleanup** - Remove unused `internal/tty` helpers: - `IsStdoutTerminal()` - `StderrTerminalWidth()` - Keep `IsStderrTerminal()` as the only exported helper in the package. - **Test realignment** - Drop tests that existed only to cover the removed helpers. - Replace direct `golang.org/x/term` assertions in `internal/tty` tests with a package-level behavior test that swaps `os.Stderr` to a pipe and verifies it is not treated as a terminal. - **Result** - `internal/tty` now reflects actual usage in the codebase instead of carrying uncalled wrapper APIs. ```go // remaining package surface func IsStderrTerminal() bool { return term.IsTerminal(int(os.Stderr.Fd())) } ```
2 parents 1aeb8fb + d69bb4a commit 87964ac

2 files changed

Lines changed: 11 additions & 68 deletions

File tree

internal/tty/tty.go

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
// Package tty provides utilities for TTY (terminal) detection.
2-
// This package uses golang.org/x/term for TTY detection, which aligns with
3-
// modern Go best practices and the spinner library v1.23.1+ implementation.
42
package tty
53

64
import (
@@ -13,19 +11,3 @@ import (
1311
func IsStderrTerminal() bool {
1412
return term.IsTerminal(int(os.Stderr.Fd()))
1513
}
16-
17-
// IsStdoutTerminal returns true if stdout is connected to a terminal.
18-
func IsStdoutTerminal() bool {
19-
return term.IsTerminal(int(os.Stdout.Fd()))
20-
}
21-
22-
// StderrTerminalWidth returns the width of the terminal connected to stderr
23-
// and true if successful. Returns 0 and false if stderr is not a terminal or
24-
// the width cannot be determined.
25-
func StderrTerminalWidth() (int, bool) {
26-
width, _, err := term.GetSize(int(os.Stderr.Fd()))
27-
if err != nil || width <= 0 {
28-
return 0, false
29-
}
30-
return width, true
31-
}

internal/tty/tty_test.go

Lines changed: 11 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,8 @@ import (
66

77
"github.com/stretchr/testify/assert"
88
"github.com/stretchr/testify/require"
9-
"golang.org/x/term"
109
)
1110

12-
// TestIsStderrTerminal verifies the function agrees with the underlying
13-
// term.IsTerminal check for os.Stderr.
14-
func TestIsStderrTerminal(t *testing.T) {
15-
expected := term.IsTerminal(int(os.Stderr.Fd()))
16-
result := IsStderrTerminal()
17-
assert.Equal(t, expected, result, "IsStderrTerminal should match term.IsTerminal(stderr)")
18-
}
19-
2011
// TestIsStderrTerminal_ConsistentResult verifies that repeated calls return the same value,
2112
// confirming deterministic behavior with no side effects.
2213
func TestIsStderrTerminal_ConsistentResult(t *testing.T) {
@@ -35,47 +26,17 @@ func TestIsStderrTerminal_NotATerminalInCI(t *testing.T) {
3526
assert.False(t, IsStderrTerminal(), "stderr should not be a terminal in CI")
3627
}
3728

38-
// TestIsStdoutTerminal verifies the function agrees with the underlying
39-
// term.IsTerminal check for os.Stdout.
40-
func TestIsStdoutTerminal(t *testing.T) {
41-
expected := term.IsTerminal(int(os.Stdout.Fd()))
42-
result := IsStdoutTerminal()
43-
assert.Equal(t, expected, result, "IsStdoutTerminal should match term.IsTerminal(stdout)")
44-
}
45-
46-
// TestTermIsTerminal_PipeIsNotTerminal verifies that the underlying
47-
// term.IsTerminal correctly identifies a pipe as not a terminal. This
48-
// documents the invariant that IsStdoutTerminal and IsStderrTerminal rely on.
49-
func TestTermIsTerminal_PipeIsNotTerminal(t *testing.T) {
29+
// TestIsStderrTerminal_WithPipe verifies that a pipe-backed stderr is not treated as a terminal.
30+
func TestIsStderrTerminal_WithPipe(t *testing.T) {
31+
originalStderr := os.Stderr
5032
r, w, err := os.Pipe()
5133
require.NoError(t, err)
52-
defer r.Close()
53-
defer w.Close()
54-
assert.False(t, term.IsTerminal(int(r.Fd())), "pipe file descriptor should not be a terminal")
55-
assert.False(t, term.IsTerminal(int(w.Fd())), "pipe write-end should not be a terminal")
56-
}
57-
58-
// TestStderrTerminalWidth verifies that StderrTerminalWidth returns consistent
59-
// results and only reports success when stderr is a terminal.
60-
func TestStderrTerminalWidth(t *testing.T) {
61-
width, ok := StderrTerminalWidth()
62-
isTerminal := term.IsTerminal(int(os.Stderr.Fd()))
63-
if isTerminal {
64-
assert.True(t, ok, "should succeed when stderr is a terminal")
65-
assert.Greater(t, width, 0, "terminal width should be positive")
66-
} else {
67-
assert.False(t, ok, "should fail when stderr is not a terminal")
68-
assert.Equal(t, 0, width, "width should be 0 when not a terminal")
69-
}
70-
}
71-
72-
// TestStderrTerminalWidth_NotATerminalInCI verifies that width detection
73-
// returns false in CI where stderr is not a terminal.
74-
func TestStderrTerminalWidth_NotATerminalInCI(t *testing.T) {
75-
if os.Getenv("CI") == "" && os.Getenv("GITHUB_ACTIONS") == "" {
76-
t.Skip("Skipping CI-specific assertion: not running in a CI environment")
77-
}
78-
width, ok := StderrTerminalWidth()
79-
assert.False(t, ok, "should not detect terminal width in CI")
80-
assert.Equal(t, 0, width, "width should be 0 in CI")
34+
t.Cleanup(func() {
35+
os.Stderr = originalStderr
36+
require.NoError(t, r.Close())
37+
require.NoError(t, w.Close())
38+
})
39+
os.Stderr = w
40+
41+
assert.False(t, IsStderrTerminal(), "pipe-backed stderr should not be a terminal")
8142
}

0 commit comments

Comments
 (0)