Skip to content

Commit 1ce0182

Browse files
yxxheroyxxhero
authored
fix: make --find-renames work with --output=dyff (#961)
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 <aiopsclub@163.com>
1 parent e1559c9 commit 1ce0182

3 files changed

Lines changed: 146 additions & 7 deletions

File tree

‎diff/diff.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,10 +256,8 @@ func doDiff(report *Report, key string, oldContent *manifest.MappingResult, newC
256256
}
257257
case newContent == nil:
258258
changeType = "REMOVE"
259-
if oldContent != nil {
260-
subjectKind = oldContent.Kind
261-
}
262-
if !options.StructuredOutput() && oldContent != nil {
259+
subjectKind = oldContent.Kind
260+
if !options.StructuredOutput() {
263261
emptyMapping := &manifest.MappingResult{}
264262
diffs = diffMappingResults(oldContent, emptyMapping, options.StripTrailingCR)
265263
}

‎diff/report.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,12 @@ func printDyffReport(r *Report, to io.Writer) {
110110

111111
currentInputFile, newInputFile, _ := ytbx.LoadFiles(currentFile.Name(), newFile.Name())
112112

113-
report, _ := dyff.CompareInputFiles(
114-
currentInputFile, newInputFile,
113+
var compareOptions []dyff.CompareOption
114+
compareOptions = append(compareOptions,
115115
dyff.IgnoreWhitespaceChanges(true),
116116
dyff.KubernetesEntityDetection(true),
117-
dyff.DetectRenames(true),
118117
)
118+
report, _ := dyff.CompareInputFiles(currentInputFile, newInputFile, compareOptions...)
119119
reportWriter := &dyff.HumanReport{
120120
Report: report,
121121
OmitHeader: true,

‎diff/report_test.go‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package diff
22

33
import (
4+
"bytes"
45
"testing"
56

7+
"github.com/aryann/difflib"
68
"github.com/stretchr/testify/require"
79
)
810

@@ -34,3 +36,142 @@ func TestLoadFromKey(t *testing.T) {
3436
require.Equal(t, expectedTemplateSpec, *templateSpec)
3537
}
3638
}
39+
40+
func TestPrintDyffReport(t *testing.T) {
41+
report := &Report{
42+
Entries: []ReportEntry{
43+
{
44+
Key: "default, nginx, Deployment (apps)",
45+
Kind: "Deployment",
46+
ChangeType: "MODIFY",
47+
Diffs: []difflib.DiffRecord{
48+
{Payload: "apiVersion: apps/v1", Delta: difflib.Common},
49+
{Payload: "kind: Deployment", Delta: difflib.Common},
50+
{Payload: "metadata:", Delta: difflib.Common},
51+
{Payload: " name: nginx", Delta: difflib.Common},
52+
{Payload: "spec:", Delta: difflib.Common},
53+
{Payload: " replicas: 2", Delta: difflib.LeftOnly},
54+
{Payload: " replicas: 3", Delta: difflib.RightOnly},
55+
},
56+
},
57+
},
58+
}
59+
60+
var buf bytes.Buffer
61+
printDyffReport(report, &buf)
62+
63+
output := buf.String()
64+
require.NotEmpty(t, output)
65+
require.Contains(t, output, "replicas", "Expected dyff output to mention replicas field")
66+
require.Contains(t, output, "- 2", "Expected dyff output to show original replicas value")
67+
require.Contains(t, output, "+ 3", "Expected dyff output to show updated replicas value")
68+
}
69+
70+
func TestPrintDyffReportWithAddAndRemove(t *testing.T) {
71+
report := &Report{
72+
Entries: []ReportEntry{
73+
{
74+
Key: "default, old-app, Deployment (apps)",
75+
Kind: "Deployment",
76+
ChangeType: "REMOVE",
77+
Diffs: []difflib.DiffRecord{
78+
{Payload: "apiVersion: apps/v1", Delta: difflib.LeftOnly},
79+
{Payload: "kind: Deployment", Delta: difflib.LeftOnly},
80+
{Payload: "metadata:", Delta: difflib.LeftOnly},
81+
{Payload: " name: old-app", Delta: difflib.LeftOnly},
82+
},
83+
},
84+
{
85+
Key: "default, new-app, Deployment (apps)",
86+
Kind: "Deployment",
87+
ChangeType: "ADD",
88+
Diffs: []difflib.DiffRecord{
89+
{Payload: "apiVersion: apps/v1", Delta: difflib.RightOnly},
90+
{Payload: "kind: Deployment", Delta: difflib.RightOnly},
91+
{Payload: "metadata:", Delta: difflib.RightOnly},
92+
{Payload: " name: new-app", Delta: difflib.RightOnly},
93+
},
94+
},
95+
},
96+
}
97+
98+
var buf bytes.Buffer
99+
printDyffReport(report, &buf)
100+
101+
output := buf.String()
102+
require.NotEmpty(t, output)
103+
require.Contains(t, output, "old-app", "Expected dyff output to show removed resource old-app")
104+
require.Contains(t, output, "new-app", "Expected dyff output to show added resource new-app")
105+
}
106+
107+
func TestPrintDyffReportDoesNotMergeAddRemove(t *testing.T) {
108+
addRemoveReport := &Report{
109+
Entries: []ReportEntry{
110+
{
111+
Key: "default, old-app, Deployment (apps)",
112+
Kind: "Deployment",
113+
ChangeType: "REMOVE",
114+
Diffs: []difflib.DiffRecord{
115+
{Payload: "apiVersion: apps/v1", Delta: difflib.LeftOnly},
116+
{Payload: "kind: Deployment", Delta: difflib.LeftOnly},
117+
{Payload: "metadata:", Delta: difflib.LeftOnly},
118+
{Payload: " name: old-app", Delta: difflib.LeftOnly},
119+
},
120+
},
121+
{
122+
Key: "default, new-app, Deployment (apps)",
123+
Kind: "Deployment",
124+
ChangeType: "ADD",
125+
Diffs: []difflib.DiffRecord{
126+
{Payload: "apiVersion: apps/v1", Delta: difflib.RightOnly},
127+
{Payload: "kind: Deployment", Delta: difflib.RightOnly},
128+
{Payload: "metadata:", Delta: difflib.RightOnly},
129+
{Payload: " name: new-app", Delta: difflib.RightOnly},
130+
},
131+
},
132+
},
133+
}
134+
135+
modifyReport := &Report{
136+
Entries: []ReportEntry{
137+
{
138+
Key: "default, app, Deployment (apps)",
139+
Kind: "Deployment",
140+
ChangeType: "MODIFY",
141+
Diffs: []difflib.DiffRecord{
142+
{Payload: "apiVersion: apps/v1", Delta: difflib.Common},
143+
{Payload: "kind: Deployment", Delta: difflib.Common},
144+
{Payload: "metadata:", Delta: difflib.Common},
145+
{Payload: " name: app", Delta: difflib.Common},
146+
{Payload: " name: old-app", Delta: difflib.LeftOnly},
147+
{Payload: " name: new-app", Delta: difflib.RightOnly},
148+
},
149+
},
150+
},
151+
}
152+
153+
var addRemoveBuf bytes.Buffer
154+
printDyffReport(addRemoveReport, &addRemoveBuf)
155+
addRemoveOutput := addRemoveBuf.String()
156+
157+
var modifyBuf bytes.Buffer
158+
printDyffReport(modifyReport, &modifyBuf)
159+
modifyOutput := modifyBuf.String()
160+
161+
require.NotEqual(t, addRemoveOutput, modifyOutput,
162+
"ADD+REMOVE output should differ from MODIFY output to verify dyff does not merge them as a rename")
163+
require.Contains(t, addRemoveOutput, "old-app")
164+
require.Contains(t, addRemoveOutput, "new-app")
165+
}
166+
167+
func TestPrintDyffReportEmpty(t *testing.T) {
168+
report := &Report{
169+
Entries: []ReportEntry{},
170+
}
171+
172+
var buf bytes.Buffer
173+
printDyffReport(report, &buf)
174+
175+
output := buf.String()
176+
require.Equal(t, "\n", output)
177+
}

0 commit comments

Comments
 (0)