go-test / deep

Golang deep variable equality test that returns human-readable differences
MIT License
751 stars 54 forks source link

fixes panic and false positive #7

Closed gmarik closed 6 years ago

gmarik commented 7 years ago

Problem

  1. deep.Equal panics on map[string]interface{} diff having different value types. See TestInterface2
  2. deep.Equal skips pointer diff. See TestInterface3
--- FAIL: TestInterface2 (0.00s)
        deep_test.go:631: panic: reflect: call of reflect.Value.Int on float64 Value
--- FAIL: TestInterface3 (0.00s)
        deep_test.go:660: expected 1 diff, got zero

Solution

  1. do not assume types are equal by calling c.equal(v1, v2)
  2. compare value pointer points to by calling c.equal(v1, v2)

Notes

  1. proposed fix changes how deep.Equal reports differences by printing actual values: see 4cc30ce how it changes expectations
  2. the 4cc30ce is a "quick fix" to make things work, so please let me know how it could be improved…
  3. thank you!
coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling c8c612d65a573dca1e4562b45a55dd10a46eb164 on gmarik:master into f49763a6ea0a91026be26f8213bebee726b4185f on go-test:master.

daniel-nichter commented 6 years ago

Thanks for the test cases and fix! Sorry it's taken about 4 months to review and merge (super busy at work and such).

The failing test is a false-positive because it looks like go1.8 differs in how it prints some stuff compared to 1.7 and 1.6. I'll fix that later.

I'll merge because it's a good fix, but re note 1: I'll need to revert the change in output because printing the full value of a struct will be more noise than signal, imho, especially if the struct has many fields. Also, I'd argue that the values don't matter at this point because the diff is saying one var is nil, so whatever the values of the non-nil struct aren't a consideration yet because we can't compare values.