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

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

Open
akalin opened this issue Jul 19, 2017 · 5 comments
Open

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

akalin opened this issue Jul 19, 2017 · 5 comments

Comments

@akalin
Copy link

akalin commented Jul 19, 2017

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
Copy link

chemidy commented Jul 23, 2017

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
Copy link
Author

akalin commented Jul 24, 2017

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
Copy link

zachmu commented Apr 2, 2019

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:

davecgh/go-spew#108

@qdm12
Copy link

qdm12 commented Feb 21, 2022

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

@autarch
Copy link

autarch commented Jul 27, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants