Skip to content

Commit 0b8f2f1

Browse files
test: address actionable tier 2 refactors from issue 478 (#526)
## Summary - extract shared helpers in review command tests - move log rendering coverage into `internal/streamfmt` and add typed stream event builders - share fix-job and storage fixture helpers across daemon and storage tests - leave the older `ci_poller_test.go` tier 2 item alone because it no longer maps cleanly to the current code ## Commits - `7af09ff` `test: extract review command helpers` - `0b9775d` `test: move log rendering tests into streamfmt` - `68ecd1b` `test: use typed stream formatter events` - `73f27b4` `test: share fix job test helpers` - `7b7f67f` `test: add db job test fixture helper` - `6f0f1bd` `test: extract migration fixture schemas` ## Test Plan - `go fmt ./...` - `go vet ./...` - `go test ./...` Related to #478.
1 parent 1c61daa commit 0b8f2f1

File tree

7 files changed

+927
-921
lines changed

7 files changed

+927
-921
lines changed

cmd/roborev/log_cmd_test.go

Lines changed: 0 additions & 211 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package main
22

33
import (
44
"bytes"
5-
"encoding/json"
65
"fmt"
76
"os"
87
"path/filepath"
@@ -15,31 +14,6 @@ import (
1514
"github.com/stretchr/testify/require"
1615
)
1716

18-
func toJSON(v any) string {
19-
b, err := json.Marshal(v)
20-
if err != nil {
21-
panic(fmt.Sprintf("toJSON: %v", err))
22-
}
23-
return string(b)
24-
}
25-
26-
func executeRenderJobLog(t *testing.T, input string, isTTY bool) string {
27-
t.Helper()
28-
var buf bytes.Buffer
29-
require.NoError(t, streamfmt.RenderLog(strings.NewReader(input), &buf, isTTY), "RenderLog")
30-
return buf.String()
31-
}
32-
33-
func assertLogContains(t *testing.T, got, want string) {
34-
t.Helper()
35-
assert.Contains(t, got, want)
36-
}
37-
38-
func assertLogNotContains(t *testing.T, got, want string) {
39-
t.Helper()
40-
assert.NotContains(t, got, want)
41-
}
42-
4317
func TestLogCleanCmd_NegativeDays(t *testing.T) {
4418
cmd := logCleanCmd()
4519
cmd.SetArgs([]string{"--days", "-1"})
@@ -54,106 +28,6 @@ func TestLogCleanCmd_OverflowDays(t *testing.T) {
5428
require.Error(t, err, "expected error for oversized --days")
5529
}
5630

57-
func TestRenderJobLog_JSONL(t *testing.T) {
58-
// Simulate Claude Code JSONL output
59-
input := strings.Join([]string{
60-
`{"type":"system","subtype":"init","session_id":"abc123"}`,
61-
`{"type":"assistant","message":{"content":[{"type":"text","text":"Reviewing the code."}]}}`,
62-
`{"type":"assistant","message":{"content":[{"type":"tool_use","name":"Read","input":{"file_path":"main.go"}}]}}`,
63-
`{"type":"tool_result","content":"file contents"}`,
64-
`{"type":"assistant","message":{"content":[{"type":"text","text":"Looks good overall."}]}}`,
65-
}, "\n")
66-
67-
out := streamfmt.StripANSI(executeRenderJobLog(t, input, true))
68-
69-
// Should contain the text messages
70-
assertLogContains(t, out, "Reviewing the code.")
71-
assertLogContains(t, out, "Read")
72-
assertLogContains(t, out, "main.go")
73-
74-
// Should NOT contain raw JSON
75-
assertLogNotContains(t, out, `"type"`)
76-
}
77-
78-
func TestRenderJobLog_Behaviors(t *testing.T) {
79-
tests := []struct {
80-
name string
81-
input string
82-
isJSON bool
83-
wantContain []string
84-
wantExclude []string
85-
wantEmpty bool
86-
}{
87-
{
88-
name: "PlainText",
89-
input: "line 1\nline 2\nline 3\n",
90-
isJSON: true,
91-
wantContain: []string{"line 1", "line 3"},
92-
},
93-
{
94-
name: "Empty",
95-
input: "",
96-
isJSON: true,
97-
wantEmpty: true,
98-
},
99-
{
100-
name: "OversizedLine",
101-
input: fmt.Sprintf(`{"type":"tool_result","content":"%s"}`, strings.Repeat("x", 2*1024*1024)),
102-
isJSON: true,
103-
wantContain: []string{}, // Just checking it doesn't error
104-
},
105-
{
106-
name: "PlainTextPreservesBlankLines",
107-
input: "line 1\n\nline 3\n",
108-
isJSON: true,
109-
wantContain: []string{"line 1\n\nline 3"},
110-
},
111-
{
112-
name: "SanitizesControlChars",
113-
input: "\x1b[31mred text\x1b[0m\n\x1b]0;evil title\x07\nclean line",
114-
isJSON: true,
115-
wantContain: []string{"red text", "clean line"},
116-
wantExclude: []string{"\x1b[", "\x1b]", "\x07"},
117-
},
118-
{
119-
name: "SanitizeMixedJSONAndControl",
120-
input: strings.Join([]string{
121-
`{"type":"assistant","message":{"content":[{"type":"text","text":"ok"}]}}`,
122-
"\x1b[1mbold agent stderr\x1b[0m",
123-
}, "\n"),
124-
isJSON: true,
125-
wantContain: []string{"ok", "bold agent stderr"},
126-
wantExclude: []string{"\x1b[1m"},
127-
},
128-
{
129-
name: "PreservesBlankLinesInMixedLog",
130-
input: strings.Join([]string{
131-
`{"type":"assistant","message":{"content":[{"type":"text","text":"hello"}]}}`,
132-
"stderr line 1",
133-
"",
134-
"stderr line 2",
135-
}, "\n"),
136-
isJSON: true,
137-
wantContain: []string{"stderr line 1\n\n", "stderr line 2"},
138-
},
139-
}
140-
141-
for _, tt := range tests {
142-
t.Run(tt.name, func(t *testing.T) {
143-
out := executeRenderJobLog(t, tt.input, tt.isJSON)
144-
if tt.wantEmpty {
145-
assert.Empty(t, out)
146-
}
147-
for _, want := range tt.wantContain {
148-
assertLogContains(t, out, want)
149-
}
150-
for _, exclude := range tt.wantExclude {
151-
assertLogNotContains(t, out, exclude)
152-
}
153-
})
154-
}
155-
}
156-
15731
func TestIsBrokenPipe(t *testing.T) {
15832
assert := assert.New(t)
15933
assert.False(isBrokenPipe(nil), "nil should not be broken pipe")
@@ -162,34 +36,6 @@ func TestIsBrokenPipe(t *testing.T) {
16236
assert.True(isBrokenPipe(fmt.Errorf("write: %w", syscall.EPIPE)), "wrapped EPIPE should be broken pipe")
16337
}
16438

165-
func TestRenderJobLog_MixedJSONAndPlainText(t *testing.T) {
166-
assert := assert.New(t)
167-
168-
// Non-JSON lines after JSON events should be preserved (e.g.
169-
// agent stderr/diagnostics mixed with JSONL stream output).
170-
input := strings.Join([]string{
171-
`{"type":"assistant","message":{"content":[{"type":"text","text":"Hello"}]}}`,
172-
`stderr: warning something`,
173-
`{"type":"assistant","message":{"content":[{"type":"text","text":"Done"}]}}`,
174-
`exit status 0`,
175-
}, "\n")
176-
177-
out := executeRenderJobLog(t, input, false)
178-
179-
assertLogContains(t, out, "Hello")
180-
assertLogContains(t, out, "stderr: warning something")
181-
assertLogContains(t, out, "Done")
182-
assertLogContains(t, out, "exit status 0")
183-
184-
// Verify ordering: "Hello" before "stderr", "stderr" before "Done".
185-
helloIdx := strings.Index(out, "Hello")
186-
stderrIdx := strings.Index(out, "stderr: warning")
187-
doneIdx := strings.Index(out, "Done")
188-
exitIdx := strings.Index(out, "exit status 0")
189-
assert.True(helloIdx < stderrIdx && stderrIdx < doneIdx && doneIdx < exitIdx, "output order wrong: Hello@%d stderr@%d Done@%d exit@%d",
190-
helloIdx, stderrIdx, doneIdx, exitIdx)
191-
}
192-
19339
func TestLogCmd_InvalidJobID(t *testing.T) {
19440
assert := assert.New(t)
19541

@@ -278,60 +124,3 @@ func TestLooksLikeJSON(t *testing.T) {
278124
assert.Equal(tt.want, got, "streamfmt.LooksLikeJSON(%q)", tt.input)
279125
}
280126
}
281-
282-
func TestRenderJobLog_OpenCodeEvents(t *testing.T) {
283-
input := strings.Join([]string{
284-
toJSON(map[string]any{
285-
"type": "step_start",
286-
"part": map[string]any{"type": "step-start"},
287-
}),
288-
toJSON(map[string]any{
289-
"type": "text",
290-
"part": map[string]any{
291-
"type": "text",
292-
"text": "Reviewing the code.",
293-
},
294-
}),
295-
toJSON(map[string]any{
296-
"type": "tool",
297-
"part": map[string]any{
298-
"type": "tool",
299-
"tool": "Read",
300-
"id": "tc_1",
301-
"state": map[string]any{
302-
"status": "running",
303-
"input": map[string]any{
304-
"file_path": "main.go",
305-
},
306-
},
307-
},
308-
}),
309-
toJSON(map[string]any{
310-
"type": "text",
311-
"part": map[string]any{
312-
"type": "text",
313-
"text": "Looks good overall.",
314-
},
315-
}),
316-
toJSON(map[string]any{
317-
"type": "step_finish",
318-
"part": map[string]any{
319-
"type": "step-finish",
320-
"reason": "stop",
321-
},
322-
}),
323-
}, "\n")
324-
325-
out := streamfmt.StripANSI(executeRenderJobLog(t, input, true))
326-
327-
assertLogContains(t, out, "Reviewing the code.")
328-
assertLogContains(t, out, "Read")
329-
assertLogContains(t, out, "main.go")
330-
assertLogContains(t, out, "Looks good overall.")
331-
332-
// Lifecycle events should be suppressed
333-
assertLogNotContains(t, out, "step_start")
334-
assertLogNotContains(t, out, "step_finish")
335-
// Raw JSON should not appear
336-
assertLogNotContains(t, out, `"type"`)
337-
}

0 commit comments

Comments
 (0)