verify.Values should compare private fields like reflect.DeepEqual about goe HOT 17 CLOSED

Well you could just take the bloody minded (aka engineering) approach:

if got, want := stuff, test.stuff; !reflect.DeepEquals(got,want) {
      t.Fatalf("..Got %+v\n.Want %+v", got, want)
      verify.Values(t, "", got, want)
}

Then your test is 100% safe and you will probably get some info from the verify, if not just have to dig a bit deeper.

from goe.

So there are two issues here:

1. NaN, I think goderive is just going to go with whatever go does here when a.(float64) == b.(float64) is done on two NaNs. But I agree with pascal, this is not ideal behavior for tests.
2. Accessing private fields in different packages, like the standard library. An ideal verify values should be able to do this or know that it can't and have a fallback strategy to reflect.DeepEqual, but the conflict in logic with NaN makes it a little wonky. Also as someone who likes using unsafe, I still would not recommend maintaining unsafe code, especially since I find the unsafe usage in reflect.DeepEqual harder to read than most unsafe code. As for goderive I am still unsure. I can fallback to reflect.DeepEqual in the case of private fields and generate local functions in the case of public fields, but in some cases I would prefer a compile error. Unfortunately I can't see a way around the fallback strategy for the standard library and thus vendored in packages. But for local packages I would prefer a compile error. So still thinking, and I will cross that bridge when I get an issue report on goderive from an actual user, at least I have a backup plan.

from goe.

I have already added it as an issue :)
awalterschulze/goderive#15

from goe.

Deep equals uses unsafe and the functionality is not exported in the reflection package.
https://golang.org/src/reflect/deepequal.go#L132
https://golang.org/src/reflect/value.go#L922

Of course verify.Values could use reflect.DeepEqual for unexported fields yet that also introduces NaN != NaN.

The only way I see to do this right is a values equivalent from @awalterschulze his goredrive.

from goe.

This should be easy to do with goderive and much harder with reflect and unsafe.
The tradeoff is the extra generated code.
This tradeoff is great when you gain speed and readability of code, but I don't know if its always worth it in the case of tests.
I am on the fence here.

from goe.

from goe.

On second thought the Equal method could also be generated in the test code but that would limit the application to tests of structs that happen within the same package.

Oh, relying on generating the Equal method would prevent verifying structs that are outside of the package or which are vendored in...

from goe.

vendored in structs should not be exposed. Or can you give an example, maybe I am not understanding the problem.

from goe.

If I have to generate an Equal method then I can do this only for the structs within the same package iff I want to keep the equal method in the test code. Everything that is outside the package you're testing would have to have a public Equal method which is exported. This can work for your own code but becomes iffy for vendored in code.

As for vendored structs that shouldn't be exposed: How about http.Client and url.URL? How would I add an Equal method if I have a URL field in my struct, for example?

from goe.

Aha makes sense. Good point. I'll have to think about that.

from goe.

1. Use unsafe. Sounds like a ton of work and I don't feel like spending it.

2. Accept NaN != NaN and use reflect.DeepEquals where needed.

3. Keep this limitation.

from goe.

@sandymcp The point is not whether or not I can work around it. Of course, I can and it isn't much work.

I think the main argument is that the results from verify.Values and reflect.DeepEqual differ and that shouldn't be. Also, it isn't obvious that they do. The reason is an implementation detail no matter how complex it is.

Since you use verify.Values in tests as a more convenient drop-in replacement for reflect.DeepEqual they should produce the same results. Otherwise, you can not trust it.

from goe.

The common usecase is testing indeed. That verify.Values tests the content to be present rather than honoring what IEEE stated to be equal makes sense to me. How else are you going to verify a float to be NaN?
Anyway, it's not that much of a problem and it seems like the least amount of work.

from goe.

goderive can now derive equal for structs in other packages as well as private fields. So I think its now comparable to reflect.DeepEqual

func deriveEqualextra_PrivateFieldAndNoEqualMethod(this, that extra.PrivateFieldAndNoEqualMethod) bool {
	thisv := reflect.Indirect(reflect.ValueOf(&this))
	thatv := reflect.Indirect(reflect.ValueOf(&that))
	return *(*int64)(unsafe.Pointer(thisv.FieldByName("number").UnsafeAddr())) == *(*int64)(unsafe.Pointer(thatv.FieldByName("number").UnsafeAddr())) &&
		deriveEqualSliceOfint64(*(*[]int64)(unsafe.Pointer(thisv.FieldByName("numbers").UnsafeAddr())), *(*[]int64)(unsafe.Pointer(thatv.FieldByName("numbers").UnsafeAddr()))) &&
		((*(**int64)(unsafe.Pointer(thisv.FieldByName("ptr").UnsafeAddr())) == nil && *(**int64)(unsafe.Pointer(thatv.FieldByName("ptr").UnsafeAddr())) == nil) || (*(**int64)(unsafe.Pointer(thisv.FieldByName("ptr").UnsafeAddr())) != nil && *(**int64)(unsafe.Pointer(thatv.FieldByName("ptr").UnsafeAddr())) != nil && **(**int64)(unsafe.Pointer(thisv.FieldByName("ptr").UnsafeAddr())) == **(**int64)(unsafe.Pointer(thatv.FieldByName("ptr").UnsafeAddr())))) &&
		deriveEqualSliceOfPtrToint64(*(*[]*int64)(unsafe.Pointer(thisv.FieldByName("numberpts").UnsafeAddr())), *(*[]*int64)(unsafe.Pointer(thatv.FieldByName("numberpts").UnsafeAddr()))) &&
		deriveEqualPtrToextra_StructWithoutEqualMethod(*(**extra.StructWithoutEqualMethod)(unsafe.Pointer(thisv.FieldByName("strct").UnsafeAddr())), *(**extra.StructWithoutEqualMethod)(unsafe.Pointer(thatv.FieldByName("strct").UnsafeAddr())))
}

from goe.

Writing a modification of equal function to rather turn an error with a description of the diff should now be an easy modification of the equal generator, but I have other functions with a higher priority for now.

from goe.

Just found this newcomer: https://godoc.org/github.com/google/go-cmp/cmp

from goe.

Sjoerd told me about it. There was a talk at GopherCon.

from goe.