fix(gomock): display nil slices distinctly from empty slices in error messages#308
Open
wasim-builds wants to merge 1 commit into
Open
fix(gomock): display nil slices distinctly from empty slices in error messages#308wasim-builds wants to merge 1 commit into
wasim-builds wants to merge 1 commit into
Conversation
… messages When comparing nil slices with empty slices, the error message showed identical values for both 'Got' and 'Want' sides, making it impossible to distinguish them (issue uber-go#69). Updated formatGottenArg to detect nil slices using reflection and display them as 'nil' instead of '[]' in error output. Closes uber-go#69
Author
|
By the way, I've really been enjoying contributing to this project! My goal is to get more involved in open source to help maintain projects and learn from great engineering teams. Is there a process or pathway for becoming a member of the uber-go GitHub organization? I'd love to continue helping out where I can! |
Author
|
Hey 👋 Friendly bump on this one — fixes confusing gomock error messages when comparing nil vs empty slices. Small change, ready for review when you have a moment! 🙏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes confusing error messages when comparing nil slices with empty slices in gomock error output.
Problem
When a nil slice is passed where an empty slice is expected (or vice versa), the error message shows identical values for both "Got" and "Want", making it impossible to distinguish them:
This is because
fmt.Sprintf("%v", ...)formats both nil slices and empty slices as[].Solution
Update
formatGottenArgto detect nil slices usingreflect.ValueOfand display them asnilinstead of[]:Changes
gomock/call.go: UpdatedformatGottenArgto check if the argument is a nil slice using reflection, and format it asnil (<type>)instead of[] (<type>).gomock/controller_test.go: AddedSliceOfArraysMethodto the testSubjectstruct. AddedTestNilSliceVsEmptySliceandTestNilSliceOfArraysVsEmptytest cases to verify the fix for both[]byteand[][32]bytetypes (the exact type from the original issue report).Testing
All existing tests pass. Two new test cases added:
TestNilSliceVsEmptySlice- verifies[]bytenil vs empty distinctionTestNilSliceOfArraysVsEmpty- verifies[][32]bytenil vs empty distinction (the exact scenario from issue Wrong print msg when comparing initialized and uninitialized slice #69)Closes #69