search

stretchr / testify

A toolkit with common assertions and mocks that plays nicely with the standard library
MIT License
22.5k stars 1.56k forks source link

Panic in assert/require.Equal when calling into go-spew #480

Open akalin opened 6 years ago

akalin commented 6 years ago

Minimal test case:

package testifyrepro

import (
    "testing"

    "github.com/stretchr/testify/assert"
)

func TestReflect(t *testing.T) {
    type s struct {
        f map[[1]byte]int
    }

    v1 := s{
        f: map[[1]byte]int{
            [1]byte{0x1}: 0,
            [1]byte{0x2}: 0,
        },
    }

    assert.Equal(t, v1, s{})
}

running go test . produces:

--- FAIL: TestReflect (0.00s)
panic: reflect.Value.Interface: cannot return value obtained from unexported field or method [recovered]
    panic: reflect.Value.Interface: cannot return value obtained from unexported field or method

goroutine 5 [running]:
testing.tRunner.func1(0xc420075040)
    /Users/akalin/homebrew/Cellar/go/1.8.3/libexec/src/testing/testing.go:622 +0x29d
panic(0x126ebe0, 0xc420011e80)
    /Users/akalin/homebrew/Cellar/go/1.8.3/libexec/src/runtime/panic.go:489 +0x2cf
reflect.valueInterface(0x126ede0, 0xc420011e69, 0xa8, 0xb9203dc317d86601, 0x11, 0xc4200b20d8)
    /Users/akalin/homebrew/Cellar/go/1.8.3/libexec/src/reflect/value.go:936 +0x1a5
reflect.Value.Interface(0x126ede0, 0xc420011e69, 0xa8, 0x0, 0x126ede0)
    /Users/akalin/homebrew/Cellar/go/1.8.3/libexec/src/reflect/value.go:925 +0x44
github.com/stretchr/testify/vendor/github.com/davecgh/go-spew/spew.valueSortLess(0x1272820, 0xc420011e69, 0xb1, 0x1272820, 0xc420011e68, 0xb1, 0xc420041838)
    /Users/akalin/go/src/github.com/stretchr/testify/vendor/github.com/davecgh/go-spew/spew/common.go:315 +0x16b
github.com/stretchr/testify/vendor/github.com/davecgh/go-spew/spew.(*valuesSorter).Less(0xc420017580, 0x1, 0x0, 0xc420011e69)
    /Users/akalin/go/src/github.com/stretchr/testify/vendor/github.com/davecgh/go-spew/spew/common.go:328 +0xfa
sort.insertionSort(0x13f6880, 0xc420017580, 0x0, 0x2)
    /Users/akalin/homebrew/Cellar/go/1.8.3/libexec/src/sort/sort.go:29 +0x71
sort.quickSort(0x13f6880, 0xc420017580, 0x0, 0x2, 0x4)
    /Users/akalin/homebrew/Cellar/go/1.8.3/libexec/src/sort/sort.go:211 +0x1eb
sort.Sort(0x13f6880, 0xc420017580)
    /Users/akalin/homebrew/Cellar/go/1.8.3/libexec/src/sort/sort.go:220 +0x79
github.com/stretchr/testify/vendor/github.com/davecgh/go-spew/spew.sortValues(0xc42001b680, 0x2, 0x2, 0x141a580)
    /Users/akalin/go/src/github.com/stretchr/testify/vendor/github.com/davecgh/go-spew/spew/common.go:340 +0x70
github.com/stretchr/testify/vendor/github.com/davecgh/go-spew/spew.(*dumpState).dump(0xc420053c10, 0x1282000, 0xc42001b590, 0x35)
    /Users/akalin/go/src/github.com/stretchr/testify/vendor/github.com/davecgh/go-spew/spew/dump.go:388 +0xf65
github.com/stretchr/testify/vendor/github.com/davecgh/go-spew/spew.(*dumpState).dump(0xc420053c10, 0x128d5e0, 0xc42001b590, 0x19)
    /Users/akalin/go/src/github.com/stretchr/testify/vendor/github.com/davecgh/go-spew/spew/dump.go:421 +0x4bf
github.com/stretchr/testify/vendor/github.com/davecgh/go-spew/spew.fdump(0x141a580, 0x13f2900, 0xc4200becb0, 0xc420053da0, 0x1, 0x1)
    /Users/akalin/go/src/github.com/stretchr/testify/vendor/github.com/davecgh/go-spew/spew/dump.go:465 +0x142
github.com/stretchr/testify/vendor/github.com/davecgh/go-spew/spew.(*ConfigState).Sdump(0x141a580, 0xc420041da0, 0x1, 0x1, 0x1, 0x0)
    /Users/akalin/go/src/github.com/stretchr/testify/vendor/github.com/davecgh/go-spew/spew/config.go:281 +0x78
github.com/stretchr/testify/assert.diff(0x128d5e0, 0xc42001b590, 0x128d5e0, 0x0, 0x0, 0x0)
    /Users/akalin/go/src/github.com/stretchr/testify/assert/assertions.go:1173 +0x1c2
github.com/stretchr/testify/assert.Equal(0x13f3440, 0xc420075040, 0x128d5e0, 0xc42001b590, 0x128d5e0, 0x0, 0x0, 0x0, 0x0, 0x1071912)
    /Users/akalin/go/src/github.com/stretchr/testify/assert/assertions.go:313 +0x230
github.com/akalin/testifyrepro.TestReflect(0xc420075040)
    /Users/akalin/go/src/github.com/akalin/testifyrepro/repro_test.go:21 +0x11e
testing.tRunner(0xc420075040, 0x12e1fa0)
    /Users/akalin/homebrew/Cellar/go/1.8.3/libexec/src/testing/testing.go:657 +0x96
created by testing.(*T).Run
    /Users/akalin/homebrew/Cellar/go/1.8.3/libexec/src/testing/testing.go:697 +0x2ca
FAIL    github.com/akalin/testifyrepro  0.013s
chemidy commented 6 years ago

Hello @akalin .

This is not a testify bug.

In go lower case fields (labels in general) are not visible outside the defining package, reflect is outside.

So you have to upper your f field to "F"

You can read this for more informations : https://blog.golang.org/laws-of-reflection

akalin commented 6 years ago

go-spew, which testify uses, explicitly handles unexported fields; see https://github.com/davecgh/go-spew. So you're probably right in that the bug doesn't lie with testify directly. Perhaps it needs to revendor go-spew to a new version, or perhaps the bug is still in go-spew's tip.

In any case, testify panicking definitely seems like a bug to me, especially if assert is used instead of require.

zachmu commented 5 years ago

I'm seeing the same behavior. This is a bug in testify, whether it's caused by testify code or indirectly by the dependency on spew. I've filed an issue with spew, since I can reproduce it independent of testify's usage:

https://github.com/davecgh/go-spew/issues/108

qdm12 commented 2 years ago

Got the same problem here, this is definitely a spew issue (or bad spew version dependency here) only happening when the structs differ.

autarch commented 1 year ago

I note that spew has not seen a commit to its default branch for 4 years. It seems to be effectively unmaintained. I'm not sure what the best fix is here. It might make sense to borg the code into testify or to fork it and maintain that fork.