-
Notifications
You must be signed in to change notification settings - Fork 4.5k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
status: modify TestStatus_ErrorDetails_Fail to replace protoimpl package #6953
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #6953 +/- ##
==========================================
- Coverage 83.71% 83.69% -0.02%
==========================================
Files 287 287
Lines 30926 30929 +3
==========================================
- Hits 25889 25886 -3
- Misses 3972 3979 +7
+ Partials 1065 1064 -1
|
got := tc.s.Details() | ||
if !cmp.Equal(got, tc.i, cmp.Comparer(proto.Equal), cmp.Comparer(equalError)) { | ||
t.Errorf("(%v).Details() = %+v, want %+v", str(tc.s), got, tc.i) | ||
details := tc.s.Details() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should first ensure len(details) == len(tc.want)
here or it could panic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's right! thanks for catching that
status/status_test.go
Outdated
t.Errorf("(%v).Details() = %+v, want %+v", str(tc.s), got, tc.i) | ||
details := tc.s.Details() | ||
for i, d := range details { | ||
// s.Deatils can either contain an error or a proto message. We |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
The protoimpl package is being currently used in a test (which got introduced in #6919). But this package cannot be accessed directly in google3, hence we need a different way of testing the case.
RELEASE NOTES: none