-
Notifications
You must be signed in to change notification settings - Fork 79
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
core/validatorapi: add support for DutySyncMessage
#1244
Conversation
Codecov ReportBase: 53.54% // Head: 53.57% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1244 +/- ##
==========================================
+ Coverage 53.54% 53.57% +0.03%
==========================================
Files 139 139
Lines 16152 16208 +56
==========================================
+ Hits 8648 8684 +36
- Misses 6260 6270 +10
- Partials 1244 1254 +10
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
||
data, ok := set[pk] | ||
require.True(t, ok) | ||
require.Equal(t, core.NewPartialSignedSyncMessage(msg, 0), data) |
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.
how do you know this code is actually called? (please do not use ctx cancel pattern, anymore, rather use something like
done := make(chan struct)
...
close(done)
...
<-done
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.
we can add a counter which is incremented each time the subscription function is called. we can use it to assert that this code block is actually called.
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.
Sure, but this test doesn't use the ctx cancel pattern
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.
as long as it is threadsafe then sure
Adds support for
DutySyncMessage
to validatorapi component. Also, adds tests.category: feature
ticket: #1188