Skip to content

[test-improver] Improve tests for tty package - #1632

Closed
github-actions[bot] wants to merge 1 commit into
mainfrom
test-improver/tty-container-test-improvements-11cd950d7850005b
Closed

github-actions[bot] wants to merge 1 commit into
mainfrom
test-improver/tty-container-test-improvements-11cd950d7850005b

Conversation

@github-actions

@github-actions github-actions Bot commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

Test Improvements: container_test.go

File Analyzed

  • Test File: internal/tty/container_test.go
  • Package: internal/tty
  • Lines of Code: 387 → 363 (net reduction of 24 lines, removing boilerplate)

Improvements Made

1. Better Testing Patterns

  • ✅ Added clearContainerEnv helper — extracts the repeated 6-line os.LookupEnv + defer save/restore block used 5 times in the file into a single reusable t.Cleanup-based helper
  • ✅ Adopted t.Setenv for all test cases that set RUNNING_IN_CONTAINER to a specific value — this is idiomatic Go since Go 1.17, provides automatic cleanup even on panic, and eliminates manual teardown
  • ✅ Per-subtest isolation in TestIsRunningInContainer_EdgeCases — moved t.Setenv() into each subtest instead of relying on a top-level defer shared across subtests (safer, enables potential future t.Parallel())

2. Increased Coverage

  • ✅ Added TestIsStderrTerminal — covers the previously untested IsStderrTerminal() function in tty.go. The function wraps term.IsTerminal for os.Stderr and had no test coverage. The new test verifies the function is deterministic (returns the same value on repeated calls) without panicking.

3. Cleaner & More Stable Tests

  • ✅ Eliminated repeated boilerplate — the 6-line save/restore pattern appeared 5 times; now replaced with clearContainerEnv(t) (1 line) or t.Setenv(...) (1 line)
  • ✅ Uses t.Helper() in clearContainerEnv for cleaner failure output
  • ✅ Better test isolation — TestIsRunningInContainer_EdgeCases subtests now each own their environment setup via t.Setenv, rather than sharing state through a top-level defer

Test Execution

All existing tests preserved. The changes are semantically equivalent for all existing test logic — t.Setenv and clearContainerEnv provide the same save/restore semantics as the original os.LookupEnv/defer pattern, but more idiomatically.

Why These Changes?

internal/tty/container_test.go was selected because:

  1. It contained a repeated 6-line manual env save/restore pattern (os.LookupEnv + defer) in 5 places — a clear candidate for t.Setenv and a helper
  2. The corresponding tty.go had an untested function (IsStderrTerminal) — a clear coverage gap
  3. These improvements are mechanical and safe, with no risk of changing test semantics

Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests

Generated by Test Improver

- Add clearContainerEnv helper to replace repeated 6-line env save/restore boilerplate
- Use t.Setenv() for env var setup in subtests (idiomatic, auto-cleanup)
- Use clearContainerEnv() for 'unset' cases (TestIsRunningInContainer_FileBasedDetection,
  TestIsRunningInContainer_Consistency, TestIsRunningInContainer_ConcurrentAccess,
  and per-subtest in TestIsRunningInContainer_EnvironmentVariable)
- Remove top-level env save/restore in TestIsRunningInContainer_EdgeCases by
  moving t.Setenv() into each subtest (proper test isolation)
- Add TestIsStderrTerminal to cover the previously untested IsStderrTerminal()
  function in tty.go

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox closed this Mar 6, 2026
@lpcox
lpcox deleted the test-improver/tty-container-test-improvements-11cd950d7850005b branch March 21, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant