Skip to content

Commit 74e48a2

Browse files
authored
fix(logger): attribute parallel logs to the correct test under go test -json (#1873)
Route stdout logging through t.Log when a *testing.T is available so go test -json attributes each line to the correct parallel test (#1871). Also teach terratest_log_parser to de-interleave the resulting indented, framework-decorated lines by the test name they embed.
1 parent 849c0c6 commit 74e48a2

4 files changed

Lines changed: 242 additions & 5 deletions

File tree

modules/core/logger/logger.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ var (
4444
// because there is no log output with t.Logf (e.g., CircleCI kills tests after 10 minutes of no log output). With
4545
// this log method, you get log output continuously.
4646
//
47+
// When a *testing.T is available, the log is emitted through t.Log rather than written to stdout directly, so that
48+
// `go test -json` attributes each line to the correct test even when tests run in parallel (see DoLog and issue
49+
// #1871). t.Log still streams immediately under `-v` on Go 1.14+, so the benefits above are preserved.
50+
//
4751
Terratest = New(terratestLogger{})
4852
// TestingT can be used to use Go's testing.T to log. If this is used, but no testing.T is provided, it will fallback
4953
// to Default.
@@ -109,6 +113,10 @@ func (testingT) Logf(t testing.TestingT, format string, args ...any) {
109113
type terratestLogger struct{}
110114

111115
func (terratestLogger) Logf(t testing.TestingT, format string, args ...any) {
116+
if h, ok := t.(helper); ok {
117+
h.Helper()
118+
}
119+
112120
DoLog(t, callDepthWrapped, os.Stdout, fmt.Sprintf(format, args...))
113121
}
114122

@@ -132,13 +140,52 @@ var MutexStdout sync.Mutex
132140
// DoLog logs the given arguments to the given writer, along with a timestamp and information about what test and file is
133141
// doing the logging.
134142
func DoLog(t testing.TestingT, callDepth int, writer io.Writer, args ...any) {
143+
if h, ok := t.(helper); ok {
144+
h.Helper()
145+
}
146+
135147
date := time.Now()
136148
prefix := fmt.Sprintf("%s %s %s:", t.Name(), date.Format(time.RFC3339), CallerPrefix(callDepth+1))
137149
allArgs := append([]any{prefix}, args...)
138150

151+
// When we would otherwise write to stdout and a *testing.T is available, route the line through t.Log instead.
152+
// This lets `go test -json` attribute each line to the correct test, which it cannot do for raw stdout writes made
153+
// by tests running in parallel: such writes bypass the framework's per-test output coordination, so the JSON runner
154+
// tags them with whichever test happened to be active, mixing up the output of parallel tests (issue #1871). An
155+
// explicit non-stdout writer (e.g. a bytes.Buffer) is always honored as-is.
156+
if writer == os.Stdout {
157+
if sink, ok := t.(logSink); ok && logViaTestingT(sink, allArgs...) {
158+
return
159+
}
160+
}
161+
139162
fmt.Fprintln(writer, allArgs...)
140163
}
141164

165+
// logSink is satisfied by *testing.T (and *testing.B / *testing.F). See DoLog for why terratest routes stdout logging
166+
// through it: doing so is what allows `go test -json` to attribute output to the right test under t.Parallel().
167+
type logSink interface {
168+
Log(args ...any)
169+
Helper()
170+
}
171+
172+
// logViaTestingT routes the given args through the testing.T's Log method, which formats them like fmt.Sprintln (the
173+
// same as fmt.Fprintln) and streams them under the testing framework so they are attributed to the right test. It
174+
// recovers if the test has already completed, since t.Log panics in that case, and reports false so the caller can fall
175+
// back to writing directly to stdout rather than crash or drop the line.
176+
func logViaTestingT(sink logSink, args ...any) (logged bool) {
177+
defer func() {
178+
if recover() != nil {
179+
logged = false
180+
}
181+
}()
182+
183+
sink.Helper()
184+
sink.Log(args...)
185+
186+
return true
187+
}
188+
142189
// CallerPrefix returns the file and line number information about the methods that called this method, based on the current
143190
// goroutine's stack. The argument callDepth is the number of stack frames to ascend, with 0 identifying the method
144191
// that called CallerPrefix, 1 identifying the method that called that method, and so on.

modules/core/logger/logger_test.go

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,52 @@ func TestCustomLogger(t *testing.T) {
5959
assert.Equal(t, "subtest log", c.logs[2])
6060
}
6161

62-
// TestLockedLog makes sure that Log and Logf which use stdout are thread-safe.
62+
// fakeTestingT implements testing.TestingT but is deliberately NOT a *testing.T and has no Log method, so logging
63+
// through it exercises DoLog's stdout fallback path. A real *testing.T is instead routed through t.Log (see DoLog).
64+
type fakeTestingT struct{ name string }
65+
66+
func (fakeTestingT) Fail() {}
67+
func (fakeTestingT) FailNow() {}
68+
func (fakeTestingT) Fatal(...any) {}
69+
func (fakeTestingT) Fatalf(string, ...any) {}
70+
func (fakeTestingT) Error(...any) {}
71+
func (fakeTestingT) Errorf(string, ...any) {}
72+
func (f fakeTestingT) Name() string { return f.name }
73+
func (fakeTestingT) Helper() {}
74+
75+
// spyTestingT satisfies both testing.TestingT and the logSink that DoLog routes stdout logging through, recording each
76+
// Log call so tests can assert on what was routed.
77+
type spyTestingT struct {
78+
fakeTestingT
79+
logged []string
80+
}
81+
82+
func (s *spyTestingT) Log(args ...any) { s.logged = append(s.logged, fmt.Sprintln(args...)) }
83+
84+
// TestDoLogRoutesThroughTestingT verifies that DoLog routes to t.Log when writing to stdout for a *testing.T (so that
85+
// `go test -json` attributes output to the correct test), while always honoring an explicit non-stdout writer.
86+
//
87+
//nolint:paralleltest // asserts on os.Stdout routing
88+
func TestDoLogRoutesThroughTestingT(t *testing.T) {
89+
// writer == os.Stdout with a testing.T-like sink: routed through Log, nothing written to the real stdout.
90+
spy := &spyTestingT{fakeTestingT: fakeTestingT{name: "TestApply1"}}
91+
logger.DoLog(spy, 1, os.Stdout, "routed-message")
92+
require.Len(t, spy.logged, 1)
93+
assert.Contains(t, spy.logged[0], "TestApply1")
94+
assert.Contains(t, spy.logged[0], "routed-message")
95+
96+
// An explicit non-stdout writer is always honored and never routed to the sink.
97+
var buf bytes.Buffer
98+
99+
spy2 := &spyTestingT{fakeTestingT: fakeTestingT{name: "TestApply2"}}
100+
101+
logger.DoLog(spy2, 1, &buf, "buffered-message")
102+
assert.Empty(t, spy2.logged)
103+
assert.Contains(t, buf.String(), "buffered-message")
104+
}
105+
106+
// TestLockedLog makes sure that Log which uses the stdout fallback path is thread-safe. It uses fakeTestingT (not a
107+
// *testing.T) so DoLog writes to stdout under MutexStdout rather than routing through t.Log.
63108
//
64109
//nolint:paralleltest // test modifies os.Stdout
65110
func TestLockedLog(t *testing.T) {
@@ -69,19 +114,21 @@ func TestLockedLog(t *testing.T) {
69114
os.Stdout = stdout
70115
})
71116

117+
ft := fakeTestingT{name: t.Name()}
118+
72119
data := []struct {
73120
fn func(string)
74121
name string
75122
}{
76123
{
77124
fn: func(s string) {
78-
logger.Log(t, s)
125+
logger.Log(ft, s)
79126
},
80127
name: "Log",
81128
},
82129
{
83130
fn: func(s string) {
84-
logger.Default.Logf(t, "%s", s)
131+
logger.Default.Logf(ft, "%s", s)
85132
},
86133
name: "Logf",
87134
},

modules/core/logger/parser/parser.go

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ var (
5656
regexStatus = regexp.MustCompile(`=== (RUN|PAUSE|CONT)\s+(.+)`)
5757
regexSummary = regexp.MustCompile(`(^FAIL$)|(^(ok|FAIL)\s+([^ ]+)\s+(?:(\d+\.\d+)s|\(cached\)|(\[\w+ failed]))(?:\s+coverage:\s+(\d+\.\d+)%\sof\sstatements(?:\sin\s.+)?)?$)`)
5858
regexPanic = regexp.MustCompile(`^panic:`)
59+
// regexIndentedTerratestLog matches a terratest log line emitted through t.Log: the testing framework indents it
60+
// and prepends its own "file.go:NN: " decoration, e.g. " apply_test.go:42: TestFoo 2006-01-02T15:04:05Z07:00
61+
// caller.go:7: message". It captures the embedded test name so parallel output can still be de-interleaved. The
62+
// "TestName <RFC3339 timestamp>" signature is what distinguishes a terratest line from an ordinary t.Log line.
63+
regexIndentedTerratestLog = regexp.MustCompile(`^\s+\S+:\d+:\s+(Test\S*)\s+\d{4}-\d{2}-\d{2}T`)
5964
)
6065

6166
// GetIndent takes a line and returns the indent string
@@ -109,6 +114,19 @@ func IsPanicLine(text string) bool {
109114
return regexPanic.MatchString(text)
110115
}
111116

117+
// IsIndentedTerratestLogLine checks whether a line is a terratest log line that was emitted through t.Log, i.e. indented
118+
// by the testing framework and prefixed with its "file.go:NN: " decoration. See regexIndentedTerratestLog.
119+
func IsIndentedTerratestLogLine(text string) bool {
120+
return regexIndentedTerratestLog.MatchString(text)
121+
}
122+
123+
// GetTestNameFromIndentedTerratestLogLine extracts the test name embedded in a terratest log line emitted through
124+
// t.Log. See regexIndentedTerratestLog.
125+
func GetTestNameFromIndentedTerratestLogLine(text string) string {
126+
m := regexIndentedTerratestLog.FindStringSubmatch(text)
127+
return m[1]
128+
}
129+
112130
// parseAndStoreTestOutput will take test log entries from terratest and aggregate the output by test. Takes advantage
113131
// of the fact that terratest logs are prefixed by the test name. This will store the broken out logs into files under
114132
// the outputDir, named by test name.
@@ -170,8 +188,8 @@ func parseAndStoreTestOutput(
170188
case strings.HasPrefix(data, "Test"):
171189
// Heuristic: `go test` will only execute test functions named `Test.*`, so we assume any line prefixed
172190
// with `Test` is a test output for a named test. Also assume that test output will be space delimited and
173-
// test names can't contain spaces (because they are function names).
174-
// This must be modified when `logger.DoLog` changes.
191+
// test names can't contain spaces (because they are function names). This handles un-indented terratest
192+
// output, i.e. the stdout fallback path in logger.DoLog (used when no *testing.T is available).
175193
vals := strings.Split(data, " ")
176194
testName := vals[0]
177195
previousTestName = testName
@@ -180,6 +198,19 @@ func parseAndStoreTestOutput(
180198
logger.Errorf("Error writing log for test %s: %s", testName, writeErr)
181199
}
182200

201+
case IsIndentedTerratestLogLine(data):
202+
// When a *testing.T is available, logger.DoLog emits through t.Log, so terratest lines are indented and
203+
// carry the framework's "file.go:NN: " decoration before the terratest prefix. The line still embeds its
204+
// owning test name, so we extract and attribute it directly. This is what preserves de-interleaving of
205+
// parallel tests: the `=== CONT` status lines alone cannot, since both tests resume up front and never
206+
// pause again, so previousTestName would otherwise misattribute every line to the last test resumed.
207+
testName := GetTestNameFromIndentedTerratestLogLine(data)
208+
previousTestName = testName
209+
210+
if writeErr := logWriter.WriteLog(logger, testName, data); writeErr != nil {
211+
logger.Errorf("Error writing log for test %s: %s", testName, writeErr)
212+
}
213+
183214
case isIndented && IsResultLine(data):
184215
// In a nested test result block, so collect the line into all the test results we have seen so far.
185216
for _, marker := range testResultMarkers {
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
package parser_test
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"strings"
7+
"testing"
8+
9+
"github.com/gruntwork-io/terratest/modules/core/v2/logger/parser"
10+
"github.com/sirupsen/logrus"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
func TestIsIndentedTerratestLogLine(t *testing.T) {
16+
t.Parallel()
17+
18+
testCases := []struct {
19+
name string
20+
in string
21+
out bool
22+
}{
23+
{
24+
name: "IndentedTerratestLine",
25+
in: " apply_test.go:42: TestFoo 2026-07-18T13:36:46-04:00 logger.go:81: applying",
26+
out: true,
27+
},
28+
{
29+
name: "IndentedSubtestLine",
30+
in: " apply_test.go:42: TestFoo/Sub1 2026-07-18T13:36:46-04:00 logger.go:81: applying",
31+
out: true,
32+
},
33+
{
34+
name: "UnindentedTerratestLine",
35+
in: "TestFoo 2026-07-18T13:36:46-04:00 logger.go:81: applying",
36+
out: false,
37+
},
38+
{
39+
name: "PlainTLogLine",
40+
in: " apply_test.go:42: some plain message",
41+
out: false,
42+
},
43+
{
44+
name: "IndentedResultLine",
45+
in: " --- PASS: TestFoo (0.02s)",
46+
out: false,
47+
},
48+
}
49+
50+
for _, testCase := range testCases {
51+
testCase := testCase
52+
t.Run(testCase.name, func(t *testing.T) {
53+
t.Parallel()
54+
assert.Equal(t, testCase.out, parser.IsIndentedTerratestLogLine(testCase.in))
55+
})
56+
}
57+
}
58+
59+
func TestGetTestNameFromIndentedTerratestLogLine(t *testing.T) {
60+
t.Parallel()
61+
62+
assert.Equal(t, "TestFoo", parser.GetTestNameFromIndentedTerratestLogLine(
63+
" apply_test.go:42: TestFoo 2026-07-18T13:36:46-04:00 logger.go:81: applying"))
64+
assert.Equal(t, "TestFoo/Sub1", parser.GetTestNameFromIndentedTerratestLogLine(
65+
" apply_test.go:42: TestFoo/Sub1 2026-07-18T13:36:46-04:00 logger.go:81: applying"))
66+
}
67+
68+
// TestSpawnParsersDeinterleavesTLogOutput verifies that the parser de-interleaves parallel-test output that terratest
69+
// now emits through t.Log (indented and decorated by the testing framework). Both tests resume up front, so the
70+
// `=== CONT` status lines cannot distinguish them; correct attribution relies on the test name embedded in each line.
71+
func TestSpawnParsersDeinterleavesTLogOutput(t *testing.T) {
72+
t.Parallel()
73+
74+
// Interleaved `go test -v` output (non-JSON) as produced after logging is routed through t.Log.
75+
sample := strings.Join([]string{
76+
"=== RUN TestParA",
77+
"=== PAUSE TestParA",
78+
"=== RUN TestParB",
79+
"=== PAUSE TestParB",
80+
"=== CONT TestParA",
81+
"=== CONT TestParB",
82+
" a_test.go:10: TestParA 2026-07-18T13:36:46-04:00 logger.go:81: MARKA payload 0",
83+
" b_test.go:20: TestParB 2026-07-18T13:36:46-04:00 logger.go:81: MARKB payload 0",
84+
" b_test.go:20: TestParB 2026-07-18T13:36:46-04:00 logger.go:81: MARKB payload 1",
85+
" a_test.go:10: TestParA 2026-07-18T13:36:46-04:00 logger.go:81: MARKA payload 1",
86+
" a_test.go:10: TestParA 2026-07-18T13:36:46-04:00 logger.go:81: MARKA payload 2",
87+
" b_test.go:20: TestParB 2026-07-18T13:36:46-04:00 logger.go:81: MARKB payload 2",
88+
"--- PASS: TestParA (0.02s)",
89+
"--- PASS: TestParB (0.02s)",
90+
"PASS",
91+
"ok \tpkg\t0.10s",
92+
"",
93+
}, "\n")
94+
95+
out := t.TempDir()
96+
parser.SpawnParsers(logrus.New(), strings.NewReader(sample), out)
97+
98+
readLog := func(test string) string {
99+
b, err := os.ReadFile(filepath.Join(out, test+".log"))
100+
require.NoError(t, err, "expected a log file for %s", test)
101+
102+
return string(b)
103+
}
104+
105+
a := readLog("TestParA")
106+
assert.Equal(t, 3, strings.Count(a, "MARKA"), "TestParA.log should contain all of its own lines")
107+
assert.NotContains(t, a, "MARKB", "TestParA.log should not contain TestParB output")
108+
109+
b := readLog("TestParB")
110+
assert.Equal(t, 3, strings.Count(b, "MARKB"), "TestParB.log should contain all of its own lines")
111+
assert.NotContains(t, b, "MARKA", "TestParB.log should not contain TestParA output")
112+
}

0 commit comments

Comments
 (0)