mirror of https://github.com/knative/pkg.git
Undo the patch sorting. (#192)
My prior change added sorting to the duck.CreatePatch method to try and stabilize the result of jsonpatch.CreatePatch, which is otherwise non-deterministic (probably walking a map?). My bad assumption was that the patch operations this generated wouldn't conflict, e.g. it should use `replace` vs. `remove` and `add`. Clearly this was bad because we start getting really strange errors trying to import this into knative/serving, e.g. https://gubernator.knative.dev/build/knative-prow/pr-logs/pull/knative_serving/2646/pull-knative-serving-integration-tests/1070435951391543298/
This commit is contained in:
parent
c267dfecb7
commit
0e41760cea
|
@ -18,7 +18,6 @@ package duck
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"sort"
|
|
||||||
|
|
||||||
jsonmergepatch "github.com/evanphx/json-patch"
|
jsonmergepatch "github.com/evanphx/json-patch"
|
||||||
"github.com/mattbaird/jsonpatch"
|
"github.com/mattbaird/jsonpatch"
|
||||||
|
@ -51,20 +50,7 @@ func CreatePatch(before, after interface{}) (JSONPatch, error) {
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
patch, err := jsonpatch.CreatePatch(rawBefore, rawAfter)
|
return jsonpatch.CreatePatch(rawBefore, rawAfter)
|
||||||
if err != nil {
|
|
||||||
return nil, err
|
|
||||||
}
|
|
||||||
|
|
||||||
// Give the patch a deterministic ordering.
|
|
||||||
sort.Slice(patch, func(i, j int) bool {
|
|
||||||
lhs, rhs := patch[i], patch[j]
|
|
||||||
if lhs.Operation != rhs.Operation {
|
|
||||||
return lhs.Operation < rhs.Operation
|
|
||||||
}
|
|
||||||
return lhs.Path < rhs.Path
|
|
||||||
})
|
|
||||||
return patch, nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
type JSONPatch []jsonpatch.JsonPatchOperation
|
type JSONPatch []jsonpatch.JsonPatchOperation
|
||||||
|
|
|
@ -163,12 +163,12 @@ func TestCreatePatch(t *testing.T) {
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
want: JSONPatch{{
|
want: JSONPatch{{
|
||||||
Operation: "remove",
|
|
||||||
Path: "/status/patchable/field2",
|
|
||||||
}, {
|
|
||||||
Operation: "replace",
|
Operation: "replace",
|
||||||
Path: "/status/patchable/field1",
|
Path: "/status/patchable/field1",
|
||||||
Value: 42.0,
|
Value: 42.0,
|
||||||
|
}, {
|
||||||
|
Operation: "remove",
|
||||||
|
Path: "/status/patchable/field2",
|
||||||
}},
|
}},
|
||||||
}, {
|
}, {
|
||||||
name: "patch array",
|
name: "patch array",
|
||||||
|
|
|
@ -22,6 +22,7 @@ import (
|
||||||
"encoding/pem"
|
"encoding/pem"
|
||||||
"fmt"
|
"fmt"
|
||||||
"reflect"
|
"reflect"
|
||||||
|
"sort"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
@ -486,6 +487,24 @@ func expectPatches(t *testing.T, a []byte, e []jsonpatch.JsonPatchOperation) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Give the patch a deterministic ordering.
|
||||||
|
// Technically this can change the meaning, but the ordering is otherwise unstable
|
||||||
|
// and difficult to test.
|
||||||
|
sort.Slice(e, func(i, j int) bool {
|
||||||
|
lhs, rhs := e[i], e[j]
|
||||||
|
if lhs.Operation != rhs.Operation {
|
||||||
|
return lhs.Operation < rhs.Operation
|
||||||
|
}
|
||||||
|
return lhs.Path < rhs.Path
|
||||||
|
})
|
||||||
|
sort.Slice(got, func(i, j int) bool {
|
||||||
|
lhs, rhs := got[i], got[j]
|
||||||
|
if lhs.Operation != rhs.Operation {
|
||||||
|
return lhs.Operation < rhs.Operation
|
||||||
|
}
|
||||||
|
return lhs.Path < rhs.Path
|
||||||
|
})
|
||||||
|
|
||||||
if diff := cmp.Diff(e, got, cmpopts.EquateEmpty()); diff != "" {
|
if diff := cmp.Diff(e, got, cmpopts.EquateEmpty()); diff != "" {
|
||||||
t.Errorf("expectPatches (-want, +got) = %v", diff)
|
t.Errorf("expectPatches (-want, +got) = %v", diff)
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue