-
Notifications
You must be signed in to change notification settings - Fork 720
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 TraceNodeIsLeader
JSON parser
#4187
Conversation
It does, just that the test is flaky |
b39c168
to
79de6c7
Compare
79de6c7
to
d30cc61
Compare
bors r+ |
4187: Fix `TraceNodeIsLeader` JSON parser r=Jasagredo a=Jasagredo The log messages are formatted slightly different in each case. In particular: ``` {...,"data":{...,"kind":"TraceAddBlockEvent.AddedToCurrentChain","...},...} ``` versus ``` {"...,"data":{...,"val":{"kind":"TraceNodeIsLeader","slot":85}},...} ``` Also the parser was intended to only get the events with the kind `TraceNodeIsLeader`. Co-authored-by: Javier Sagredo <jasataco@gmail.com>
Build failed: |
bors retry |
Build succeeded: |
k <- v .: "val" >>= (.: "kind") | ||
if k == "TraceNodeIsLeader" | ||
then TraceNodeIsLeader k <$> (v .: "val" >>= (.: "slot")) | ||
else fail "Not the right kind" |
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.
@jasagredo @newhoggy What is the failure "Not the right kind" going to indicate to the user? What is not the right kind? The error message says pretty much nothing.
@jasagredo can you improve this message to something like "Expected kind TraceNodeIsLeader but got: " <> k
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 thing. This is only used in tests though, but yeah, a more informative error message would be better.
The log messages are formatted slightly different in each case. In particular:
versus
Also the parser was intended to only get the events with the kind
TraceNodeIsLeader
.