Skip to content

Commit ba9e574

Browse files
author
Antoine Pelisse
committed
Fix bogus conflicts comparison and reporting
A couple of things that are broken with conflicts: - We're comparing strings with DeepEqual, which does care about the order (and we're more considering it like a set, so it's sort-of random). - When we print the error, we don't distinguish between errors and conflict errors, leading to a confusing message. - When there is a difference we don't report specifically what it is, so you have to match the sets yourself, which is unhelpful. Address all of these by having a comparison function that helps us detect what has changed, so that we can compare and report properly.
1 parent 81b61da commit ba9e574

File tree

1 file changed

+37
-2
lines changed

1 file changed

+37
-2
lines changed

internal/fixture/state.go

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,25 @@ type Operation interface {
152152
run(*State) error
153153
}
154154

155+
func hasConflict(conflicts merge.Conflicts, conflict merge.Conflict) bool {
156+
for i := range conflicts {
157+
if reflect.DeepEqual(conflict, conflicts[i]) {
158+
return true
159+
}
160+
}
161+
return false
162+
}
163+
164+
func addedConflicts(one, other merge.Conflicts) merge.Conflicts {
165+
added := merge.Conflicts{}
166+
for _, conflict := range other {
167+
if !hasConflict(one, conflict) {
168+
added = append(added, conflict)
169+
}
170+
}
171+
return added
172+
}
173+
155174
// Apply is a type of operation. It is a non-forced apply run by a
156175
// manager with a given object. Since non-forced apply operation can
157176
// conflict, the user can specify the expected conflicts. If conflicts
@@ -167,8 +186,24 @@ var _ Operation = &Apply{}
167186

168187
func (a Apply) run(state *State) error {
169188
err := state.Apply(a.Object, a.APIVersion, a.Manager, false)
170-
if (err != nil || a.Conflicts != nil) && !reflect.DeepEqual(err, a.Conflicts) {
171-
return fmt.Errorf("expected conflicts: %v, got %v", a.Conflicts, err)
189+
if err != nil {
190+
if _, ok := err.(merge.Conflicts); !ok {
191+
return err
192+
}
193+
}
194+
if a.Conflicts != nil {
195+
conflicts := merge.Conflicts{}
196+
if err != nil {
197+
conflicts = err.(merge.Conflicts)
198+
}
199+
if len(addedConflicts(a.Conflicts, conflicts)) != 0 || len(addedConflicts(conflicts, a.Conflicts)) != 0 {
200+
return fmt.Errorf("Expected conflicts:\n%v\ngot\n%v\nadded:\n%v\nremoved:\n%v",
201+
a.Conflicts.Error(),
202+
conflicts.Error(),
203+
addedConflicts(a.Conflicts, conflicts).Error(),
204+
addedConflicts(conflicts, a.Conflicts).Error(),
205+
)
206+
}
172207
}
173208
return nil
174209

0 commit comments

Comments
 (0)