stretchr / testify

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

assert: support byte slice in Contains #1526

Open nickajacks1 opened 5 months ago

nickajacks1 commented 5 months ago

Summary

Update assert.Contains and assert.NotContains to check if a byte slice contains another byte slice.

Changes

If both the list and element arguments are byte slices, use bytes.Contains to check if element is a subslice. This mirrors the way strings are treated by assert.Contains.

Motivation

The change makes Contains more consistent and predictable. One could argue that developers could easily cast byte slices to strings, but the fact that that Contains does not treat strings and byte slices the same way breaks the principle of least astonishment (and probably would warrant a new rule in testifylint).

foo := []byte(`Text`)
b, err := os.ReadFile("file.txt")
require.NoError(t, err)
require.Contains(t, b, foo)
brackendawson commented 4 months ago

Would this change be more useful to more people if when contains is a slice or array of the same type as s, then the assertion checks that the run of contains appears at least once in s? Rather than this only working with []byte, and not even array of byte.

eg:

Contains(t, []int{1,2,3,4,5,6,7,8,9}, []int{4,5,6})                        // -> true
Contains(t, []int{1,2,3,4,5,6,7,8,9}, []int{3,5,7})                        // -> false
Contains(t, []byte("My hovercraft is full of eels"), []byte("hovercraft")) // -> true
Contains(t, [...]byte("I am an array"), [...]byte("arr"))                  // -> true
Contains(t, []any{"hi", []byte{}, false, 9}, []byte{})                     // -> true
nickajacks1 commented 4 months ago

Would this change be more useful to more people if when contains is a slice or array of the same type as s, then the assertion checks that the run of contains appears at least once in s? Rather than this only working with []byte, and not even array of byte.

eg:

Contains(t, []int{1,2,3,4,5,6,7,8,9}, []int{4,5,6})                        // -> true
Contains(t, []int{1,2,3,4,5,6,7,8,9}, []int{3,5,7})                        // -> false
Contains(t, []byte("My hovercraft is full of eels"), []byte("hovercraft")) // -> true
Contains(t, [...]byte("I am an array"), [...]byte("arr"))                  // -> true
Contains(t, []any{"hi", []byte{}, false, 9}, []byte{})                     // -> true

Yeah actually I think I was looking for that functionality before and was disappointed to find that it didn't exist. This is a good idea, I'll look at reworking this to support "subslice" checking.

dolmen commented 4 months ago

@nickajacks1 Let's keep this PR focused on []byte support to allow us to merge this quickly, and submit other changes later.