Skip to content

Commit 6c36ce0

Browse files
committed
Fix incorrect diff for mixing types in a list
Bump `neat` to include fix for quoting numbers. Include scalar tag in hash calculation to differentiate between types. Simplify variables declarations. Move `createLookUpMap` function into `simpleLists` to avoid unnecessary method on `compare`, since it's only used in one function.
1 parent 92a8c68 commit 6c36ce0

7 files changed

Lines changed: 43 additions & 28 deletions

File tree

assets/issues/issue-580/from.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
list:
2+
- 123
3+
- 123

assets/issues/issue-580/to.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
list:
2+
- "123"
3+
- 123

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ require (
66
github.com/caarlos0/env/v11 v11.4.0
77
github.com/gonvenience/bunt v1.4.3
88
github.com/gonvenience/idem v0.0.3
9-
github.com/gonvenience/neat v1.3.17
9+
github.com/gonvenience/neat v1.3.18
1010
github.com/gonvenience/term v1.0.5
1111
github.com/gonvenience/text v1.0.10
1212
github.com/gonvenience/ytbx v1.4.8

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ github.com/gonvenience/idem v0.0.3 h1:rZ2f17JU5GHa3b5M5R2fClz0dYN3EFGhHHGo3AZz/1
2929
github.com/gonvenience/idem v0.0.3/go.mod h1:ChZ+RP8e30+uCBcCIzN/0di6lTO2PucjemgKfzQUQEw=
3030
github.com/gonvenience/neat v1.3.17 h1:S/F0XNE4sc/b3APfLDqC9xM476Or55WhXZ7F/Sy07QY=
3131
github.com/gonvenience/neat v1.3.17/go.mod h1:h+b8M0LFDZUKS5D4xaoPd2qTLrVpX+bDRlDJphOAS/s=
32+
github.com/gonvenience/neat v1.3.18 h1:WxWoXhsTHA6CStNrGgSEjGTt5MwIm+7Xs+VZmQIuXZA=
33+
github.com/gonvenience/neat v1.3.18/go.mod h1:DTaEyHIOjSkMa066EoZZl3k5KCG/rFGE67n0cjm/9qk=
3234
github.com/gonvenience/term v1.0.5 h1:PYfBH7FB1V+tuuJl4KYrqG/tzAOUnvTy8IFa9YqYrJY=
3335
github.com/gonvenience/term v1.0.5/go.mod h1:CYvcU7H3nE6eOP0gvGfYz4BjGJzM1GeNp+fx4IBWKLs=
3436
github.com/gonvenience/text v1.0.10 h1:QRqtC/KMk57K7y4jHi4HjLxf8u+tg+/tIRCS5afywNE=

internal/cmd/cmds_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ type-change-1
326326
327327
type-change-2
328328
± type change from string to int
329-
- 12
329+
- "12"
330330
+ 12
331331
332332
whitespaces

pkg/dyff/compare_test.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -937,7 +937,7 @@ b: bar
937937
})
938938
})
939939

940-
Context("checking known issues of compare", func() {
940+
Context("checking known/reported compare issues", func() {
941941
It("should not return order change differences in case the named-entry list does not have unique identifiers", func() {
942942
from, to, err := ytbx.LoadFiles("../../assets/issues/issue-38/from.yml", "../../assets/issues/issue-38/to.yml")
943943
Expect(err).To(BeNil())
@@ -1025,7 +1025,6 @@ b: bar
10251025
})
10261026

10271027
It("should work with non-standard identifier containing dots and slashes in named entry lists", func() {
1028-
assets()
10291028
from, to, err := ytbx.LoadFiles(assets("issues/issue-605/from.yml"), assets("issues/issue-605/to.yml"))
10301029
Expect(err).ToNot(HaveOccurred())
10311030
Expect(from).ToNot(BeNil())
@@ -1036,6 +1035,18 @@ b: bar
10361035
Expect(results).ToNot(BeNil())
10371036
Expect(results.Diffs).To(HaveLen(0))
10381037
})
1038+
1039+
It("should differentiate correctly between number as string and number", func() {
1040+
from, to, err := ytbx.LoadFiles(assets("issues/issue-580/from.yml"), assets("issues/issue-580/to.yml"))
1041+
Expect(err).ToNot(HaveOccurred())
1042+
Expect(from).ToNot(BeNil())
1043+
Expect(to).ToNot(BeNil())
1044+
1045+
results, err := dyff.CompareInputFiles(from, to)
1046+
Expect(err).ToNot(HaveOccurred())
1047+
Expect(results).ToNot(BeNil())
1048+
Expect(results.Diffs).To(HaveLen(1))
1049+
})
10391050
})
10401051

10411052
Context("input files containing Kubernetes resources", func() {

pkg/dyff/core.go

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -523,11 +523,8 @@ func (compare *compare) sequenceNodes(path ytbx.Path, from *yamlv3.Node, to *yam
523523
}
524524

525525
func (compare *compare) simpleLists(path ytbx.Path, from *yamlv3.Node, to *yamlv3.Node) ([]Diff, error) {
526-
removals := make([]*yamlv3.Node, 0)
527-
additions := make([]*yamlv3.Node, 0)
528-
529-
fromLength := len(from.Content)
530-
toLength := len(to.Content)
526+
var removals, additions []*yamlv3.Node
527+
var fromLength, toLength = len(from.Content), len(to.Content)
531528

532529
// Special case if both lists only contain one entry, then directly compare
533530
// the two entries with each other
@@ -539,12 +536,25 @@ func (compare *compare) simpleLists(path ytbx.Path, from *yamlv3.Node, to *yamlv
539536
)
540537
}
541538

542-
fromLookup := compare.createLookUpMap(from)
543-
toLookup := compare.createLookUpMap(to)
539+
var createLookUpMap = func(sequenceNode *yamlv3.Node) map[uint64][]int {
540+
var result = make(map[uint64][]int, len(sequenceNode.Content))
541+
for idx, entry := range sequenceNode.Content {
542+
var hash = compare.calcNodeHash(entry)
543+
if _, ok := result[hash]; !ok {
544+
result[hash] = []int{}
545+
}
546+
547+
result[hash] = append(result[hash], idx)
548+
}
549+
550+
return result
551+
}
552+
553+
fromLookup := createLookUpMap(from)
554+
toLookup := createLookUpMap(to)
544555

545556
// Fill two lists with the hashes of the entries of each list
546-
fromCommon := make([]*yamlv3.Node, 0, fromLength)
547-
toCommon := make([]*yamlv3.Node, 0, toLength)
557+
var fromCommon, toCommon []*yamlv3.Node
548558

549559
for idxPos, fromValue := range from.Content {
550560
hash := compare.calcNodeHash(fromValue)
@@ -1038,20 +1048,6 @@ func isEmptyDocument(node *yamlv3.Node) bool {
10381048
return false
10391049
}
10401050

1041-
func (compare *compare) createLookUpMap(sequenceNode *yamlv3.Node) map[uint64][]int {
1042-
result := make(map[uint64][]int, len(sequenceNode.Content))
1043-
for idx, entry := range sequenceNode.Content {
1044-
hash := compare.calcNodeHash(entry)
1045-
if _, ok := result[hash]; !ok {
1046-
result[hash] = []int{}
1047-
}
1048-
1049-
result[hash] = append(result[hash], idx)
1050-
}
1051-
1052-
return result
1053-
}
1054-
10551051
func (compare *compare) basicType(node *yamlv3.Node) interface{} {
10561052
switch node.Kind {
10571053
case yamlv3.DocumentNode:
@@ -1098,7 +1094,7 @@ func (compare *compare) calcNodeHash(node *yamlv3.Node) (hash uint64) {
10981094
hash, err = hashstructure.Hash(compare.basicType(node), nil)
10991095

11001096
case yamlv3.ScalarNode:
1101-
hash, err = hashstructure.Hash(node.Value, nil)
1097+
hash, err = hashstructure.Hash(node.Tag+"/"+node.Value, nil)
11021098

11031099
case yamlv3.AliasNode:
11041100
hash = compare.calcNodeHash(followAlias(node))

0 commit comments

Comments
 (0)