Skip to content

Commit dfdbd7f

Browse files
committed
Catch panic when using go-spew
go-spew can panic when trying to diff certain types of values, there are open issues about this on their GitHub repository. go-spew is unfortunately unmaintained, we cannot expect a fix any time soon. Also, because of go-spew's design, there are multiple causes for a panic, and fixing all of them would be a huge undertaking. We already return an empty diff when the types are not comparable, or when the values are not from types that can be easily diffed by go-spew. Let's hide the panic by recovering from it, and returning an empty diff instead. This is not ideal, but at least it prevents the entire program from crashing. The expected/actual values will still be printed, just without the diff.
1 parent 429ee0b commit dfdbd7f

File tree

2 files changed

+58
-11
lines changed

2 files changed

+58
-11
lines changed

assert/assertions.go

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1948,19 +1948,28 @@ func diff(expected interface{}, actual interface{}) string {
19481948

19491949
var e, a string
19501950

1951-
switch et {
1952-
case reflect.TypeOf(""):
1953-
e = reflect.ValueOf(expected).String()
1954-
a = reflect.ValueOf(actual).String()
1955-
case reflect.TypeOf(time.Time{}):
1956-
e = spewConfigStringerEnabled.Sdump(expected)
1957-
a = spewConfigStringerEnabled.Sdump(actual)
1958-
default:
1959-
e = spewConfig.Sdump(expected)
1960-
a = spewConfig.Sdump(actual)
1951+
// Use spew to create string representations of the objects
1952+
// We have to guard against panics in spew.
1953+
// See https://github.com/stretchr/testify/issues/480
1954+
panicked, _, _ := didPanic(func() {
1955+
switch et {
1956+
case reflect.TypeOf(""):
1957+
e = reflect.ValueOf(expected).String()
1958+
a = reflect.ValueOf(actual).String()
1959+
case reflect.TypeOf(time.Time{}):
1960+
e = spewConfigStringerEnabled.Sdump(expected)
1961+
a = spewConfigStringerEnabled.Sdump(actual)
1962+
default:
1963+
e = spewConfig.Sdump(expected)
1964+
a = spewConfig.Sdump(actual)
1965+
}
1966+
})
1967+
if panicked {
1968+
// silently ignore diff if we panic during spew, the library is no longer maintained
1969+
return ""
19611970
}
19621971

1963-
diff, _ := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{
1972+
diff, err := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{
19641973
A: difflib.SplitLines(e),
19651974
B: difflib.SplitLines(a),
19661975
FromFile: "Expected",
@@ -1970,6 +1979,11 @@ func diff(expected interface{}, actual interface{}) string {
19701979
Context: 1,
19711980
})
19721981

1982+
if err != nil {
1983+
// silently ignore diff if the external library fails to compute it
1984+
return ""
1985+
}
1986+
19731987
return "\n\nDiff:\n" + diff
19741988
}
19751989

assert/assertions_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -837,6 +837,39 @@ func TestEqualFormatting(t *testing.T) {
837837
}
838838
}
839839

840+
func TestEqualFormattingWithPanic(t *testing.T) {
841+
t.Parallel()
842+
843+
type structWithUnexportedMapWithArrayKey struct {
844+
m interface{}
845+
}
846+
847+
for _, c := range []struct {
848+
a interface{}
849+
b interface{}
850+
}{
851+
{
852+
// from the issue https://github.com/stretchr/testify/pull/1816
853+
a: structWithUnexportedMapWithArrayKey{
854+
map[[1]byte]*struct{}{
855+
{1}: nil,
856+
{2}: nil,
857+
},
858+
},
859+
b: structWithUnexportedMapWithArrayKey{},
860+
},
861+
} {
862+
863+
mockT := new(mockTestingT)
864+
NotPanics(t, func() {
865+
Equal(mockT, c.a, c.b)
866+
}, "should not panic")
867+
868+
True(t, mockT.Failed(), "should have failed")
869+
Contains(t, mockT.errorString(), "Not equal:", "error message should mention inequality")
870+
}
871+
}
872+
840873
func TestFormatUnequalValues(t *testing.T) {
841874
t.Parallel()
842875

0 commit comments

Comments
 (0)