Skip to content

Commit 019f184

Browse files
stbenjamclaude
andcommitted
Simplify extractJSON: remove startChar param, add tests
Drop the startChar parameter — the helper now finds the first line starting with '{' or '[' automatically. Use json.Decoder to extract the complete JSON value, which correctly handles trailing log lines that might contain braces. Keep null handling in ListImages where it's semantically relevant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5ebf4c4 commit 019f184

2 files changed

Lines changed: 77 additions & 10 deletions

File tree

pkg/test/extensions/binary.go

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -335,15 +335,15 @@ var extensionBinaries = []TestBinary{
335335
},
336336
}
337337

338-
// extractJSON finds the first JSON value in output that starts with startChar,
339-
// skipping any non-JSON log lines that precede it. This is necessary because some extension binaries
340-
// output warnings or debug logging to stdout before the JSON payload.
341-
func extractJSON(output []byte, startChar byte) ([]byte, error) {
338+
// extractJSON finds the first JSON object or array in output, skipping any non-JSON log lines
339+
// that precede it. This is necessary because some extension binaries output warnings or debug
340+
// logging to stdout before the JSON payload.
341+
func extractJSON(output []byte) ([]byte, error) {
342342
jsonBegins := -1
343343
lines := bytes.Split(output, []byte("\n"))
344344
for i, line := range lines {
345345
trimmed := bytes.TrimSpace(line)
346-
if len(trimmed) > 0 && trimmed[0] == startChar {
346+
if len(trimmed) > 0 && (trimmed[0] == '{' || trimmed[0] == '[') {
347347
jsonBegins = 0
348348
for j := 0; j < i; j++ {
349349
jsonBegins += len(lines[j]) + 1 // +1 for the newline character
@@ -362,9 +362,6 @@ func extractJSON(output []byte, startChar byte) ([]byte, error) {
362362
if err := dec.Decode(&raw); err != nil {
363363
return nil, fmt.Errorf("no valid JSON found in output: %w", err)
364364
}
365-
if len(raw) == 0 || raw[0] != startChar {
366-
return nil, fmt.Errorf("no valid JSON found in output: %s", string(output))
367-
}
368365
return raw, nil
369366
}
370367

@@ -385,7 +382,7 @@ func (b *TestBinary) Info(ctx context.Context) (*Extension, error) {
385382
logrus.Errorf("Command output for %s: %s", binName, string(infoJson))
386383
return nil, fmt.Errorf("failed running '%s info': %w\nOutput: %s", b.binaryPath, err, infoJson)
387384
}
388-
jsonData, err := extractJSON(infoJson, '{')
385+
jsonData, err := extractJSON(infoJson)
389386
if err != nil {
390387
logrus.Errorf("No valid JSON found in output from %s info command", binName)
391388
logrus.Errorf("Raw output from %s: %s", binName, string(infoJson))
@@ -576,7 +573,7 @@ func (b *TestBinary) ListImages(ctx context.Context) (ImageSet, error) {
576573
return nil, fmt.Errorf("failed running '%s images': %w\nOutput: %s", b.binaryPath, err, output)
577574
}
578575

579-
jsonData, err := extractJSON(output, '[')
576+
jsonData, err := extractJSON(output)
580577
if err != nil {
581578
// Extensions that have no images may output "null" instead of an array
582579
if bytes.Contains(output, []byte("null")) {
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package extensions
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
func TestExtractJSON(t *testing.T) {
11+
tests := []struct {
12+
name string
13+
input string
14+
want string
15+
wantErr bool
16+
}{
17+
{
18+
name: "clean JSON object",
19+
input: `{"key": "value"}`,
20+
want: `{"key": "value"}`,
21+
},
22+
{
23+
name: "clean JSON array",
24+
input: `[{"index": 1}]`,
25+
want: `[{"index": 1}]`,
26+
},
27+
{
28+
name: "warning lines before JSON object",
29+
input: "W0414 08:58:39.856273 46367 controller.go:47] some warning\nW0414 08:58:39.865859 46367 feature_gate.go:352] another warning\n{\"key\": \"value\"}\n",
30+
want: `{"key": "value"}`,
31+
},
32+
{
33+
name: "warning lines before JSON array",
34+
input: "W0414 08:58:39.856273 46367 controller.go:47] some warning\n[{\"index\": 1}]\n",
35+
want: `[{"index": 1}]`,
36+
},
37+
{
38+
name: "log lines after JSON are ignored",
39+
input: "{\"key\": \"value\"}\nW0414 trailing log with } brace\n",
40+
want: `{"key": "value"}`,
41+
},
42+
{
43+
name: "no JSON in output",
44+
input: "W0414 just warnings\nI0414 and info lines\n",
45+
wantErr: true,
46+
},
47+
{
48+
name: "empty output",
49+
input: "",
50+
wantErr: true,
51+
},
52+
{
53+
name: "null is not a JSON object or array",
54+
input: "W0414 warning\nnull\n",
55+
wantErr: true,
56+
},
57+
}
58+
59+
for _, tt := range tests {
60+
t.Run(tt.name, func(t *testing.T) {
61+
got, err := extractJSON([]byte(tt.input))
62+
if tt.wantErr {
63+
assert.Error(t, err)
64+
return
65+
}
66+
require.NoError(t, err)
67+
assert.JSONEq(t, tt.want, string(got))
68+
})
69+
}
70+
}

0 commit comments

Comments
 (0)