Skip to content

Commit ea098bf

Browse files
committed
fix: panic in spew when dealing with unexported fields
This PR adapts stretchr#1828 It fixes an edge case the spew lib when diffing a map with keys of type array. Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
1 parent a5a03bc commit ea098bf

File tree

3 files changed

+164
-0
lines changed

3 files changed

+164
-0
lines changed

internal/assertions/equal_test.go

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,109 @@ func TestEqualBytes(t *testing.T) {
344344
}
345345
}
346346

347+
func TestEqualValuePanics(t *testing.T) {
348+
t.Parallel()
349+
350+
for tt := range panicCases() {
351+
t.Run(tt.name, func(t *testing.T) {
352+
t.Parallel()
353+
354+
mock := new(mockT)
355+
NotPanics(t, func() {
356+
Equal(mock, tt.value1, tt.value2)
357+
}, "should not panic")
358+
359+
if !tt.expectEqual {
360+
True(t, mock.Failed(), "should have failed")
361+
Contains(t, mock.errorString(), "Not equal:", "error message should mention inequality")
362+
363+
return
364+
}
365+
366+
False(t, mock.Failed(), "should have been successful")
367+
Empty(t, mock.errorString())
368+
})
369+
}
370+
}
371+
372+
type panicCase struct {
373+
name string
374+
value1 any
375+
value2 any
376+
expectEqual bool
377+
}
378+
379+
func panicCases() iter.Seq[panicCase] {
380+
type structWithUnexportedMapWithArrayKey struct {
381+
m any
382+
}
383+
384+
return slices.Values([]panicCase{
385+
{
386+
// from issue https://github.com/stretchr/testify/pull/1816
387+
name: "panic behavior on struct with array key and unexported field (some keys vs none)",
388+
value1: structWithUnexportedMapWithArrayKey{
389+
map[[1]byte]*struct{}{
390+
{1}: nil,
391+
{2}: nil,
392+
},
393+
},
394+
value2: structWithUnexportedMapWithArrayKey{
395+
map[[1]byte]*struct{}{},
396+
},
397+
expectEqual: false,
398+
},
399+
{
400+
name: "panic behavior on struct with array key and unexported field (same keys)",
401+
value1: structWithUnexportedMapWithArrayKey{
402+
map[[1]byte]*struct{}{
403+
{1}: nil,
404+
{2}: nil,
405+
},
406+
},
407+
value2: structWithUnexportedMapWithArrayKey{
408+
map[[1]byte]*struct{}{
409+
{2}: nil,
410+
{1}: nil,
411+
},
412+
},
413+
expectEqual: true,
414+
},
415+
{
416+
name: "panic behavior on struct with array key and unexported field (non-nil values)",
417+
value1: structWithUnexportedMapWithArrayKey{
418+
map[[1]byte]*struct{}{
419+
{1}: {},
420+
{2}: nil,
421+
},
422+
},
423+
value2: structWithUnexportedMapWithArrayKey{
424+
map[[1]byte]*struct{}{
425+
{1}: {},
426+
{2}: nil,
427+
},
428+
},
429+
expectEqual: true,
430+
},
431+
{
432+
name: "panic behavior on struct with array key and unexported field (different, non-nil values)",
433+
value1: structWithUnexportedMapWithArrayKey{
434+
map[[1]byte]*struct{}{
435+
{1}: {},
436+
{2}: nil,
437+
},
438+
},
439+
value2: structWithUnexportedMapWithArrayKey{
440+
map[[1]byte]*struct{}{
441+
{1}: nil,
442+
{2}: {},
443+
},
444+
},
445+
expectEqual: false,
446+
},
447+
})
448+
}
449+
347450
type equalCase struct {
348451
expected any
349452
actual any

internal/spew/common.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,12 @@ func valueSortLess(a, b reflect.Value) bool {
314314
for i := range l {
315315
av := a.Index(i)
316316
bv := b.Index(i)
317+
318+
if !av.CanInterface() || !bv.CanInterface() {
319+
// Unexported fields would panic on Interface() call.
320+
continue
321+
}
322+
317323
if av.Interface() == bv.Interface() {
318324
continue
319325
}

internal/spew/common_impl_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package spew
2+
3+
import (
4+
"iter"
5+
"reflect"
6+
"slices"
7+
"testing"
8+
)
9+
10+
func TestPanicUnexportedFields(t *testing.T) {
11+
t.Parallel()
12+
13+
for tt := range panicCases() {
14+
t.Run(tt.name, func(t *testing.T) {
15+
v1 := reflect.ValueOf(tt.value1)
16+
v2 := reflect.ValueOf(tt.value2)
17+
isLess := valueSortLess(v1, v2)
18+
19+
if isLess {
20+
t.Error("expected an ordered set")
21+
}
22+
})
23+
}
24+
}
25+
26+
type panicCase struct {
27+
name string
28+
value1 any
29+
value2 any
30+
}
31+
32+
func panicCases() iter.Seq[panicCase] {
33+
type structWithUnexportedMapWithArrayKey struct {
34+
m any
35+
}
36+
37+
return slices.Values([]panicCase{
38+
{
39+
// from issue https://github.com/stretchr/testify/pull/1816
40+
name: "panic behavior on struct with array key and unexported field",
41+
value1: structWithUnexportedMapWithArrayKey{
42+
map[[1]byte]*struct{}{
43+
{1}: nil,
44+
{2}: nil,
45+
},
46+
},
47+
value2: structWithUnexportedMapWithArrayKey{
48+
map[[1]byte]*struct{}{
49+
{2}: nil,
50+
{1}: nil,
51+
},
52+
},
53+
},
54+
})
55+
}

0 commit comments

Comments
 (0)