-
Notifications
You must be signed in to change notification settings - Fork 49
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
Add call Empty(values ...any) #250
Comments
Thank you for your interest in contributing to this project! One of the primary design goals for this library is to keep the number of functions to a minimum. The only functions in the Another goal is to avoid having many ways to do something. As much as possible there should be only a single way of asserting some condition, to help make tests more consistent and easier to read. I believe this proposal is not aligned with these goals. As you point out, there's already a really easy way to assert that these types have a zero or nil value. Another way to assert an empty slice would be: assert.Equal(t, emptySlice, []string{})
// or
assert.Equal(t, len(emptySlice), 0)
// or, with more detail
assert.Equal(t, len(emptySlice), 0, "got %#v", emptySlice) These should give you a similar error message, without the need for another function. I think it would be valuable to add this example to the docs, and to add a Unfortunately I don't think it's a good idea to add an |
@dnephin Thanks for the speedy response to my requested issue and fix. I understand having as few calls as possible, but this would improve test readability. In the cases of an empty pointer, taking and the length of a slice, or expanding on structs; assert.Empty would be a singular call with the goal of filling all nil, empty, and no item cases. For example: // will panic contrary to the example i gave originally
var emptyStringSlice []string
assert.Equal(t, emptyStringSlice, []string{})
// fails with +++ []string(nil){}
var emptyStringSlice []string
assert.DeepEqual(t, emptyStringSlice, []string{})
assert.DeepEqual(t, emptyStringSlice, nil)
// Or in the case of a pointer to a []string, assertion fails. +++ nil any( (*[]string)(nil)
var emptyStringSlice *[]string
assert.Equal(t, emptyStringSlice, nil)
// To check the above is nil,
var emptyStringSlice *[]string
var anotherEmptySlice *[]string
assert.Equal(t, emptyStringSlice, anotherEmptySlice) I agree that fewer calls makes assert simple to digest, but I also think that Empty would be a valuable addition to the package. If you are unswayed I would be happy to add as an example, and can use the Empty in my own tests as an example. Where would I go about submitting to the docs? |
I guess the reason I've never run into these sub-optimal failure messages myself is because I always write the assertion as assert.Equal(t, len(actual), 0, "got %#v", actual) I believe it's generally a good practice to write Go code that doesn't care about the different between an empty slice and a nil slice, so the assertions I write follow the same rules. I guess there may be some rare cases where that distinction is important, but I've never encountered them myself. Pretty soon I think we'll be able to start using generics for I think with generics we may be able to extend The place to document the existing options would be the examples in the godoc here: Lines 50 to 59 in f44676a
|
It looks like what you call "empty" is called "zero value" in the Go world. See for example So I think an |
The text was updated successfully, but these errors were encountered: