Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
6 changes: 3 additions & 3 deletions diff/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Comment on lines 111 to 115
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return error from ytbx.LoadFiles is currently ignored. If loading/parsing either temp file fails, dyff comparison may silently produce empty/incorrect output. Consider checking the error and writing a clear message (or falling back to the default diff output) instead of proceeding with a potentially invalid input state.

Copilot uses AI. Check for mistakes.
dyff.KubernetesEntityDetection(true),
dyff.DetectRenames(true),
)
report, _ := dyff.CompareInputFiles(currentInputFile, newInputFile, compareOptions...)
reportWriter := &dyff.HumanReport{
Report: report,
OmitHeader: true,
Expand Down
141 changes: 141 additions & 0 deletions diff/report_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package diff

import (
"bytes"
"testing"

"github.com/aryann/difflib"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -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")
}
Comment on lines +70 to +165
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new dyff tests don't assert the regression fix described in #632. In particular, TestPrintDyffReportWithAddAndRemove only checks non-empty output, so it would pass even if dyff still reorders/pairs removed+added resources (the original bug). Please strengthen this test (or add another) to assert that add/remove output is distinguishable from a rename-as-modify scenario (e.g., compare outputs for REMOVE+ADD vs a single MODIFY entry and assert they differ / contain expected identifiers).

Copilot uses AI. Check for mistakes.

func TestPrintDyffReportEmpty(t *testing.T) {
report := &Report{
Entries: []ReportEntry{},
}

var buf bytes.Buffer
printDyffReport(report, &buf)

output := buf.String()
require.Equal(t, "\n", output)
}
Comment on lines +167 to +177
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The empty-report assertion is very strict (expects exactly a single newline). Since dyff's human output formatting can change across dyff versions, this may be brittle. Consider asserting that the output is empty/whitespace-only (or that it contains no diff markers) rather than matching an exact string.

Copilot uses AI. Check for mistakes.
Loading