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

fix: update Empty()'s error message #1398

Closed
wants to merge 1 commit into from
Closed

fix: update Empty()'s error message #1398

wants to merge 1 commit into from

Conversation

flowck
Copy link

@flowck flowck commented Jun 5, 2023

Summary

The message returned by Empty() is a bit confusing because it states the following: Should be empty, but was [1, 2].

Changes

  • Update the error message thrown/panicked by assert.Empty()
  • Update the error message thrown/panicked by assert.NotEmpty()

Testcase

https://go.dev/play/p/Rx_KYuAQe2J

The message returned by Empty() is a bit confusing because it states the following: Should be empty, but was.
@dolmen dolmen added the pkg-assert Change related to package testify/assert label Jul 5, 2023
@@ -705,7 +705,7 @@ func Empty(t TestingT, object interface{}, msgAndArgs ...interface{}) bool {
if h, ok := t.(tHelper); ok {
h.Helper()
}
Fail(t, fmt.Sprintf("Should be empty, but was %v", object), msgAndArgs...)
Fail(t, fmt.Sprintf("Should be empty, but isn't %v", object), msgAndArgs...)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The message shows the unexpected value. is is ok.

Suggested change
Fail(t, fmt.Sprintf("Should be empty, but isn't %v", object), msgAndArgs...)
Fail(t, fmt.Sprintf("Should be empty, but is %v", object), msgAndArgs...)

@dolmen
Copy link
Collaborator

dolmen commented Jul 6, 2023

The problem you see in the original message is not obvious to me. Could you explain with more details what you find confusing in that message and how you are solving the issue?

@flowck flowck closed this by deleting the head repository Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted pkg-assert Change related to package testify/assert
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants