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

arrays value types in a zero-initialized state are considered empty #1126

Merged
merged 2 commits into from Jun 20, 2022

Conversation

adamluzsi
Copy link
Contributor

@adamluzsi adamluzsi commented Nov 9, 2021

Summary

With this pull request, arrays value types in a zero-initialized state are considered empty.
Before that, only the zero-length arrays were considered empty.
The new behavior is backward compatible with the existing currently test coverage.

Changes

  • added test cases for assert.Empty with array value types.
  • added test cases for assert.NotEmpty with array value types.
  • Adjust implementation according to the new test cases
  • minor refactoring

Motivation

When I was reading the code source for learning purposes.
I discovered by chance that the assert.isEmpty / case reflect.Array code part doesn't have coverage.

assert.Empty(t, [1]int{})
assert.NotEmpty(t, [1]int{42})

Related issues

I'm not aware of any, but I didn't deeply check through the issues.

@adamluzsi
Copy link
Contributor Author

@adamluzsi adamluzsi commented Nov 10, 2021

@glesica, when you have time, can you perhaps take a look at the change? : )

@adamluzsi
Copy link
Contributor Author

@adamluzsi adamluzsi commented Jun 8, 2022

Any news on reviewing this PR?

Copy link
Collaborator

@boyan-soubachov boyan-soubachov left a comment

Makes sense. Thank you for your contribution :)

@boyan-soubachov boyan-soubachov merged commit 840cb80 into stretchr:master Jun 20, 2022
3 checks passed
@adamluzsi
Copy link
Contributor Author

@adamluzsi adamluzsi commented Jun 20, 2022

yay, I'm happy I was able to contribute to the project. :)))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants