From 9d5217bf34ffa3c5a5287c0f584a180d4737e255 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Tue, 24 Mar 2026 09:39:22 +0800 Subject: [PATCH] fix: make --find-renames work with --output=dyff The dyff output format was always using dyff.DetectRenames(true), which overrode helm-diff's --find-renames option. This caused renames detected by helm-diff to be re-evaluated by dyff, resulting in inconsistent output compared to other formats (diff, simple). Fixes #632 Signed-off-by: yxxhero --- diff/diff.go | 6 +- diff/report.go | 6 +- diff/report_test.go | 141 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+), 7 deletions(-) diff --git a/diff/diff.go b/diff/diff.go index 637a7305..d0ef4854 100644 --- a/diff/diff.go +++ b/diff/diff.go @@ -256,10 +256,8 @@ func doDiff(report *Report, key string, oldContent *manifest.MappingResult, newC } case newContent == nil: changeType = "REMOVE" - if oldContent != nil { - subjectKind = oldContent.Kind - } - if !options.StructuredOutput() && oldContent != nil { + subjectKind = oldContent.Kind + if !options.StructuredOutput() { emptyMapping := &manifest.MappingResult{} diffs = diffMappingResults(oldContent, emptyMapping, options.StripTrailingCR) } diff --git a/diff/report.go b/diff/report.go index 96508aec..6922c0d6 100644 --- a/diff/report.go +++ b/diff/report.go @@ -110,12 +110,12 @@ func printDyffReport(r *Report, to io.Writer) { currentInputFile, newInputFile, _ := ytbx.LoadFiles(currentFile.Name(), newFile.Name()) - report, _ := dyff.CompareInputFiles( - currentInputFile, newInputFile, + var compareOptions []dyff.CompareOption + compareOptions = append(compareOptions, dyff.IgnoreWhitespaceChanges(true), dyff.KubernetesEntityDetection(true), - dyff.DetectRenames(true), ) + report, _ := dyff.CompareInputFiles(currentInputFile, newInputFile, compareOptions...) reportWriter := &dyff.HumanReport{ Report: report, OmitHeader: true, diff --git a/diff/report_test.go b/diff/report_test.go index 1a2aff60..9c4936e4 100644 --- a/diff/report_test.go +++ b/diff/report_test.go @@ -1,8 +1,10 @@ package diff import ( + "bytes" "testing" + "github.com/aryann/difflib" "github.com/stretchr/testify/require" ) @@ -34,3 +36,142 @@ func TestLoadFromKey(t *testing.T) { require.Equal(t, expectedTemplateSpec, *templateSpec) } } + +func TestPrintDyffReport(t *testing.T) { + report := &Report{ + Entries: []ReportEntry{ + { + Key: "default, nginx, Deployment (apps)", + Kind: "Deployment", + ChangeType: "MODIFY", + Diffs: []difflib.DiffRecord{ + {Payload: "apiVersion: apps/v1", Delta: difflib.Common}, + {Payload: "kind: Deployment", Delta: difflib.Common}, + {Payload: "metadata:", Delta: difflib.Common}, + {Payload: " name: nginx", Delta: difflib.Common}, + {Payload: "spec:", Delta: difflib.Common}, + {Payload: " replicas: 2", Delta: difflib.LeftOnly}, + {Payload: " replicas: 3", Delta: difflib.RightOnly}, + }, + }, + }, + } + + var buf bytes.Buffer + printDyffReport(report, &buf) + + output := buf.String() + require.NotEmpty(t, output) + require.Contains(t, output, "replicas", "Expected dyff output to mention replicas field") + require.Contains(t, output, "- 2", "Expected dyff output to show original replicas value") + require.Contains(t, output, "+ 3", "Expected dyff output to show updated replicas value") +} + +func TestPrintDyffReportWithAddAndRemove(t *testing.T) { + report := &Report{ + Entries: []ReportEntry{ + { + Key: "default, old-app, Deployment (apps)", + Kind: "Deployment", + ChangeType: "REMOVE", + Diffs: []difflib.DiffRecord{ + {Payload: "apiVersion: apps/v1", Delta: difflib.LeftOnly}, + {Payload: "kind: Deployment", Delta: difflib.LeftOnly}, + {Payload: "metadata:", Delta: difflib.LeftOnly}, + {Payload: " name: old-app", Delta: difflib.LeftOnly}, + }, + }, + { + Key: "default, new-app, Deployment (apps)", + Kind: "Deployment", + ChangeType: "ADD", + Diffs: []difflib.DiffRecord{ + {Payload: "apiVersion: apps/v1", Delta: difflib.RightOnly}, + {Payload: "kind: Deployment", Delta: difflib.RightOnly}, + {Payload: "metadata:", Delta: difflib.RightOnly}, + {Payload: " name: new-app", Delta: difflib.RightOnly}, + }, + }, + }, + } + + var buf bytes.Buffer + printDyffReport(report, &buf) + + output := buf.String() + require.NotEmpty(t, output) + require.Contains(t, output, "old-app", "Expected dyff output to show removed resource old-app") + require.Contains(t, output, "new-app", "Expected dyff output to show added resource new-app") +} + +func TestPrintDyffReportDoesNotMergeAddRemove(t *testing.T) { + addRemoveReport := &Report{ + Entries: []ReportEntry{ + { + Key: "default, old-app, Deployment (apps)", + Kind: "Deployment", + ChangeType: "REMOVE", + Diffs: []difflib.DiffRecord{ + {Payload: "apiVersion: apps/v1", Delta: difflib.LeftOnly}, + {Payload: "kind: Deployment", Delta: difflib.LeftOnly}, + {Payload: "metadata:", Delta: difflib.LeftOnly}, + {Payload: " name: old-app", Delta: difflib.LeftOnly}, + }, + }, + { + Key: "default, new-app, Deployment (apps)", + Kind: "Deployment", + ChangeType: "ADD", + Diffs: []difflib.DiffRecord{ + {Payload: "apiVersion: apps/v1", Delta: difflib.RightOnly}, + {Payload: "kind: Deployment", Delta: difflib.RightOnly}, + {Payload: "metadata:", Delta: difflib.RightOnly}, + {Payload: " name: new-app", Delta: difflib.RightOnly}, + }, + }, + }, + } + + modifyReport := &Report{ + Entries: []ReportEntry{ + { + Key: "default, app, Deployment (apps)", + Kind: "Deployment", + ChangeType: "MODIFY", + Diffs: []difflib.DiffRecord{ + {Payload: "apiVersion: apps/v1", Delta: difflib.Common}, + {Payload: "kind: Deployment", Delta: difflib.Common}, + {Payload: "metadata:", Delta: difflib.Common}, + {Payload: " name: app", Delta: difflib.Common}, + {Payload: " name: old-app", Delta: difflib.LeftOnly}, + {Payload: " name: new-app", Delta: difflib.RightOnly}, + }, + }, + }, + } + + var addRemoveBuf bytes.Buffer + printDyffReport(addRemoveReport, &addRemoveBuf) + addRemoveOutput := addRemoveBuf.String() + + var modifyBuf bytes.Buffer + printDyffReport(modifyReport, &modifyBuf) + modifyOutput := modifyBuf.String() + + require.NotEqual(t, addRemoveOutput, modifyOutput, + "ADD+REMOVE output should differ from MODIFY output to verify dyff does not merge them as a rename") + require.Contains(t, addRemoveOutput, "old-app") + require.Contains(t, addRemoveOutput, "new-app") +} + +func TestPrintDyffReportEmpty(t *testing.T) { + report := &Report{ + Entries: []ReportEntry{}, + } + + var buf bytes.Buffer + printDyffReport(report, &buf) + + output := buf.String() + require.Equal(t, "\n", output) +}