Skip to content

Commit dfd601e

Browse files
committed
fix: use Contains+SplitN for artifact parsing, preserve failed decodes
Address three issues with artifact capture: 1. (High) Artifact parsing used HasPrefix which fails with real go test -json output where t.Logf prefixes lines with source location (e.g. "runner.go:102: ARTIFACT:..."). Now uses Contains+SplitN, matching the established CONSTRAINT_RESULT parsing pattern. 2. (Medium) Add slog.Warn when --result is used with --evidence-dir to inform users that saved results don't include diagnostic artifacts. 3. (Low) Failed artifact decodes now log a warning and preserve the original line in reason output instead of silently dropping it. Also extracts artifact parsing into a standalone extractArtifacts() function with comprehensive table-driven tests covering source-prefixed lines, malformed input preservation, and multi-artifact interleaving.
1 parent 785daed commit dfd601e

File tree

3 files changed

+179
-13
lines changed

3 files changed

+179
-13
lines changed

pkg/cli/validate.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ func runValidation(
227227
evidenceSource := result
228228
if evidenceResultPath != "" {
229229
slog.Info("loading saved result for evidence rendering", "path", evidenceResultPath)
230+
slog.Warn("saved results do not include diagnostic artifacts; evidence output will contain check status only")
230231
saved, loadErr := serializer.FromFile[validator.ValidationResult](evidenceResultPath)
231232
if loadErr != nil {
232233
return errors.Wrap(errors.ErrCodeInvalidRequest, "failed to load evidence result", loadErr)

pkg/validator/phases.go

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,6 +1001,37 @@ func parseConstraintResult(output []string) *ConstraintValidation {
10011001
return nil
10021002
}
10031003

1004+
// extractArtifacts separates artifact lines from regular test output.
1005+
// ARTIFACT: lines are base64-encoded JSON produced by TestRunner.Cancel().
1006+
// t.Logf prefixes output with source location (e.g. "runner.go:102: ARTIFACT:..."),
1007+
// so we use Contains + SplitN (same approach as CONSTRAINT_RESULT parsing).
1008+
// Lines that contain ARTIFACT: but fail to decode are preserved in reason output
1009+
// and a warning is logged, so debugging context is never silently lost.
1010+
func extractArtifacts(output []string) ([]checks.Artifact, []string) {
1011+
var artifacts []checks.Artifact
1012+
var reasonLines []string
1013+
for _, line := range output {
1014+
if !strings.Contains(line, "ARTIFACT:") {
1015+
reasonLines = append(reasonLines, line)
1016+
continue
1017+
}
1018+
parts := strings.SplitN(line, "ARTIFACT:", 2)
1019+
if len(parts) != 2 {
1020+
reasonLines = append(reasonLines, line)
1021+
continue
1022+
}
1023+
encoded := strings.TrimSpace(parts[1])
1024+
a, err := checks.DecodeArtifact(encoded)
1025+
if err != nil {
1026+
slog.Warn("failed to decode artifact line", "error", err)
1027+
reasonLines = append(reasonLines, line)
1028+
continue
1029+
}
1030+
artifacts = append(artifacts, *a)
1031+
}
1032+
return artifacts, reasonLines
1033+
}
1034+
10041035
func (v *Validator) runPhaseJob(
10051036
ctx context.Context,
10061037
deployer *agent.Deployer,
@@ -1123,19 +1154,8 @@ func (v *Validator) runPhaseJob(
11231154
}
11241155

11251156
// Extract artifacts from test output and build reason from remaining lines.
1126-
// ARTIFACT: lines are base64-encoded JSON produced by TestRunner.Cancel().
1127-
var reasonLines []string
1128-
for _, line := range test.Output {
1129-
trimmed := strings.TrimSpace(line)
1130-
if strings.HasPrefix(trimmed, "ARTIFACT:") {
1131-
encoded := strings.TrimPrefix(trimmed, "ARTIFACT:")
1132-
if a, err := checks.DecodeArtifact(encoded); err == nil {
1133-
checkResult.Artifacts = append(checkResult.Artifacts, *a)
1134-
}
1135-
} else {
1136-
reasonLines = append(reasonLines, line)
1137-
}
1138-
}
1157+
artifacts, reasonLines := extractArtifacts(test.Output)
1158+
checkResult.Artifacts = artifacts
11391159

11401160
// Build reason from last few non-artifact output lines
11411161
if len(reasonLines) > 0 {
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
// Copyright (c) 2025, NVIDIA CORPORATION. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package validator
16+
17+
import (
18+
"fmt"
19+
"strings"
20+
"testing"
21+
22+
"github.com/NVIDIA/aicr/pkg/validator/checks"
23+
)
24+
25+
func TestExtractArtifacts(t *testing.T) {
26+
// Build a valid encoded artifact for test cases.
27+
sample := checks.Artifact{Label: "DRA Controller", Data: "Status: Available"}
28+
encoded, err := sample.Encode()
29+
if err != nil {
30+
t.Fatalf("failed to encode test artifact: %v", err)
31+
}
32+
33+
tests := []struct {
34+
name string
35+
output []string
36+
wantArtifactCount int
37+
wantArtifactLabel string
38+
wantReasonCount int
39+
wantReasonHas string // substring that must appear in reason lines
40+
}{
41+
{
42+
name: "bare ARTIFACT line (no source prefix)",
43+
output: []string{
44+
"normal log line",
45+
"ARTIFACT:" + encoded,
46+
"another log line",
47+
},
48+
wantArtifactCount: 1,
49+
wantArtifactLabel: "DRA Controller",
50+
wantReasonCount: 2,
51+
},
52+
{
53+
name: "t.Logf source-prefixed ARTIFACT line",
54+
output: []string{
55+
"=== RUN TestDRASupport",
56+
fmt.Sprintf(" runner.go:102: ARTIFACT:%s", encoded),
57+
"--- PASS: TestDRASupport (1.23s)",
58+
},
59+
wantArtifactCount: 1,
60+
wantArtifactLabel: "DRA Controller",
61+
wantReasonCount: 2,
62+
},
63+
{
64+
name: "deep source prefix with tab",
65+
output: []string{
66+
fmt.Sprintf(" conformance_test.go:45: ARTIFACT:%s", encoded),
67+
},
68+
wantArtifactCount: 1,
69+
wantArtifactLabel: "DRA Controller",
70+
wantReasonCount: 0,
71+
},
72+
{
73+
name: "no artifacts",
74+
output: []string{
75+
"just normal test output",
76+
"nothing special here",
77+
},
78+
wantArtifactCount: 0,
79+
wantReasonCount: 2,
80+
},
81+
{
82+
name: "malformed artifact preserved in reason",
83+
output: []string{
84+
" runner.go:102: ARTIFACT:not-valid-base64!!!",
85+
"normal line",
86+
},
87+
wantArtifactCount: 0,
88+
wantReasonCount: 2,
89+
wantReasonHas: "ARTIFACT:",
90+
},
91+
{
92+
name: "empty output",
93+
output: []string{},
94+
wantArtifactCount: 0,
95+
wantReasonCount: 0,
96+
},
97+
{
98+
name: "multiple artifacts with interleaved output",
99+
output: []string{
100+
"log line 1",
101+
fmt.Sprintf(" runner.go:102: ARTIFACT:%s", encoded),
102+
"log line 2",
103+
fmt.Sprintf(" runner.go:102: ARTIFACT:%s", encoded),
104+
"log line 3",
105+
},
106+
wantArtifactCount: 2,
107+
wantArtifactLabel: "DRA Controller",
108+
wantReasonCount: 3,
109+
},
110+
}
111+
112+
for _, tt := range tests {
113+
t.Run(tt.name, func(t *testing.T) {
114+
artifacts, reasonLines := extractArtifacts(tt.output)
115+
116+
if len(artifacts) != tt.wantArtifactCount {
117+
t.Errorf("artifact count: got %d, want %d", len(artifacts), tt.wantArtifactCount)
118+
}
119+
120+
if tt.wantArtifactLabel != "" && len(artifacts) > 0 {
121+
if artifacts[0].Label != tt.wantArtifactLabel {
122+
t.Errorf("artifact label: got %q, want %q", artifacts[0].Label, tt.wantArtifactLabel)
123+
}
124+
}
125+
126+
if len(reasonLines) != tt.wantReasonCount {
127+
t.Errorf("reason line count: got %d, want %d\nlines: %v",
128+
len(reasonLines), tt.wantReasonCount, reasonLines)
129+
}
130+
131+
if tt.wantReasonHas != "" {
132+
found := false
133+
for _, line := range reasonLines {
134+
if strings.Contains(line, tt.wantReasonHas) {
135+
found = true
136+
break
137+
}
138+
}
139+
if !found {
140+
t.Errorf("reason lines should contain %q, got: %v", tt.wantReasonHas, reasonLines)
141+
}
142+
}
143+
})
144+
}
145+
}

0 commit comments

Comments
 (0)