Replace apimachinery diff.StringDiff with cmp

I forced failures of most of these tests and verified readability
This commit is contained in:
Tim Hockin 2023-03-23 11:10:32 -07:00
parent bd24043551
commit 97bab44cae
No known key found for this signature in database
10 changed files with 23 additions and 80 deletions

View File

@ -23,13 +23,12 @@ import (
"testing" "testing"
"github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp"
"github.com/google/gofuzz" fuzz "github.com/google/gofuzz"
"k8s.io/apimachinery/pkg/api/apitesting/fuzzer" "k8s.io/apimachinery/pkg/api/apitesting/fuzzer"
"k8s.io/apimachinery/pkg/api/apitesting/roundtrip" "k8s.io/apimachinery/pkg/api/apitesting/roundtrip"
"k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/api/legacyscheme"
) )
@ -71,7 +70,7 @@ func doDeepCopyTest(t *testing.T, kind schema.GroupVersionKind, f *fuzz.Fuzzer)
} }
if !bytes.Equal(prefuzzData.Bytes(), postfuzzData.Bytes()) { if !bytes.Equal(prefuzzData.Bytes(), postfuzzData.Bytes()) {
t.Log(diff.StringDiff(prefuzzData.String(), postfuzzData.String())) t.Log(cmp.Diff(prefuzzData.String(), postfuzzData.String()))
t.Errorf("Fuzzing copy modified original of %#v", kind) t.Errorf("Fuzzing copy modified original of %#v", kind)
return return
} }

View File

@ -22,13 +22,13 @@ import (
"reflect" "reflect"
"testing" "testing"
"github.com/google/go-cmp/cmp"
"sigs.k8s.io/yaml" "sigs.k8s.io/yaml"
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1" rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
"k8s.io/component-helpers/auth/rbac/validation" "k8s.io/component-helpers/auth/rbac/validation"
"k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/api/legacyscheme"
@ -253,7 +253,7 @@ func testObjects(t *testing.T, list *api.List, fixtureFilename string) {
t.Logf("Could not update data in %s: %v", filename, err) t.Logf("Could not update data in %s: %v", filename, err)
} }
} else { } else {
t.Logf("Diff between bootstrap data and fixture data in %s:\n-------------\n%s", filename, diff.StringDiff(string(yamlData), string(expectedYAML))) t.Logf("Diff between bootstrap data and fixture data in %s:\n-------------\n%s", filename, cmp.Diff(string(yamlData), string(expectedYAML)))
t.Logf("If the change is expected, re-run with %s=true to update the fixtures", updateEnvVar) t.Logf("If the change is expected, re-run with %s=true to update the fixtures", updateEnvVar)
} }
} }

View File

@ -21,13 +21,13 @@ import (
"strings" "strings"
"testing" "testing"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
apiextensionsinternal "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apiextensionsinternal "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/apimachinery/pkg/util/json" "k8s.io/apimachinery/pkg/util/json"
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apiserver/pkg/endpoints" "k8s.io/apiserver/pkg/endpoints"
@ -438,7 +438,7 @@ func TestNewBuilder(t *testing.T) {
gotListSchema := got.listSchema.Properties["items"].Items.Schema gotListSchema := got.listSchema.Properties["items"].Items.Schema
if !reflect.DeepEqual(&wantedItemsSchema, gotListSchema) { if !reflect.DeepEqual(&wantedItemsSchema, gotListSchema) {
t.Errorf("unexpected list schema: %s (want/got)", schemaDiff(&wantedItemsSchema, gotListSchema)) t.Errorf("unexpected list schema:\n%s", schemaDiff(&wantedItemsSchema, gotListSchema))
} }
}) })
} }
@ -560,15 +560,8 @@ func properties(p map[string]spec.Schema) sets.String {
} }
func schemaDiff(a, b *spec.Schema) string { func schemaDiff(a, b *spec.Schema) string {
as, err := json.Marshal(a) // This option construct allows diffing all fields, even unexported ones.
if err != nil { return cmp.Diff(a, b, cmp.Exporter(func(reflect.Type) bool { return true }))
panic(err)
}
bs, err := json.Marshal(b)
if err != nil {
panic(err)
}
return diff.StringDiff(string(as), string(bs))
} }
func TestBuildOpenAPIV2(t *testing.T) { func TestBuildOpenAPIV2(t *testing.T) {

View File

@ -27,30 +27,16 @@ import (
"k8s.io/apimachinery/pkg/util/dump" "k8s.io/apimachinery/pkg/util/dump"
) )
// StringDiff diffs a and b and returns a human readable diff.
func StringDiff(a, b string) string {
ba := []byte(a)
bb := []byte(b)
out := []byte{}
i := 0
for ; i < len(ba) && i < len(bb); i++ {
if ba[i] != bb[i] {
break
}
out = append(out, ba[i])
}
out = append(out, []byte("\n\nA: ")...)
out = append(out, ba[i:]...)
out = append(out, []byte("\n\nB: ")...)
out = append(out, bb[i:]...)
out = append(out, []byte("\n\n")...)
return string(out)
}
func legacyDiff(a, b interface{}) string { func legacyDiff(a, b interface{}) string {
return cmp.Diff(a, b) return cmp.Diff(a, b)
} }
// StringDiff diffs a and b and returns a human readable diff.
// DEPRECATED: use github.com/google/go-cmp/cmp.Diff
func StringDiff(a, b string) string {
return legacyDiff(a, b)
}
// ObjectDiff prints the diff of two go objects and fails if the objects // ObjectDiff prints the diff of two go objects and fails if the objects
// contain unhandled unexported fields. // contain unhandled unexported fields.
// DEPRECATED: use github.com/google/go-cmp/cmp.Diff // DEPRECATED: use github.com/google/go-cmp/cmp.Diff

View File

@ -1,29 +0,0 @@
/*
Copyright 2016 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package diff
import (
"testing"
)
func TestStringDiff(t *testing.T) {
diff := StringDiff("aaabb", "aaacc")
expect := "aaa\n\nA: bb\n\nB: cc\n\n"
if diff != expect {
t.Errorf("diff returned %v", diff)
}
}

View File

@ -2061,12 +2061,6 @@ func TestWatchTable(t *testing.T) {
actual = append(actual, &event) actual = append(actual, &event)
} }
if !reflect.DeepEqual(test.expected, actual) { if !reflect.DeepEqual(test.expected, actual) {
for i := range test.expected {
if i >= len(actual) {
break
}
t.Logf("%s", diff.StringDiff(string(test.expected[i].Object.Raw), string(actual[i].Object.Raw)))
}
t.Fatalf("unexpected: %s", diff.ObjectReflectDiff(test.expected, actual)) t.Fatalf("unexpected: %s", diff.ObjectReflectDiff(test.expected, actual))
} }
}) })

View File

@ -27,12 +27,12 @@ import (
"strings" "strings"
"testing" "testing"
"github.com/google/go-cmp/cmp"
"github.com/spf13/cobra" "github.com/spf13/cobra"
yaml "gopkg.in/yaml.v2" yaml "gopkg.in/yaml.v2"
"k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
"k8s.io/cli-runtime/pkg/genericiooptions" "k8s.io/cli-runtime/pkg/genericiooptions"
"k8s.io/cli-runtime/pkg/resource" "k8s.io/cli-runtime/pkg/resource"
@ -124,7 +124,7 @@ func TestEdit(t *testing.T) {
// Convenience to allow recapturing the input and persisting it here // Convenience to allow recapturing the input and persisting it here
os.WriteFile(inputFile, body, os.FileMode(0644)) os.WriteFile(inputFile, body, os.FileMode(0644))
} else { } else {
t.Errorf("%s, step %d: diff in edit content:\n%s", name, i, diff.StringDiff(string(body), string(expectedInput))) t.Errorf("%s, step %d: diff in edit content:\n%s", name, i, cmp.Diff(string(body), string(expectedInput)))
t.Logf("If the change in input is expected, rerun tests with %s=true to update input fixtures", updateEnvVar) t.Logf("If the change in input is expected, rerun tests with %s=true to update input fixtures", updateEnvVar)
} }
} }
@ -147,7 +147,7 @@ func TestEdit(t *testing.T) {
// Convenience to allow recapturing the input and persisting it here // Convenience to allow recapturing the input and persisting it here
os.WriteFile(inputFile, body, os.FileMode(0644)) os.WriteFile(inputFile, body, os.FileMode(0644))
} else { } else {
t.Errorf("%s, step %d: diff in edit content:\n%s", name, i, diff.StringDiff(string(body), string(expectedInput))) t.Errorf("%s, step %d: diff in edit content:\n%s", name, i, cmp.Diff(string(body), string(expectedInput)))
t.Logf("If the change in input is expected, rerun tests with %s=true to update input fixtures", updateEnvVar) t.Logf("If the change in input is expected, rerun tests with %s=true to update input fixtures", updateEnvVar)
} }
} }

View File

@ -26,6 +26,7 @@ import (
"strings" "strings"
"testing" "testing"
"github.com/google/go-cmp/cmp"
appsv1 "k8s.io/api/apps/v1" appsv1 "k8s.io/api/apps/v1"
autoscalingv1 "k8s.io/api/autoscaling/v1" autoscalingv1 "k8s.io/api/autoscaling/v1"
batchv1 "k8s.io/api/batch/v1" batchv1 "k8s.io/api/batch/v1"
@ -36,7 +37,6 @@ import (
metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1"
"k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer/streaming" "k8s.io/apimachinery/pkg/runtime/serializer/streaming"
"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/apimachinery/pkg/watch" "k8s.io/apimachinery/pkg/watch"
"k8s.io/cli-runtime/pkg/genericiooptions" "k8s.io/cli-runtime/pkg/genericiooptions"
"k8s.io/cli-runtime/pkg/resource" "k8s.io/cli-runtime/pkg/resource"
@ -1497,7 +1497,7 @@ func TestGetMultipleTypeObjectsAsList(t *testing.T) {
} }
` `
if e, a := expected, buf.String(); e != a { if e, a := expected, buf.String(); e != a {
t.Errorf("did not match: %v", diff.StringDiff(e, a)) t.Errorf("did not match:\n%v", cmp.Diff(e, a))
} }
} }

View File

@ -20,7 +20,7 @@ import (
"bytes" "bytes"
"testing" "testing"
"k8s.io/apimachinery/pkg/util/diff" "github.com/google/go-cmp/cmp"
) )
func TestFormatterWrite(t *testing.T) { func TestFormatterWrite(t *testing.T) {
@ -113,7 +113,7 @@ fringilla velit.
` `
if buf.String() != want { if buf.String() != want {
t.Errorf("Diff:\n%s", diff.StringDiff(buf.String(), want)) t.Errorf("Diff:\n%s", cmp.Diff(buf.String(), want))
} }
} }

View File

@ -24,9 +24,9 @@ import (
"strings" "strings"
"testing" "testing"
"github.com/google/go-cmp/cmp"
corev1 "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/kubernetes/scheme"
"k8s.io/pod-security-admission/api" "k8s.io/pod-security-admission/api"
@ -167,7 +167,7 @@ func testFixtureFile(t *testing.T, dir, name string, pod *corev1.Pod) string {
t.Logf("Could not update data in %s: %v", filename, err) t.Logf("Could not update data in %s: %v", filename, err)
} }
} else { } else {
t.Logf("Diff between generated data and fixture data in %s:\n-------------\n%s", filename, diff.StringDiff(string(yamlData), string(expectedYAML))) t.Logf("Diff between generated data and fixture data in %s:\n-------------\n%s", filename, cmp.Diff(string(yamlData), string(expectedYAML)))
t.Logf("If the change is expected, re-run with %s=true to update the fixtures", updateEnvVar) t.Logf("If the change is expected, re-run with %s=true to update the fixtures", updateEnvVar)
} }
} }