diff --git a/deep.go b/deep.go index dbc89c0..3e6636c 100644 --- a/deep.go +++ b/deep.go @@ -27,7 +27,9 @@ var ( LogErrors = false // CompareUnexportedFields causes unexported struct fields, like s in - // T{s int}, to be compared when true. + // T{s int}, to be compared when true. This does not work for comparing + // error or Time types on unexported fields because methods on unexported + // fields cannot be called. CompareUnexportedFields = false // NilSlicesAreEmpty causes a nil slice to be equal to an empty slice. @@ -137,18 +139,23 @@ func (c *cmp) equals(a, b reflect.Value, level int) { bElem := bKind == reflect.Ptr || bKind == reflect.Interface // If both types implement the error interface, compare the error strings. - // This must be done before dereferencing because the interface is on a - // pointer receiver. Re https://github.com/go-test/deep/issues/31, a/b might - // be primitive kinds; see TestErrorPrimitiveKind. - if aType.Implements(errorType) && bType.Implements(errorType) { - if (!aElem || !a.IsNil()) && (!bElem || !b.IsNil()) { - aString := a.MethodByName("Error").Call(nil)[0].String() - bString := b.MethodByName("Error").Call(nil)[0].String() - if aString != bString { - c.saveDiff(aString, bString) - return - } + // This must be done before dereferencing because errors.New() returns a + // pointer to a struct that implements the interface: + // func (e *errorString) Error() string { + // And we check CanInterface as a hack to make sure the underlying method + // is callable because https://github.com/golang/go/issues/32438 + // Issues: + // https://github.com/go-test/deep/issues/31 + // https://github.com/go-test/deep/issues/45 + if (aType.Implements(errorType) && bType.Implements(errorType)) && + ((!aElem || !a.IsNil()) && (!bElem || !b.IsNil())) && + (a.CanInterface() && b.CanInterface()) { + aString := a.MethodByName("Error").Call(nil)[0].String() + bString := b.MethodByName("Error").Call(nil)[0].String() + if aString != bString { + c.saveDiff(aString, bString) } + return } // Dereference pointers and interface{} diff --git a/deep_test.go b/deep_test.go index 2a03392..98fba1f 100644 --- a/deep_test.go +++ b/deep_test.go @@ -1304,21 +1304,7 @@ func TestError(t *testing.T) { func TestErrorWithOtherFields(t *testing.T) { a := errors.New("it broke") - b := errors.New("it broke") - - diff := deep.Equal(a, b) - if len(diff) != 0 { - t.Fatalf("expected zero diffs, got %d: %s", len(diff), diff) - } - - b = errors.New("it fell apart") - diff = deep.Equal(a, b) - if len(diff) != 1 { - t.Fatalf("expected 1 diff, got %d: %s", len(diff), diff) - } - if diff[0] != "it broke != it fell apart" { - t.Errorf("got '%s', expected 'it broke != it fell apart'", diff[0]) - } + b := errors.New("it fell apart") // Both errors set type tWithError struct { @@ -1333,7 +1319,7 @@ func TestErrorWithOtherFields(t *testing.T) { Error: b, Other: "ok", } - diff = deep.Equal(t1, t2) + diff := deep.Equal(t1, t2) if len(diff) != 1 { t.Fatalf("expected 1 diff, got %d: %s", len(diff), diff) } @@ -1409,6 +1395,25 @@ func TestErrorPrimitiveKind(t *testing.T) { if len(diff) != 0 { t.Fatalf("expected zero diffs, got %d: %s", len(diff), diff) } + + err2 = "def" + diff = deep.Equal(err1, err2) + if len(diff) != 1 { + t.Fatalf("expected 1 diff, got %d: %s", len(diff), diff) + } +} + +func TestErrorUnexported(t *testing.T) { + // https://github.com/go-test/deep/issues/45 + type foo struct { + bar error + } + defaultCompareUnexportedFields := deep.CompareUnexportedFields + deep.CompareUnexportedFields = true + defer func() { deep.CompareUnexportedFields = defaultCompareUnexportedFields }() + e1 := foo{bar: fmt.Errorf("error")} + e2 := foo{bar: fmt.Errorf("error")} + deep.Equal(e1, e2) } func TestNil(t *testing.T) {