Skip to content

Commit 16ed41d

Browse files
authored
fix: enable dyff rename detection when --find-renames is set (#980)
* fix: enable dyff rename detection when --find-renames is set The condition guarding dyff.DetectRenames(true) was inverted: it enabled rename detection only when FindRenames was NOT set (<= 0) and disabled it when the user explicitly requested it via --find-renames. Flip the condition from <= 0 to > 0 so that dyff's rename detection is enabled when --find-renames is set to any positive value, matching the documented behavior. Fixes #976 Signed-off-by: yxxhero <aiopsclub@163.com> * fix: propagate FindRenames to Report and update tests Pass FindRenames from Options to Report so that printDyffReport can use it to conditionally enable dyff.DetectRenames. Update test name and add FindRenames field to test reports for consistency. Signed-off-by: yxxhero <aiopsclub@163.com> * fix: preserve FindRenames in doSuppress and add regression test Copy FindRenames into the filtered report in doSuppress so that --find-renames still works when --suppress-output-line-regex is used. Add TestPrintDyffReportFindRenamesChangesOutput to verify that dyff output differs between FindRenames == 0 and FindRenames > 0. Signed-off-by: yxxhero <aiopsclub@163.com> * refactor: make Report.findRenames unexported and improve tests Make findRenames an unexported field on Report since it is internal state for dyff printing, not part of the public API. Rename TestPrintDyffReportDoesNotMergeAddRemoveWithFindRenames to TestPrintDyffReportAddRemoveDiffersFromModify for clarity. Replace the brittle NotEqual assertion in the rename detection test with specific assertions on stable dyff output markers (presence of 'one document removed/added' vs 'value change') to reliably verify that rename detection is toggled by findRenames. Signed-off-by: yxxhero <aiopsclub@163.com> --------- Signed-off-by: yxxhero <aiopsclub@163.com>
1 parent a2a9488 commit 16ed41d

3 files changed

Lines changed: 63 additions & 6 deletions

File tree

diff/diff.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func ManifestReport(oldIndex, newIndex map[string]*manifest.MappingResult, optio
6565
}
6666

6767
func generateReport(oldIndex, newIndex map[string]*manifest.MappingResult, newOwnedReleases map[string]OwnershipDiff, options *Options) (bool, *Report, error) {
68-
report := Report{}
68+
report := Report{findRenames: options.FindRenames}
6969
report.setupReportFormat(options.OutputFormat)
7070
var possiblyRemoved []string
7171

@@ -118,7 +118,9 @@ func doSuppress(report Report, suppressedOutputLineRegex []string) (Report, erro
118118
return report, nil
119119
}
120120

121-
filteredReport := Report{}
121+
filteredReport := Report{
122+
findRenames: report.findRenames,
123+
}
122124
filteredReport.format = report.format
123125
filteredReport.Entries = []ReportEntry{}
124126

diff/report.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@ import (
2020

2121
// Report to store report data and format
2222
type Report struct {
23-
format ReportFormat
24-
Entries []ReportEntry
25-
mode string
23+
format ReportFormat
24+
Entries []ReportEntry
25+
mode string
26+
findRenames float32
2627
}
2728

2829
// ReportEntry to store changes between releases
@@ -115,6 +116,9 @@ func printDyffReport(r *Report, to io.Writer) {
115116
dyff.IgnoreWhitespaceChanges(true),
116117
dyff.KubernetesEntityDetection(true),
117118
)
119+
if r.findRenames > 0 {
120+
compareOptions = append(compareOptions, dyff.DetectRenames(true))
121+
}
118122
report, _ := dyff.CompareInputFiles(currentInputFile, newInputFile, compareOptions...)
119123
reportWriter := &dyff.HumanReport{
120124
Report: report,

diff/report_test.go

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,9 @@ func TestPrintDyffReportWithAddAndRemove(t *testing.T) {
104104
require.Contains(t, output, "new-app", "Expected dyff output to show added resource new-app")
105105
}
106106

107-
func TestPrintDyffReportDoesNotMergeAddRemove(t *testing.T) {
107+
func TestPrintDyffReportAddRemoveDiffersFromModify(t *testing.T) {
108108
addRemoveReport := &Report{
109+
findRenames: 1.0,
109110
Entries: []ReportEntry{
110111
{
111112
Key: "default, old-app, Deployment (apps)",
@@ -133,6 +134,7 @@ func TestPrintDyffReportDoesNotMergeAddRemove(t *testing.T) {
133134
}
134135

135136
modifyReport := &Report{
137+
findRenames: 1.0,
136138
Entries: []ReportEntry{
137139
{
138140
Key: "default, app, Deployment (apps)",
@@ -164,6 +166,55 @@ func TestPrintDyffReportDoesNotMergeAddRemove(t *testing.T) {
164166
require.Contains(t, addRemoveOutput, "new-app")
165167
}
166168

169+
func TestPrintDyffReportRenameDetectionEnabledWithFindRenames(t *testing.T) {
170+
entries := []ReportEntry{
171+
{
172+
Key: "default, old-app, Deployment (apps)",
173+
Kind: "Deployment",
174+
ChangeType: "REMOVE",
175+
Diffs: []difflib.DiffRecord{
176+
{Payload: "apiVersion: apps/v1", Delta: difflib.LeftOnly},
177+
{Payload: "kind: Deployment", Delta: difflib.LeftOnly},
178+
{Payload: "metadata:", Delta: difflib.LeftOnly},
179+
{Payload: " name: old-app", Delta: difflib.LeftOnly},
180+
{Payload: "spec:", Delta: difflib.LeftOnly},
181+
{Payload: " replicas: 3", Delta: difflib.LeftOnly},
182+
},
183+
},
184+
{
185+
Key: "default, new-app, Deployment (apps)",
186+
Kind: "Deployment",
187+
ChangeType: "ADD",
188+
Diffs: []difflib.DiffRecord{
189+
{Payload: "apiVersion: apps/v1", Delta: difflib.RightOnly},
190+
{Payload: "kind: Deployment", Delta: difflib.RightOnly},
191+
{Payload: "metadata:", Delta: difflib.RightOnly},
192+
{Payload: " name: new-app", Delta: difflib.RightOnly},
193+
{Payload: "spec:", Delta: difflib.RightOnly},
194+
{Payload: " replicas: 3", Delta: difflib.RightOnly},
195+
},
196+
},
197+
}
198+
199+
var noRenameBuf bytes.Buffer
200+
printDyffReport(&Report{findRenames: 0, Entries: entries}, &noRenameBuf)
201+
noRenameOutput := noRenameBuf.String()
202+
203+
require.Contains(t, noRenameOutput, "one document removed",
204+
"Without findRenames, dyff should report a separate document removal")
205+
require.Contains(t, noRenameOutput, "one document added",
206+
"Without findRenames, dyff should report a separate document addition")
207+
208+
var withRenameBuf bytes.Buffer
209+
printDyffReport(&Report{findRenames: 1.0, Entries: entries}, &withRenameBuf)
210+
withRenameOutput := withRenameBuf.String()
211+
212+
require.NotContains(t, withRenameOutput, "one document removed",
213+
"With findRenames > 0, dyff should match documents as a rename, not report removal")
214+
require.Contains(t, withRenameOutput, "value change",
215+
"With findRenames > 0, dyff should show the matched rename as a value change")
216+
}
217+
167218
func TestPrintDyffReportEmpty(t *testing.T) {
168219
report := &Report{
169220
Entries: []ReportEntry{},

0 commit comments

Comments
 (0)