Skip to content

Commit 877d168

Browse files
Make cygwin gate tests non-vacuous via pure shouldStyle
Bugbot correctly flagged that the cygwin regression tests used non-terminal writers (temp files, bytes.Buffer), so IsTerminalWriter returned false before the TERM check was ever reached — they passed identically with or without the fix. Split the decision into a pure shouldStyle(noColor, term, isTerminalWriter) so tests can simulate a terminal writer and actually reach the NO_COLOR and TERM gates, and drop the vacuous tests in status_test.go and mdrender_test.go that asserted outcomes already guaranteed by the writer check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 8b3e335 commit 877d168

5 files changed

Lines changed: 47 additions & 92 deletions

File tree

cmd/entire/cli/interactive/interactive.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,20 +97,28 @@ func IsTerminalWriter(w io.Writer) bool {
9797
// legacy consoles that can't handle ANSI escapes are excluded, and otherwise
9898
// the answer is whether w is a terminal.
9999
func ShouldStyle(w io.Writer) bool {
100-
if os.Getenv("NO_COLOR") != "" {
100+
return shouldStyle(os.Getenv("NO_COLOR"), os.Getenv("TERM"), IsTerminalWriter(w))
101+
}
102+
103+
// shouldStyle is the pure decision behind ShouldStyle, split out so tests can
104+
// exercise the NO_COLOR/TERM gates with a simulated terminal writer — `go
105+
// test` never has a real one, so testing through ShouldStyle would
106+
// short-circuit on the terminal check and never reach the earlier gates.
107+
func shouldStyle(noColor, term string, isTerminalWriter bool) bool {
108+
if noColor != "" {
101109
return false
102110
}
103-
if termLacksANSI() {
111+
if termLacksANSI(term) {
104112
return false
105113
}
106-
return IsTerminalWriter(w)
114+
return isTerminalWriter
107115
}
108116

109-
// termLacksANSI reports whether the current TERM identifies a legacy console
110-
// that does not reliably handle ANSI escape sequences. The canonical case is
117+
// termLacksANSI reports whether term identifies a legacy console that does
118+
// not reliably handle ANSI escape sequences. The canonical case is
111119
// TERM=cygwin: writing the ESC byte (0x1B) ends up rendered as the CP437
112120
// glyph U+2190 LEFTWARDS ARROW ("←") instead of starting an SGR sequence, so
113121
// styled output appears as literal text like "←[32m●←[m" (see GH #1267).
114-
func termLacksANSI() bool {
115-
return os.Getenv("TERM") == "cygwin"
122+
func termLacksANSI(term string) bool {
123+
return term == "cygwin"
116124
}

cmd/entire/cli/interactive/interactive_test.go

Lines changed: 31 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -95,45 +95,44 @@ func TestIsTerminalWriter_Pipe(t *testing.T) {
9595
}
9696
}
9797

98-
func TestTermLacksANSI(t *testing.T) {
98+
// TestShouldStyle_Gates exercises the pure decision with a simulated
99+
// terminal writer (isTerminalWriter=true) so the NO_COLOR and TERM gates are
100+
// actually reached — `go test` has no real terminal, so calling ShouldStyle
101+
// directly would short-circuit on the terminal check and pass vacuously.
102+
// TERM=cygwin case is the regression test for GH #1267.
103+
func TestShouldStyle_Gates(t *testing.T) {
104+
t.Parallel()
99105
cases := []struct {
100-
term string
101-
want bool
106+
name string
107+
noColor string
108+
term string
109+
isTerminal bool
110+
want bool
102111
}{
103-
{"cygwin", true},
104-
{"xterm-256color", false},
105-
{"dumb", false},
106-
{"", false},
112+
{"terminal with ANSI-capable TERM", "", "xterm-256color", true, true},
113+
{"TERM=cygwin disables on a terminal", "", "cygwin", true, false},
114+
{"NO_COLOR disables on a terminal", "1", "xterm-256color", true, false},
115+
{"non-terminal writer disables", "", "xterm-256color", false, false},
116+
{"TERM=dumb defers to the terminal check", "", "dumb", true, true},
117+
{"empty TERM defers to the terminal check", "", "", true, true},
107118
}
108119
for _, c := range cases {
109-
t.Run(c.term, func(t *testing.T) {
110-
t.Setenv("TERM", c.term)
111-
if got := termLacksANSI(); got != c.want {
112-
t.Errorf("termLacksANSI() with TERM=%q = %v; want %v", c.term, got, c.want)
120+
t.Run(c.name, func(t *testing.T) {
121+
t.Parallel()
122+
if got := shouldStyle(c.noColor, c.term, c.isTerminal); got != c.want {
123+
t.Errorf("shouldStyle(%q, %q, %v) = %v; want %v",
124+
c.noColor, c.term, c.isTerminal, got, c.want)
113125
}
114126
})
115127
}
116128
}
117129

118-
// TestShouldStyle exercises the disabling gates; the enabling path needs a
119-
// real terminal writer, which `go test` doesn't provide.
120-
func TestShouldStyle(t *testing.T) {
121-
t.Run("NO_COLOR disables", func(t *testing.T) {
122-
t.Setenv("NO_COLOR", "1")
123-
if ShouldStyle(os.Stdout) {
124-
t.Error("ShouldStyle(os.Stdout) = true with NO_COLOR set; want false")
125-
}
126-
})
127-
t.Run("TERM=cygwin disables", func(t *testing.T) {
128-
t.Setenv("TERM", "cygwin")
129-
if ShouldStyle(os.Stdout) {
130-
t.Error("ShouldStyle(os.Stdout) = true with TERM=cygwin; want false")
131-
}
132-
})
133-
t.Run("non-terminal writer disables", func(t *testing.T) {
134-
t.Setenv("TERM", "xterm-256color")
135-
if ShouldStyle(&bytes.Buffer{}) {
136-
t.Error("ShouldStyle(*bytes.Buffer) = true; want false")
137-
}
138-
})
130+
// TestShouldStyle_ReadsEnv verifies the exported wrapper plumbs the process
131+
// env into the decision: NO_COLOR is the first gate, so it disables styling
132+
// regardless of whether stdout is a terminal.
133+
func TestShouldStyle_ReadsEnv(t *testing.T) {
134+
t.Setenv("NO_COLOR", "1")
135+
if ShouldStyle(os.Stdout) {
136+
t.Error("ShouldStyle(os.Stdout) = true with NO_COLOR set; want false")
137+
}
139138
}

cmd/entire/cli/mdrender/mdrender_test.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -107,21 +107,6 @@ func TestRenderForWriter_NoColorEnvForcesRaw(t *testing.T) {
107107
}
108108
}
109109

110-
// TERM=cygwin must fall back to raw markdown; see interactive.termLacksANSI.
111-
// Regression test for GH #1267.
112-
func TestRenderForWriter_TermCygwinForcesRaw(t *testing.T) {
113-
t.Setenv("TERM", "cygwin")
114-
115-
const md = "# Heading"
116-
out, err := mdrender.RenderForWriter(&bytes.Buffer{}, md)
117-
if err != nil {
118-
t.Fatalf("RenderForWriter: %v", err)
119-
}
120-
if out != md {
121-
t.Errorf("expected raw markdown when TERM=cygwin, got: %q", out)
122-
}
123-
}
124-
125110
// TestRender_EmptyInputDoesNotPanic verifies the renderer handles edge cases
126111
// (empty string, whitespace-only) without erroring.
127112
func TestRender_EmptyInputDoesNotPanic(t *testing.T) {

cmd/entire/cli/status_test.go

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -961,43 +961,6 @@ func TestShouldUseColor_RegularFile(t *testing.T) {
961961
}
962962
}
963963

964-
// TERM=cygwin must disable color; see interactive.termLacksANSI.
965-
// Regression test for GH #1267.
966-
func TestShouldUseColor_TermCygwinDisables(t *testing.T) {
967-
t.Setenv("TERM", "cygwin")
968-
969-
f, err := os.CreateTemp(t.TempDir(), "test")
970-
if err != nil {
971-
t.Fatal(err)
972-
}
973-
defer f.Close()
974-
975-
if shouldUseColor(f) {
976-
t.Error("shouldUseColor should be false when TERM=cygwin")
977-
}
978-
}
979-
980-
// formatSettingsStatusShort should not emit any ESC (0x1B) bytes when
981-
// TERM=cygwin. Regression test for GH #1267.
982-
func TestFormatSettingsStatusShort_TermCygwinNoEscapes(t *testing.T) {
983-
t.Setenv("TERM", "cygwin")
984-
985-
f, err := os.CreateTemp(t.TempDir(), "tty")
986-
if err != nil {
987-
t.Fatal(err)
988-
}
989-
defer f.Close()
990-
991-
sty := newStatusStyles(f)
992-
out := formatSettingsStatusShort(context.Background(), &EntireSettings{Enabled: true}, sty)
993-
if strings.ContainsRune(out, 0x1B) {
994-
t.Errorf("output contains ESC (0x1B) under TERM=cygwin: %q", out)
995-
}
996-
if strings.ContainsRune(out, '←') {
997-
t.Errorf("output contains U+2190 LEFTWARDS ARROW under TERM=cygwin: %q", out)
998-
}
999-
}
1000-
1001964
func TestNewStatusStyles_NonTTY(t *testing.T) {
1002965
t.Parallel()
1003966

cmd/entire/cli/uiform/uiform.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func IsAccessibleMode() bool {
2121

2222
// Theme returns Entire's standard huh theme.
2323
//
24-
//nolint:ireturn // huh.Theme is an interface in v2
24+
2525
func Theme() huh.Theme {
2626
return huh.ThemeFunc(huh.ThemeDracula)
2727
}

0 commit comments

Comments
 (0)