Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove go-cmp dependency #501

Merged
merged 2 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changelog/5faed85326334a93a9b0a96284389287.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id": "5faed853-2633-4a93-a9b0-a96284389287",
"type": "bugfix",
"description": "Remove runtime dependency on go-cmp.",
"modules": [
"."
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -445,29 +445,7 @@ protected void writeAssertComplexEqual(
GoWriter writer, String expect, String actual, String[] ignoreTypes
) {
writer.addUseImports(SmithyGoDependency.SMITHY_TESTING);
writer.addUseImports(SmithyGoDependency.GO_CMP);
writer.addUseImports(SmithyGoDependency.GO_CMP_OPTIONS);
writer.addUseImports(SmithyGoDependency.SMITHY_DOCUMENT);
writer.addUseImports(SmithyGoDependency.MATH);

writer.openBlock("opts := cmp.Options{", "}", () -> {
writer.openBlock("cmpopts.IgnoreUnexported(", "),", () -> {
for (String ignoreType : ignoreTypes) {
writer.write("$L,", ignoreType);
}
});
writer.write("cmp.FilterValues(func(x, y float64) bool {\n"
+ "\treturn math.IsNaN(x) && math.IsNaN(y)\n"
+ "},"
+ "cmp.Comparer(func(_, _ interface{}) bool { return true })),");
writer.write("cmp.FilterValues(func(x, y float32) bool {\n"
+ "\treturn math.IsNaN(float64(x)) && math.IsNaN(float64(y))\n"
+ "},"
+ "cmp.Comparer(func(_, _ interface{}) bool { return true })),");
writer.write("cmpopts.IgnoreTypes(smithydocument.NoSerde{}),");
});

writer.openBlock("if err := smithytesting.CompareValues($L, $L, opts...); err != nil {", "}",
writer.openBlock("if err := smithytesting.CompareValues($L, $L); err != nil {", "}",
expect, actual, () -> {
writer.write("t.Errorf(\"expect $L value match:\\n%v\", err)", expect);
});
Expand Down
2 changes: 0 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
module github.com/aws/smithy-go

go 1.20

require github.com/google/go-cmp v0.5.8
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,2 +0,0 @@
github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg=
github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
1 change: 0 additions & 1 deletion modman.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
[dependencies]
"github.com/google/go-cmp" = "v0.5.8"
"github.com/jmespath/go-jmespath" = "v0.4.0"

[modules]
Expand Down
17 changes: 9 additions & 8 deletions testing/document.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@ import (
"bytes"
"encoding/json"
"fmt"
"reflect"
"sort"
"strings"

"github.com/aws/smithy-go/testing/xml"

"github.com/google/go-cmp/cmp"
)

// JSONEqual compares two JSON documents and identifies if the documents contain
Expand All @@ -25,8 +24,8 @@ func JSONEqual(expectBytes, actualBytes []byte) error {
return fmt.Errorf("failed to unmarshal actual bytes, %v", err)
}

if diff := cmp.Diff(expect, actual); len(diff) != 0 {
return fmt.Errorf("JSON mismatch (-expect +actual):\n%s", diff)
if !reflect.DeepEqual(expect, actual) {
return fmt.Errorf("JSON mismatch: %v != %v", expect, actual)
}

return nil
Expand Down Expand Up @@ -60,8 +59,8 @@ func XMLEqual(expectBytes, actualBytes []byte) error {
return err
}

if diff := cmp.Diff(actualString, expectedString); len(diff) != 0 {
return fmt.Errorf("XML mismatch (-expect +actual):\n%s", diff)
if expectedString != actualString {
return fmt.Errorf("XML mismatch: %v != %v", expectedString, actualString)
}

return nil
Expand All @@ -85,8 +84,10 @@ func AssertXMLEqual(t T, expect, actual []byte) bool {
// contain the same values. Returns an error if the two documents are not
// equal.
func URLFormEqual(expectBytes, actualBytes []byte) error {
if diff := cmp.Diff(parseFormBody(expectBytes), parseFormBody(actualBytes)); len(diff) != 0 {
return fmt.Errorf("Query mismatch (-expect +actual):\n%s", diff)
expect := parseFormBody(expectBytes)
actual := parseFormBody(actualBytes)
if !reflect.DeepEqual(expect, actual) {
return fmt.Errorf("Query mismatch: %v != %v", expect, actual)
}
return nil
}
Expand Down
223 changes: 163 additions & 60 deletions testing/struct.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,41 +4,148 @@ import (
"bytes"
"encoding/hex"
"fmt"
"github.com/aws/smithy-go/document"
"io"
"io/ioutil"
"net/http"
"math"
"reflect"

"github.com/google/go-cmp/cmp"
"github.com/aws/smithy-go/document"
"github.com/aws/smithy-go/middleware"
)

// CompareValues compares two values to determine if they are equal.
func CompareValues(expect, actual interface{}, opts ...cmp.Option) error {
opts = append(make([]cmp.Option, 0, len(opts)+1), opts...)

var skippedReaders filterSkipDifferentIoReader

opts = append(opts,
cmp.Transformer("http.NoBody", transformHTTPNoBodyToNil),
cmp.FilterValues(skippedReaders.filter, cmp.Ignore()),
cmp.Comparer(compareDocumentTypes),
)
// CompareValues compares two values to determine if they are equal,
// specialized for comparison of SDK operation output types.
//
// CompareValues expects the two values to be of the same underlying type.
// Doing otherwise will result in undefined behavior.
//
// The third variadic argument is vestigial from a previous implementation that
// depended on go-cmp. Values passed therein have no effect.
func CompareValues(expect, actual interface{}, _ ...interface{}) error {
return deepEqual(reflect.ValueOf(expect), reflect.ValueOf(actual), "<root>")
}

if diff := cmp.Diff(expect, actual, opts...); len(diff) != 0 {
return fmt.Errorf("values do not match\n%s", diff)
func deepEqual(expect, actual reflect.Value, path string) error {
if et, at := expect.Kind(), actual.Kind(); et != at {
return fmt.Errorf("%s: kind %s != %s", path, et, at)
}

var errs []error
for _, s := range skippedReaders {
if err := CompareReaders(s.A, s.B); err != nil {
errs = append(errs, err)
// there are a handful of short-circuit cases here within the context of
// operation responses:
// - ResultMetadata (we don't care)
// - document.Interface (check for marshaled []byte equality)
// - io.Reader (check for Read() []byte equality)
ei, ai := expect.Interface(), actual.Interface()
if _, _, ok := asMetadatas(ei, ai); ok {
return nil
}
if e, a, ok := asDocuments(ei, ai); ok {
if !compareDocumentTypes(e, a) {
return fmt.Errorf("%s: document values unequal", path)
}
return nil
}
if len(errs) != 0 {
return fmt.Errorf("io.Readers have different values\n%v", errs)
if e, a, ok := asReaders(ei, ai); ok {
if err := CompareReaders(e, a); err != nil {
return fmt.Errorf("%s: %w", path, err)
}
return nil
}

return nil
switch expect.Kind() {
case reflect.Pointer:
if expect.Type() != actual.Type() {
return fmt.Errorf("%s: type mismatch", path)
}

expect = deref(expect)
actual = deref(actual)
ek, ak := expect.Kind(), actual.Kind()
if ek == reflect.Invalid || ak == reflect.Invalid {
// one was a nil pointer, so they both must be nil
if ek == ak {
return nil
}
return fmt.Errorf("%s: %s != %s", path, fmtNil(ek), fmtNil(ak))
}
if err := deepEqual(expect, actual, path); err != nil {
return err
}
return nil
case reflect.Slice:
if expect.Len() != actual.Len() {
return fmt.Errorf("%s: slice length unequal", path)
}
for i := 0; i < expect.Len(); i++ {
ipath := fmt.Sprintf("%s[%d]", path, i)
if err := deepEqual(expect.Index(i), actual.Index(i), ipath); err != nil {
return err
}
}
return nil
case reflect.Map:
if expect.Len() != actual.Len() {
return fmt.Errorf("%s: map length unequal", path)
}
for _, k := range expect.MapKeys() {
kpath := fmt.Sprintf("%s[%q]", path, k.String())
if err := deepEqual(expect.MapIndex(k), actual.MapIndex(k), kpath); err != nil {
return err
}
}
return nil
case reflect.Struct:
for i := 0; i < expect.NumField(); i++ {
if !expect.Field(i).CanInterface() {
continue // unexported
}
fpath := fmt.Sprintf("%s.%s", path, expect.Type().Field(i).Name)
if err := deepEqual(expect.Field(i), actual.Field(i), fpath); err != nil {
return err
}
}
return nil
case reflect.Float32, reflect.Float64:
// NaN != NaN by definition but we just care about bitwise equality
ef, af := math.Float64bits(expect.Float()), math.Float64bits(actual.Float())
if ef != af {
return fmt.Errorf("%s: float 0x%x != 0x%x", path, ef, af)
}
return nil
default:
// everything else is just scalars and can be delegated
if !reflect.DeepEqual(ei, ai) {
return fmt.Errorf("%s: %v != %v", path, ei, ai)
}
return nil
}
}

func asMetadatas(i, j interface{}) (ii, jj middleware.Metadata, ok bool) {
ii, iok := i.(middleware.Metadata)
jj, jok := j.(middleware.Metadata)
return ii, jj, iok || jok
}

func asDocuments(i, j interface{}) (ii, jj documentInterface, ok bool) {
ii, iok := i.(documentInterface)
jj, jok := j.(documentInterface)
return ii, jj, iok || jok
}

func asReaders(i, j interface{}) (ii, jj io.Reader, ok bool) {
ii, iok := i.(io.Reader)
jj, jok := j.(io.Reader)
return ii, jj, iok || jok
}

func deref(v reflect.Value) reflect.Value {
switch v.Kind() {
case reflect.Interface, reflect.Ptr:
for v.Kind() == reflect.Interface || v.Kind() == reflect.Ptr {
v = v.Elem()
}
}
return v
}

type documentInterface interface {
Expand All @@ -47,6 +154,13 @@ type documentInterface interface {
}

func compareDocumentTypes(x documentInterface, y documentInterface) bool {
if x == nil {
x = nopMarshaler{}
}
if y == nil {
y = nopMarshaler{}
}

xBytes, err := x.MarshalSmithyDocument()
if err != nil {
panic(fmt.Sprintf("MarshalSmithyDocument error: %v", err))
Expand All @@ -58,49 +172,22 @@ func compareDocumentTypes(x documentInterface, y documentInterface) bool {
return JSONEqual(xBytes, yBytes) == nil
}

func transformHTTPNoBodyToNil(v io.Reader) io.Reader {
if v == http.NoBody {
return nil
}
return v
}

type filterSkipDifferentIoReader []skippedReaders

func (f *filterSkipDifferentIoReader) filter(a, b io.Reader) bool {
if a == nil || b == nil {
return false
}
//at, bt := reflect.TypeOf(a), reflect.TypeOf(b)
//for at.Kind() == reflect.Ptr {
// at = at.Elem()
//}
//for bt.Kind() == reflect.Ptr {
// bt = bt.Elem()
//}

//// The underlying reader types are the same they can be compared directly.
//if at == bt {
// return false
//}

*f = append(*f, skippedReaders{A: a, B: b})
return true
}

type skippedReaders struct {
A, B io.Reader
}

// CompareReaders two io.Reader values together to determine if they are equal.
// Will read the contents of the readers until they are empty.
func CompareReaders(expect, actual io.Reader) error {
e, err := ioutil.ReadAll(expect)
if expect == nil {
expect = nopReader{}
}
if actual == nil {
actual = nopReader{}
}

e, err := io.ReadAll(expect)
if err != nil {
return fmt.Errorf("failed to read expect body, %w", err)
}

a, err := ioutil.ReadAll(actual)
a, err := io.ReadAll(actual)
if err != nil {
return fmt.Errorf("failed to read actual body, %w", err)
}
Expand All @@ -112,3 +199,19 @@ func CompareReaders(expect, actual io.Reader) error {

return nil
}

func fmtNil(k reflect.Kind) string {
if k == reflect.Invalid {
return "nil"
}
return "non-nil"
}

type nopReader struct{}

func (nopReader) Read(p []byte) (int, error) { return 0, io.EOF }

type nopMarshaler struct{}

func (nopMarshaler) MarshalSmithyDocument() ([]byte, error) { return nil, nil }
func (nopMarshaler) UnmarshalSmithyDocument(v interface{}) error { return nil }
Loading
Loading