Skip to content

Commit a8e802e

Browse files
committed
refactor: improve error handling in structured entry creation and update tests
1 parent fe2a468 commit a8e802e

File tree

2 files changed

+34
-37
lines changed

2 files changed

+34
-37
lines changed

diff/diff.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import (
44
"bytes"
55
"fmt"
66
"io"
7-
"log"
87
"math"
8+
"os"
99
"regexp"
1010
"sort"
1111
"strings"
@@ -278,14 +278,16 @@ func doDiff(report *Report, key string, oldContent *manifest.MappingResult, newC
278278
if options.StructuredOutput() {
279279
entry, err := buildStructuredEntry(key, changeType, subjectKind, options.SuppressedKinds, oldContent, newContent)
280280
if err != nil {
281-
// Log error with context before panicking to aid debugging
282-
log.Printf("Error building structured entry for %s (kind: %s, changeType: %s): %v", key, subjectKind, changeType, err)
283-
panic(fmt.Errorf("failed to build structured entry: %w", err))
284-
}
285-
if changeType == "MODIFY" && !entry.ChangesSuppressed && len(entry.Changes) == 0 {
286-
return
281+
// Log warning and omit field-level changes for this entry
282+
// printStructuredReport() will still output a basic entry with name and changeType
283+
fmt.Fprintf(os.Stderr, "Warning: failed to build structured entry for %s (kind: %s, changeType: %s): %v\n",
284+
key, subjectKind, changeType, err)
285+
} else {
286+
if changeType == "MODIFY" && !entry.ChangesSuppressed && len(entry.Changes) == 0 {
287+
return
288+
}
289+
structured = entry
287290
}
288-
structured = entry
289291
}
290292

291293
report.addEntry(key, options.SuppressedKinds, subjectKind, options.OutputContext, diffs, changeType, structured)

diff/diff_test.go

Lines changed: 24 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -756,10 +756,14 @@ data:
756756
key: makeMapping(":\n not-valid: value"),
757757
}
758758

759-
requireStructuredPanic(t, func() {
760-
var buf bytes.Buffer
761-
_ = Manifests(oldIndex, newIndex, opts, &buf)
762-
}, "failed to build structured entry", "convert new manifest")
759+
var buf bytes.Buffer
760+
changed := Manifests(oldIndex, newIndex, opts, &buf)
761+
require.True(t, changed, "Should report resource-level change even when YAML parsing fails")
762+
763+
var entries []StructuredEntry
764+
require.NoError(t, json.Unmarshal(buf.Bytes(), &entries))
765+
require.Len(t, entries, 1)
766+
require.Equal(t, "MODIFY", entries[0].ChangeType)
763767
})
764768

765769
t.Run("InvalidOldManifestYAML", func(t *testing.T) {
@@ -770,10 +774,14 @@ data:
770774
key: makeMapping(validConfigMap),
771775
}
772776

773-
requireStructuredPanic(t, func() {
774-
var buf bytes.Buffer
775-
_ = Manifests(oldIndex, newIndex, opts, &buf)
776-
}, "failed to build structured entry", "convert old manifest")
777+
var buf bytes.Buffer
778+
changed := Manifests(oldIndex, newIndex, opts, &buf)
779+
require.True(t, changed, "Should report resource-level change even when YAML parsing fails")
780+
781+
var entries []StructuredEntry
782+
require.NoError(t, json.Unmarshal(buf.Bytes(), &entries))
783+
require.Len(t, entries, 1)
784+
require.Equal(t, "MODIFY", entries[0].ChangeType)
777785
})
778786

779787
t.Run("ArrayDocumentProducesJSONUnmarshalError", func(t *testing.T) {
@@ -790,10 +798,14 @@ data:
790798
key: makeMapping(listManifest),
791799
}
792800

793-
requireStructuredPanic(t, func() {
794-
var buf bytes.Buffer
795-
_ = Manifests(map[string]*manifest.MappingResult{}, newIndex, opts, &buf)
796-
}, "failed to build structured entry", "convert new manifest", "cannot unmarshal array")
801+
var buf bytes.Buffer
802+
changed := Manifests(map[string]*manifest.MappingResult{}, newIndex, opts, &buf)
803+
require.True(t, changed, "Should report resource-level change even when JSON unmarshal fails")
804+
805+
var entries []StructuredEntry
806+
require.NoError(t, json.Unmarshal(buf.Bytes(), &entries))
807+
require.Len(t, entries, 1)
808+
require.Equal(t, "ADD", entries[0].ChangeType)
797809
})
798810
}
799811

@@ -967,23 +979,6 @@ spec:
967979
require.Empty(t, entries, "Should have no entries for empty manifests")
968980
})
969981
}
970-
971-
func requireStructuredPanic(t *testing.T, fn func(), substrings ...string) {
972-
t.Helper()
973-
defer func() {
974-
if r := recover(); r != nil {
975-
err, ok := r.(error)
976-
require.True(t, ok, "panic should be an error, got %T", r)
977-
for _, sub := range substrings {
978-
require.Contains(t, err.Error(), sub)
979-
}
980-
return
981-
}
982-
t.Fatalf("expected panic but function completed successfully")
983-
}()
984-
fn()
985-
}
986-
987982
func TestManifestsWithRedactedSecrets(t *testing.T) {
988983
ansi.DisableColors(true)
989984

0 commit comments

Comments
 (0)