fix: eliminate data race in ci and job tests by avoiding global os.Stdout/os.Stderr mutation

Timo Furrerrequested to merge
duo/fix/3112-data-race-ci-job-tests into main

Description

Fix a data race in TestPipelineCmd and TestJobCmd that is exposed when running the test suite with the race detector (make test-race).

Root Cause

internal/commands/ci/ci_test.go and internal/commands/job/job_test.go were capturing command output by replacing the global os.Stdout and os.Stderr variables:

oldOut := os.Stdout
rOut, wOut, _ := os.Pipe()
os.Stdout = wOut   // ← writes to global
// ...
go func() {        // ← goroutine reads from pipe concurrently
    io.Copy(&buf, rOut)
}()

When the Go test runner executes tests from different packages concurrently (the default behaviour), this causes a data race: one goroutine writes to os.Stdout/os.Stderr while another goroutine in a different package reads them (e.g. via iostreams.IsTerminal(os.Stdout) or fmt.Fprintf(os.Stderr, ...)).

The race was pre-existing but is reliably surfaced by the make test-race job in the pipeline for !3112 (merged) (bumping go-isatty to v0.0.21).

Fix

  • Rewrite TestPipelineCmd and TestJobCmd to use cmdtest.TestIOStreams() and cobra's cmd.SetOut/cmd.SetErr to capture output without touching global state.
  • Fix ci.go to write the deprecation warning to cmd.ErrOrStderr() instead of os.Stderr directly, so the output is captured correctly in tests and respects any stderr override set on the command.

Related Issues

Related to !3112 (merged) (data race in tests:unit job).

How has this been tested?

The fix removes the global os.Stdout/os.Stderr mutation from the two test files. The tests now use the same cmdtest.TestIOStreams() pattern used throughout the rest of the test suite. The race detector should no longer flag these tests.

Merge request reports