Skip to content

Commit 53f1aee

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #95 from lavalamp/fixvalue
reflect.DeepEqual, not even once
2 parents c23a8cb + e2d5a6e commit 53f1aee

File tree

6 files changed

+299
-8
lines changed

6 files changed

+299
-8
lines changed

‎schema/elements_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func TestResolve(t *testing.T) {
7878
t.Errorf("expected Atom %v, got %v", tt.expectAtom, atom)
7979
}
8080
if exist != tt.expectExist {
81-
t.Errorf("expeted exist %t, got %t", tt.expectExist, exist)
81+
t.Errorf("expected exist %t, got %t", tt.expectExist, exist)
8282
}
8383
})
8484
}

‎schema/equals.go‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
/*
2+
Copyright 2019 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package schema
18+
19+
// Equals returns true iff the two Schemas are equal.
20+
func (a Schema) Equals(b Schema) bool {
21+
if len(a.Types) != len(b.Types) {
22+
return false
23+
}
24+
for i := range a.Types {
25+
if !a.Types[i].Equals(b.Types[i]) {
26+
return false
27+
}
28+
}
29+
return true
30+
}
31+
32+
// Equals returns true iff the two TypeRefs are equal.
33+
//
34+
// Note that two typerefs that have an equivalent type but where one is
35+
// inlined and the other is named, are not considered equal.
36+
func (a TypeRef) Equals(b TypeRef) bool {
37+
if (a.NamedType == nil) != (b.NamedType == nil) {
38+
return false
39+
}
40+
if a.NamedType != nil {
41+
if *a.NamedType != *b.NamedType {
42+
return false
43+
}
44+
//return true
45+
}
46+
return a.Inlined.Equals(b.Inlined)
47+
}
48+
49+
// Equals returns true iff the two TypeDefs are equal.
50+
func (a TypeDef) Equals(b TypeDef) bool {
51+
if a.Name != b.Name {
52+
return false
53+
}
54+
return a.Atom.Equals(b.Atom)
55+
}
56+
57+
// Equals returns true iff the two Atoms are equal.
58+
func (a Atom) Equals(b Atom) bool {
59+
if (a.Scalar == nil) != (b.Scalar == nil) {
60+
return false
61+
}
62+
if (a.List == nil) != (b.List == nil) {
63+
return false
64+
}
65+
if (a.Map == nil) != (b.Map == nil) {
66+
return false
67+
}
68+
switch {
69+
case a.Scalar != nil:
70+
return *a.Scalar == *b.Scalar
71+
case a.List != nil:
72+
return a.List.Equals(*b.List)
73+
case a.Map != nil:
74+
return a.Map.Equals(*b.Map)
75+
}
76+
return true
77+
}
78+
79+
// Equals returns true iff the two Maps are equal.
80+
func (a Map) Equals(b Map) bool {
81+
if !a.ElementType.Equals(b.ElementType) {
82+
return false
83+
}
84+
if a.ElementRelationship != b.ElementRelationship {
85+
return false
86+
}
87+
if len(a.Fields) != len(b.Fields) {
88+
return false
89+
}
90+
for i := range a.Fields {
91+
if !a.Fields[i].Equals(b.Fields[i]) {
92+
return false
93+
}
94+
}
95+
if len(a.Unions) != len(b.Unions) {
96+
return false
97+
}
98+
for i := range a.Unions {
99+
if !a.Unions[i].Equals(b.Unions[i]) {
100+
return false
101+
}
102+
}
103+
return true
104+
}
105+
106+
// Equals returns true iff the two Unions are equal.
107+
func (a Union) Equals(b Union) bool {
108+
if (a.Discriminator == nil) != (b.Discriminator == nil) {
109+
return false
110+
}
111+
if a.Discriminator != nil {
112+
if *a.Discriminator != *b.Discriminator {
113+
return false
114+
}
115+
}
116+
if a.DeduceInvalidDiscriminator != b.DeduceInvalidDiscriminator {
117+
return false
118+
}
119+
if len(a.Fields) != len(b.Fields) {
120+
return false
121+
}
122+
for i := range a.Fields {
123+
if !a.Fields[i].Equals(b.Fields[i]) {
124+
return false
125+
}
126+
}
127+
return true
128+
}
129+
130+
// Equals returns true iff the two UnionFields are equal.
131+
func (a UnionField) Equals(b UnionField) bool {
132+
if a.FieldName != b.FieldName {
133+
return false
134+
}
135+
if a.DiscriminatorValue != b.DiscriminatorValue {
136+
return false
137+
}
138+
return true
139+
}
140+
141+
// Equals returns true iff the two StructFields are equal.
142+
func (a StructField) Equals(b StructField) bool {
143+
if a.Name != b.Name {
144+
return false
145+
}
146+
return a.Type.Equals(b.Type)
147+
}
148+
149+
// Equals returns true iff the two Lists are equal.
150+
func (a List) Equals(b List) bool {
151+
if !a.ElementType.Equals(b.ElementType) {
152+
return false
153+
}
154+
if a.ElementRelationship != b.ElementRelationship {
155+
return false
156+
}
157+
if len(a.Keys) != len(b.Keys) {
158+
return false
159+
}
160+
for i := range a.Keys {
161+
if a.Keys[i] != b.Keys[i] {
162+
return false
163+
}
164+
}
165+
return true
166+
}

‎schema/equals_test.go‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
/*
2+
Copyright 2019 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package schema
18+
19+
import (
20+
"reflect"
21+
"testing"
22+
"testing/quick"
23+
)
24+
25+
func TestEquals(t *testing.T) {
26+
// In general this test will make sure people update things when they
27+
// add a field.
28+
//
29+
// The "copy known fields" section of these function is to break if folks
30+
// add new fields without fixing the Equals function and this test.
31+
funcs := []interface{}{
32+
func(x Schema) bool {
33+
if !x.Equals(x) {
34+
return false
35+
}
36+
var y Schema
37+
y.Types = x.Types
38+
return x.Equals(y) == reflect.DeepEqual(x, y)
39+
},
40+
func(x TypeDef) bool {
41+
if !x.Equals(x) {
42+
return false
43+
}
44+
var y TypeDef
45+
y.Name = x.Name
46+
y.Atom = x.Atom
47+
return x.Equals(y) == reflect.DeepEqual(x, y)
48+
},
49+
func(x TypeRef) bool {
50+
if !x.Equals(x) {
51+
return false
52+
}
53+
var y TypeRef
54+
y.NamedType = x.NamedType
55+
y.Inlined = x.Inlined
56+
return x.Equals(y) == reflect.DeepEqual(x, y)
57+
},
58+
func(x Atom) bool {
59+
if !x.Equals(x) {
60+
return false
61+
}
62+
var y Atom
63+
y.Scalar = x.Scalar
64+
y.List = x.List
65+
y.Map = x.Map
66+
return x.Equals(y) == reflect.DeepEqual(x, y)
67+
},
68+
func(x Map) bool {
69+
if !x.Equals(x) {
70+
return false
71+
}
72+
var y Map
73+
y.ElementType = x.ElementType
74+
y.ElementRelationship = x.ElementRelationship
75+
y.Fields = x.Fields
76+
y.Unions = x.Unions
77+
return x.Equals(y) == reflect.DeepEqual(x, y)
78+
},
79+
func(x Union) bool {
80+
if !x.Equals(x) {
81+
return false
82+
}
83+
var y Union
84+
y.Discriminator = x.Discriminator
85+
y.DeduceInvalidDiscriminator = x.DeduceInvalidDiscriminator
86+
y.Fields = x.Fields
87+
return x.Equals(y) == reflect.DeepEqual(x, y)
88+
},
89+
func(x UnionField) bool {
90+
if !x.Equals(x) {
91+
return false
92+
}
93+
var y UnionField
94+
y.DiscriminatorValue = x.DiscriminatorValue
95+
y.FieldName = x.FieldName
96+
return x.Equals(y) == reflect.DeepEqual(x, y)
97+
},
98+
func(x StructField) bool {
99+
if !x.Equals(x) {
100+
return false
101+
}
102+
var y StructField
103+
y.Name = x.Name
104+
y.Type = x.Type
105+
return x.Equals(y) == reflect.DeepEqual(x, y)
106+
},
107+
func(x List) bool {
108+
if !x.Equals(x) {
109+
return false
110+
}
111+
var y List
112+
y.ElementType = x.ElementType
113+
y.ElementRelationship = x.ElementRelationship
114+
y.Keys = x.Keys
115+
return x.Equals(y) == reflect.DeepEqual(x, y)
116+
},
117+
}
118+
for i, f := range funcs {
119+
if err := quick.Check(f, nil); err != nil {
120+
t.Errorf("%v: %v", i, err)
121+
}
122+
}
123+
}

‎typed/merge.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ limitations under the License.
1717
package typed
1818

1919
import (
20-
"reflect"
21-
2220
"sigs.k8s.io/structured-merge-diff/fieldpath"
2321
"sigs.k8s.io/structured-merge-diff/schema"
2422
"sigs.k8s.io/structured-merge-diff/value"
@@ -78,7 +76,7 @@ func (w *mergingWalker) merge() (errs ValidationErrors) {
7876

7977
alhs := deduceAtom(a, w.lhs)
8078
arhs := deduceAtom(a, w.rhs)
81-
if reflect.DeepEqual(alhs, arhs) {
79+
if alhs.Equals(arhs) {
8280
errs = append(errs, handleAtom(arhs, w.typeRef, w)...)
8381
} else {
8482
w2 := *w

‎typed/typed.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package typed
1818

1919
import (
2020
"fmt"
21-
"reflect"
2221

2322
"sigs.k8s.io/structured-merge-diff/fieldpath"
2423
"sigs.k8s.io/structured-merge-diff/schema"
@@ -118,8 +117,8 @@ func (tv TypedValue) Compare(rhs *TypedValue) (c *Comparison, err error) {
118117
c.Added.Insert(w.path)
119118
} else if w.rhs == nil {
120119
c.Removed.Insert(w.path)
121-
} else if !reflect.DeepEqual(w.rhs, w.lhs) {
122-
// TODO: reflect.DeepEqual is not sufficient for this.
120+
} else if !w.rhs.Equals(*w.lhs) {
121+
// TODO: Equality is not sufficient for this.
123122
// Need to implement equality check on the value type.
124123
c.Modified.Insert(w.path)
125124
}
@@ -209,7 +208,7 @@ func merge(lhs, rhs *TypedValue, rule, postRule mergeRule) (*TypedValue, error)
209208
return nil, errorFormatter{}.
210209
errorf("expected objects with types from the same schema")
211210
}
212-
if !reflect.DeepEqual(lhs.typeRef, rhs.typeRef) {
211+
if !lhs.typeRef.Equals(rhs.typeRef) {
213212
return nil, errorFormatter{}.
214213
errorf("expected objects of the same type, but got %v and %v", lhs.typeRef, rhs.typeRef)
215214
}

‎value/value.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ type Value struct {
3434
Null bool // represents an explicit `"foo" = null`
3535
}
3636

37+
// Equals returns true iff the two values are equal.
38+
func (v Value) Equals(rhs Value) bool {
39+
return !v.Less(rhs) && !rhs.Less(v)
40+
}
41+
3742
// Less provides a total ordering for Value (so that they can be sorted, even
3843
// if they are of different types).
3944
func (v Value) Less(rhs Value) bool {

0 commit comments

Comments
 (0)