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

assert: support byte slice in Contains #1526

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

nickajacks1
Copy link

@nickajacks1 nickajacks1 commented Jan 27, 2024

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)

Copy link
Collaborator

@arjunmahishi arjunmahishi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably worth updating the examples

Here:

// Contains asserts that the specified string, list(array, slice...) or map contains the
// specified substring or element.
//
// assert.Contains(t, "Hello World", "World")
// assert.Contains(t, ["Hello", "World"], "World")
// assert.Contains(t, {"Hello": "World"}, "Hello")

And here:

// NotContains asserts that the specified string, list(array, slice...) or map does NOT contain the
// specified substring or element.
//
// assert.NotContains(t, "Hello World", "Earth")
// assert.NotContains(t, ["Hello", "World"], "Earth")
// assert.NotContains(t, {"Hello": "World"}, "Earth")

So, that the godoc reflects that []byte is now supported.

assert/assertions.go Outdated Show resolved Hide resolved
@nickajacks1 nickajacks1 force-pushed the contains-bytes branch 2 times, most recently from 4be271c to 5583f5d Compare February 18, 2024 02:29
@brackendawson brackendawson added this to the v1.9.0 milestone Feb 25, 2024
@brackendawson
Copy link
Collaborator

brackendawson commented Feb 25, 2024

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

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.

@brackendawson brackendawson modified the milestones: v1.9.0, v1.10 Feb 28, 2024
@dolmen dolmen added bug pkg-assert Change related to package testify/assert pkg-require Change related to package testify/require labels Mar 7, 2024
@dolmen
Copy link
Collaborator

dolmen commented Mar 7, 2024

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

assert/assertions.go Show resolved Hide resolved
assert/assertions.go Outdated Show resolved Hide resolved
assert/assertions.go Outdated Show resolved Hide resolved
@dolmen dolmen added enhancement and removed bug labels Mar 7, 2024
@nickajacks1 nickajacks1 requested a review from dolmen April 10, 2024 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement pkg-assert Change related to package testify/assert pkg-require Change related to package testify/require
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants