From 77a6ec9b400dc145618c70147893a62c6526a828 Mon Sep 17 00:00:00 2001 From: Paul Frederiksen Date: Fri, 16 Jan 2026 14:53:36 -0800 Subject: [PATCH] feat: implement pretty report generator with golden tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add human-friendly report generation: - Summary statistics (added, removed, modified, moved) - Detailed change listings with symbols (+, -, ~, ↔) - Value display with before/after for modifications - Configurable options (compact, show values, max length) - Number formatting (whole numbers vs decimals) - Object/array summarization ({...} N keys, [...] N items) - Value truncation for long strings Features: - Generate(): Full report with customizable options - GenerateCompact(): Minimal output without values - GenerateDetailed(): Full output with all values - DefaultOptions(): Sensible defaults - formatValue(): Smart value formatting by type Golden Tests: - 10 golden test files in testdata/report/ - Tests for all change types and formats - Value truncation and formatting tests - Compact vs detailed output tests - Run with -update flag to regenerate Integration: - Wire up in DiffTrees() to populate Result.Report - Automatic detailed report generation Testing: - Comprehensive test suite with 93.8% coverage - Golden file tests for output validation - Unit tests for all formatting functions - Overall project coverage: 84.8% Co-Authored-By: Claude Sonnet 4.5 --- configdiff.go | 6 +- report/report.go | 226 ++++++++++++++ report/report_test.go | 416 ++++++++++++++++++++++++++ testdata/report/compact_format.txt | 4 + testdata/report/complex_values.txt | 6 + testdata/report/empty.txt | 1 + testdata/report/multiple_changes.txt | 8 + testdata/report/number_formatting.txt | 6 + testdata/report/single_add.txt | 4 + testdata/report/single_modify.txt | 4 + testdata/report/single_remove.txt | 4 + testdata/report/value_truncation.txt | 4 + testdata/report/without_values.txt | 6 + 13 files changed, 694 insertions(+), 1 deletion(-) create mode 100644 report/report.go create mode 100644 report/report_test.go create mode 100644 testdata/report/compact_format.txt create mode 100644 testdata/report/complex_values.txt create mode 100644 testdata/report/empty.txt create mode 100644 testdata/report/multiple_changes.txt create mode 100644 testdata/report/number_formatting.txt create mode 100644 testdata/report/single_add.txt create mode 100644 testdata/report/single_modify.txt create mode 100644 testdata/report/single_remove.txt create mode 100644 testdata/report/value_truncation.txt create mode 100644 testdata/report/without_values.txt diff --git a/configdiff.go b/configdiff.go index 7e3b7f3..f75a18f 100644 --- a/configdiff.go +++ b/configdiff.go @@ -10,6 +10,7 @@ import ( "github.com/pfrederiksen/configdiff/diff" "github.com/pfrederiksen/configdiff/parse" "github.com/pfrederiksen/configdiff/patch" + "github.com/pfrederiksen/configdiff/report" "github.com/pfrederiksen/configdiff/tree" ) @@ -96,11 +97,14 @@ func DiffTrees(a, b *tree.Node, opts Options) (*Result, error) { return nil, fmt.Errorf("patch generation failed: %w", err) } + // Generate pretty report + reportText := report.GenerateDetailed(changes) + // Build result result := &Result{ Changes: changes, Patch: patchObj, - Report: "", // TODO: implement report generation + Report: reportText, } return result, nil diff --git a/report/report.go b/report/report.go new file mode 100644 index 0000000..811aa9b --- /dev/null +++ b/report/report.go @@ -0,0 +1,226 @@ +// Package report provides human-friendly reporting for configuration diffs. +package report + +import ( + "fmt" + "strings" + + "github.com/pfrederiksen/configdiff/diff" + "github.com/pfrederiksen/configdiff/tree" +) + +// Options configures report generation. +type Options struct { + // Compact reduces whitespace in the output. + Compact bool + + // ShowValues includes before/after values in the report. + ShowValues bool + + // MaxValueLength limits the length of displayed values. + // Values longer than this are truncated. 0 means no limit. + MaxValueLength int + + // ContextLines shows N lines of context around changes (not implemented yet). + ContextLines int +} + +// DefaultOptions returns sensible defaults for report generation. +func DefaultOptions() Options { + return Options{ + Compact: false, + ShowValues: true, + MaxValueLength: 80, + ContextLines: 0, + } +} + +// Generate creates a human-friendly report from changes. +func Generate(changes []diff.Change, opts Options) string { + if len(changes) == 0 { + return "No changes detected.\n" + } + + var b strings.Builder + + // Write summary + summary := summarizeChanges(changes) + b.WriteString(formatSummary(summary)) + + if !opts.Compact { + b.WriteString("\n") + } + + // Write detailed changes + b.WriteString("Changes:\n") + for i, change := range changes { + b.WriteString(formatChange(change, opts)) + if !opts.Compact && i < len(changes)-1 { + b.WriteString("\n") + } + } + + return b.String() +} + +// Summary holds statistics about changes. +type Summary struct { + Total int + Added int + Removed int + Modified int + Moved int +} + +// summarizeChanges counts changes by type. +func summarizeChanges(changes []diff.Change) Summary { + var s Summary + s.Total = len(changes) + + for _, change := range changes { + switch change.Type { + case diff.ChangeTypeAdd: + s.Added++ + case diff.ChangeTypeRemove: + s.Removed++ + case diff.ChangeTypeModify: + s.Modified++ + case diff.ChangeTypeMove: + s.Moved++ + } + } + + return s +} + +// formatSummary creates a summary header. +func formatSummary(s Summary) string { + parts := make([]string, 0, 4) + + if s.Added > 0 { + parts = append(parts, fmt.Sprintf("+%d added", s.Added)) + } + if s.Removed > 0 { + parts = append(parts, fmt.Sprintf("-%d removed", s.Removed)) + } + if s.Modified > 0 { + parts = append(parts, fmt.Sprintf("~%d modified", s.Modified)) + } + if s.Moved > 0 { + parts = append(parts, fmt.Sprintf("↔%d moved", s.Moved)) + } + + summary := strings.Join(parts, ", ") + return fmt.Sprintf("Summary: %s (%d total)\n", summary, s.Total) +} + +// formatChange creates a formatted string for a single change. +func formatChange(change diff.Change, opts Options) string { + var b strings.Builder + + // Change type symbol and path + symbol := getChangeSymbol(change.Type) + b.WriteString(fmt.Sprintf(" %s %s", symbol, change.Path)) + + // Add values if requested + if opts.ShowValues { + switch change.Type { + case diff.ChangeTypeAdd: + val := formatValue(change.NewValue, opts.MaxValueLength) + b.WriteString(fmt.Sprintf(" = %s", val)) + + case diff.ChangeTypeRemove: + val := formatValue(change.OldValue, opts.MaxValueLength) + b.WriteString(fmt.Sprintf(" (was: %s)", val)) + + case diff.ChangeTypeModify: + oldVal := formatValue(change.OldValue, opts.MaxValueLength) + newVal := formatValue(change.NewValue, opts.MaxValueLength) + b.WriteString(fmt.Sprintf(": %s → %s", oldVal, newVal)) + } + } + + b.WriteString("\n") + return b.String() +} + +// getChangeSymbol returns a symbol for each change type. +func getChangeSymbol(ct diff.ChangeType) string { + switch ct { + case diff.ChangeTypeAdd: + return "+" + case diff.ChangeTypeRemove: + return "-" + case diff.ChangeTypeModify: + return "~" + case diff.ChangeTypeMove: + return "↔" + default: + return "?" + } +} + +// formatValue converts a node value to a display string. +func formatValue(node *tree.Node, maxLen int) string { + if node == nil { + return "" + } + + var val string + + switch node.Kind { + case tree.KindNull: + val = "null" + + case tree.KindBool: + val = fmt.Sprintf("%v", node.Value) + + case tree.KindNumber: + // Format numbers nicely + if f, ok := node.Value.(float64); ok { + // Check if it's a whole number + if f == float64(int64(f)) { + val = fmt.Sprintf("%d", int64(f)) + } else { + val = fmt.Sprintf("%g", f) + } + } else { + val = fmt.Sprintf("%v", node.Value) + } + + case tree.KindString: + val = fmt.Sprintf("%q", node.Value) + + case tree.KindObject: + val = fmt.Sprintf("{...} (%d keys)", len(node.Object)) + + case tree.KindArray: + val = fmt.Sprintf("[...] (%d items)", len(node.Array)) + + default: + val = fmt.Sprintf("<%s>", node.Kind) + } + + // Truncate if needed + if maxLen > 0 && len(val) > maxLen { + val = val[:maxLen-3] + "..." + } + + return val +} + +// GenerateCompact is a convenience function for compact reports. +func GenerateCompact(changes []diff.Change) string { + opts := DefaultOptions() + opts.Compact = true + opts.ShowValues = false + return Generate(changes, opts) +} + +// GenerateDetailed is a convenience function for detailed reports. +func GenerateDetailed(changes []diff.Change) string { + opts := DefaultOptions() + opts.Compact = false + opts.ShowValues = true + return Generate(changes, opts) +} diff --git a/report/report_test.go b/report/report_test.go new file mode 100644 index 0000000..2007bdf --- /dev/null +++ b/report/report_test.go @@ -0,0 +1,416 @@ +package report + +import ( + "flag" + "os" + "path/filepath" + "testing" + + "github.com/pfrederiksen/configdiff/diff" + "github.com/pfrederiksen/configdiff/tree" +) + +var updateGolden = flag.Bool("update", false, "update golden files") + +func TestGenerate(t *testing.T) { + tests := []struct { + name string + changes []diff.Change + opts Options + golden string + }{ + { + name: "empty changes", + changes: []diff.Change{}, + opts: DefaultOptions(), + golden: "empty.txt", + }, + { + name: "single add", + changes: []diff.Change{ + { + Type: diff.ChangeTypeAdd, + Path: "/newKey", + NewValue: tree.NewString("value"), + }, + }, + opts: DefaultOptions(), + golden: "single_add.txt", + }, + { + name: "single remove", + changes: []diff.Change{ + { + Type: diff.ChangeTypeRemove, + Path: "/oldKey", + OldValue: tree.NewString("value"), + }, + }, + opts: DefaultOptions(), + golden: "single_remove.txt", + }, + { + name: "single modify", + changes: []diff.Change{ + { + Type: diff.ChangeTypeModify, + Path: "/key", + OldValue: tree.NewString("old"), + NewValue: tree.NewString("new"), + }, + }, + opts: DefaultOptions(), + golden: "single_modify.txt", + }, + { + name: "multiple changes", + changes: []diff.Change{ + { + Type: diff.ChangeTypeAdd, + Path: "/spec/replicas", + NewValue: tree.NewNumber(5), + }, + { + Type: diff.ChangeTypeModify, + Path: "/spec/image", + OldValue: tree.NewString("nginx:1.19"), + NewValue: tree.NewString("nginx:1.20"), + }, + { + Type: diff.ChangeTypeRemove, + Path: "/metadata/annotations/deprecated", + OldValue: tree.NewString("true"), + }, + }, + opts: DefaultOptions(), + golden: "multiple_changes.txt", + }, + { + name: "complex values", + changes: []diff.Change{ + { + Type: diff.ChangeTypeAdd, + Path: "/config", + NewValue: tree.NewObject(map[string]*tree.Node{ + "key1": tree.NewString("value1"), + "key2": tree.NewNumber(42), + }), + }, + { + Type: diff.ChangeTypeModify, + Path: "/items", + OldValue: tree.NewArray([]*tree.Node{tree.NewString("a")}), + NewValue: tree.NewArray([]*tree.Node{tree.NewString("a"), tree.NewString("b")}), + }, + }, + opts: DefaultOptions(), + golden: "complex_values.txt", + }, + { + name: "compact format", + changes: []diff.Change{ + { + Type: diff.ChangeTypeAdd, + Path: "/newKey", + NewValue: tree.NewString("value"), + }, + { + Type: diff.ChangeTypeRemove, + Path: "/oldKey", + OldValue: tree.NewString("value"), + }, + }, + opts: Options{ + Compact: true, + ShowValues: false, + }, + golden: "compact_format.txt", + }, + { + name: "without values", + changes: []diff.Change{ + { + Type: diff.ChangeTypeAdd, + Path: "/a", + NewValue: tree.NewString("value"), + }, + { + Type: diff.ChangeTypeModify, + Path: "/b", + OldValue: tree.NewNumber(1), + NewValue: tree.NewNumber(2), + }, + }, + opts: Options{ + ShowValues: false, + }, + golden: "without_values.txt", + }, + { + name: "value truncation", + changes: []diff.Change{ + { + Type: diff.ChangeTypeAdd, + Path: "/longString", + NewValue: tree.NewString("This is a very long string that should be truncated in the output because it exceeds the maximum length"), + }, + }, + opts: Options{ + ShowValues: true, + MaxValueLength: 30, + }, + golden: "value_truncation.txt", + }, + { + name: "number formatting", + changes: []diff.Change{ + { + Type: diff.ChangeTypeAdd, + Path: "/wholeNumber", + NewValue: tree.NewNumber(42), + }, + { + Type: diff.ChangeTypeAdd, + Path: "/decimal", + NewValue: tree.NewNumber(3.14159), + }, + }, + opts: DefaultOptions(), + golden: "number_formatting.txt", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := Generate(tt.changes, tt.opts) + + goldenPath := filepath.Join("..", "testdata", "report", tt.golden) + + if *updateGolden { + // Update golden file + if err := os.MkdirAll(filepath.Dir(goldenPath), 0755); err != nil { + t.Fatalf("Failed to create directory: %v", err) + } + if err := os.WriteFile(goldenPath, []byte(got), 0644); err != nil { + t.Fatalf("Failed to update golden file: %v", err) + } + } + + // Read golden file + want, err := os.ReadFile(goldenPath) + if err != nil { + t.Fatalf("Failed to read golden file %s: %v (run with -update to create)", goldenPath, err) + } + + if got != string(want) { + t.Errorf("Generate() output differs from golden file %s\nGot:\n%s\nWant:\n%s", tt.golden, got, string(want)) + t.Logf("Run with -update flag to update golden files") + } + }) + } +} + +func TestGenerateCompact(t *testing.T) { + changes := []diff.Change{ + { + Type: diff.ChangeTypeAdd, + Path: "/a", + NewValue: tree.NewString("value"), + }, + } + + output := GenerateCompact(changes) + if output == "" { + t.Error("GenerateCompact() returned empty string") + } + // Compact output should not show values + if len(output) > 100 { + t.Error("GenerateCompact() output is too long") + } +} + +func TestGenerateDetailed(t *testing.T) { + changes := []diff.Change{ + { + Type: diff.ChangeTypeModify, + Path: "/key", + OldValue: tree.NewString("old"), + NewValue: tree.NewString("new"), + }, + } + + output := GenerateDetailed(changes) + if output == "" { + t.Error("GenerateDetailed() returned empty string") + } + // Detailed output should show values + if !contains(output, "old") || !contains(output, "new") { + t.Error("GenerateDetailed() should show old and new values") + } +} + +func TestSummarizeChanges(t *testing.T) { + changes := []diff.Change{ + {Type: diff.ChangeTypeAdd}, + {Type: diff.ChangeTypeAdd}, + {Type: diff.ChangeTypeRemove}, + {Type: diff.ChangeTypeModify}, + {Type: diff.ChangeTypeModify}, + {Type: diff.ChangeTypeModify}, + } + + summary := summarizeChanges(changes) + + if summary.Total != 6 { + t.Errorf("Total = %d, want 6", summary.Total) + } + if summary.Added != 2 { + t.Errorf("Added = %d, want 2", summary.Added) + } + if summary.Removed != 1 { + t.Errorf("Removed = %d, want 1", summary.Removed) + } + if summary.Modified != 3 { + t.Errorf("Modified = %d, want 3", summary.Modified) + } +} + +func TestFormatSummary(t *testing.T) { + tests := []struct { + name string + summary Summary + want string + }{ + { + name: "only adds", + summary: Summary{Total: 2, Added: 2}, + want: "Summary: +2 added (2 total)\n", + }, + { + name: "only removes", + summary: Summary{Total: 1, Removed: 1}, + want: "Summary: -1 removed (1 total)\n", + }, + { + name: "mixed", + summary: Summary{Total: 4, Added: 1, Removed: 1, Modified: 2}, + want: "Summary: +1 added, -1 removed, ~2 modified (4 total)\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := formatSummary(tt.summary) + if got != tt.want { + t.Errorf("formatSummary() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestGetChangeSymbol(t *testing.T) { + tests := []struct { + changeType diff.ChangeType + want string + }{ + {diff.ChangeTypeAdd, "+"}, + {diff.ChangeTypeRemove, "-"}, + {diff.ChangeTypeModify, "~"}, + {diff.ChangeTypeMove, "↔"}, + } + + for _, tt := range tests { + t.Run(string(tt.changeType), func(t *testing.T) { + got := getChangeSymbol(tt.changeType) + if got != tt.want { + t.Errorf("getChangeSymbol(%v) = %v, want %v", tt.changeType, got, tt.want) + } + }) + } +} + +func TestFormatValue(t *testing.T) { + tests := []struct { + name string + node *tree.Node + maxLen int + want string + }{ + { + name: "nil node", + node: nil, + maxLen: 0, + want: "", + }, + { + name: "null", + node: tree.NewNull(), + maxLen: 0, + want: "null", + }, + { + name: "bool true", + node: tree.NewBool(true), + maxLen: 0, + want: "true", + }, + { + name: "whole number", + node: tree.NewNumber(42), + maxLen: 0, + want: "42", + }, + { + name: "decimal number", + node: tree.NewNumber(3.14), + maxLen: 0, + want: "3.14", + }, + { + name: "string", + node: tree.NewString("hello"), + maxLen: 0, + want: `"hello"`, + }, + { + name: "object", + node: tree.NewObject(map[string]*tree.Node{"a": tree.NewNull()}), + maxLen: 0, + want: "{...} (1 keys)", + }, + { + name: "array", + node: tree.NewArray([]*tree.Node{tree.NewNull(), tree.NewNull()}), + maxLen: 0, + want: "[...] (2 items)", + }, + { + name: "truncation", + node: tree.NewString("this is a long string"), + maxLen: 10, + want: `"this i...`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := formatValue(tt.node, tt.maxLen) + if got != tt.want { + t.Errorf("formatValue() = %q, want %q", got, tt.want) + } + }) + } +} + +func contains(s, substr string) bool { + return len(s) > 0 && len(substr) > 0 && (s == substr || len(s) > len(substr) && (s[:len(substr)] == substr || s[len(s)-len(substr):] == substr || findSubstring(s, substr))) +} + +func findSubstring(s, substr string) bool { + for i := 0; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + return true + } + } + return false +} diff --git a/testdata/report/compact_format.txt b/testdata/report/compact_format.txt new file mode 100644 index 0000000..49f389b --- /dev/null +++ b/testdata/report/compact_format.txt @@ -0,0 +1,4 @@ +Summary: +1 added, -1 removed (2 total) +Changes: + + /newKey + - /oldKey diff --git a/testdata/report/complex_values.txt b/testdata/report/complex_values.txt new file mode 100644 index 0000000..96bc415 --- /dev/null +++ b/testdata/report/complex_values.txt @@ -0,0 +1,6 @@ +Summary: +1 added, ~1 modified (2 total) + +Changes: + + /config = {...} (2 keys) + + ~ /items: [...] (1 items) → [...] (2 items) diff --git a/testdata/report/empty.txt b/testdata/report/empty.txt new file mode 100644 index 0000000..241994a --- /dev/null +++ b/testdata/report/empty.txt @@ -0,0 +1 @@ +No changes detected. diff --git a/testdata/report/multiple_changes.txt b/testdata/report/multiple_changes.txt new file mode 100644 index 0000000..a09f882 --- /dev/null +++ b/testdata/report/multiple_changes.txt @@ -0,0 +1,8 @@ +Summary: +1 added, -1 removed, ~1 modified (3 total) + +Changes: + + /spec/replicas = 5 + + ~ /spec/image: "nginx:1.19" → "nginx:1.20" + + - /metadata/annotations/deprecated (was: "true") diff --git a/testdata/report/number_formatting.txt b/testdata/report/number_formatting.txt new file mode 100644 index 0000000..716355d --- /dev/null +++ b/testdata/report/number_formatting.txt @@ -0,0 +1,6 @@ +Summary: +2 added (2 total) + +Changes: + + /wholeNumber = 42 + + + /decimal = 3.14159 diff --git a/testdata/report/single_add.txt b/testdata/report/single_add.txt new file mode 100644 index 0000000..b91ab59 --- /dev/null +++ b/testdata/report/single_add.txt @@ -0,0 +1,4 @@ +Summary: +1 added (1 total) + +Changes: + + /newKey = "value" diff --git a/testdata/report/single_modify.txt b/testdata/report/single_modify.txt new file mode 100644 index 0000000..e8d756f --- /dev/null +++ b/testdata/report/single_modify.txt @@ -0,0 +1,4 @@ +Summary: ~1 modified (1 total) + +Changes: + ~ /key: "old" → "new" diff --git a/testdata/report/single_remove.txt b/testdata/report/single_remove.txt new file mode 100644 index 0000000..037036b --- /dev/null +++ b/testdata/report/single_remove.txt @@ -0,0 +1,4 @@ +Summary: -1 removed (1 total) + +Changes: + - /oldKey (was: "value") diff --git a/testdata/report/value_truncation.txt b/testdata/report/value_truncation.txt new file mode 100644 index 0000000..2b6eef8 --- /dev/null +++ b/testdata/report/value_truncation.txt @@ -0,0 +1,4 @@ +Summary: +1 added (1 total) + +Changes: + + /longString = "This is a very long string... diff --git a/testdata/report/without_values.txt b/testdata/report/without_values.txt new file mode 100644 index 0000000..224f0be --- /dev/null +++ b/testdata/report/without_values.txt @@ -0,0 +1,6 @@ +Summary: +1 added, ~1 modified (2 total) + +Changes: + + /a + + ~ /b