Additional check for not-leader slots in LeadershipSchedule tests #5110
Conversation
carbolymer
requested review from
dcoutts,
Jimbo4350,
erikd,
newhoggy,
JaredCorduan and
CarlosLopezDeLara
as code owners
| getRelevantLeaderSlots :: FilePath -> Int -> H.PropertyT (ReaderT IntegrationState (ResourceT IO)) [Int] | ||
| getRelevantLeaderSlots poolNodeStdoutFile slotLowerBound = do | ||
| getRelevantSlots :: FilePath -> Int -> H.PropertyT (ReaderT IntegrationState (ResourceT IO)) ([Int], [Int]) | ||
| getRelevantSlots poolNodeStdoutFile slotLowerBound = do |
There was a problem hiding this comment.
This function now returns all slots seen in the logs. This is useful to double check if we didn't omit any slot, especially those with TraceNodeNotLeader entries in logs.
There was a problem hiding this comment.
Are TraceNodeIsLeader and TraceNodeNotLeader definitely the only two TraceNode kinds?
There was a problem hiding this comment.
There's also TraceNodeCannotForge - should we check for it as well?
carbolymer
force-pushed
the
mgalazyn/fix/check-for-missing-epochs-in-leadershipschedule
branch
from
5712fa8
to
45657ed
Compare
iohk-bors
bot
deleted the
mgalazyn/fix/check-for-missing-epochs-in-leadershipschedule
branch
| if k == "TraceNodeIsLeader" | ||
| then TraceNodeIsLeader k <$> (v .: "val" >>= (.: "slot")) | ||
| else fail $ "Expected kind was TraceNodeIsLeader, found " <> show k <> "instead" | ||
| data TraceNode |
There was a problem hiding this comment.
Why the rename? TraceNodeIsLeader was a good name.
There was a problem hiding this comment.
I needed a type which would also capture TraceNodeNotLeader . So my options were:
- create another similar type, with the same fields, duplicate
ToJSONinstance - rename the type, create two constructors
TraceNodeIsLeaderandTraceNodeNotLeader, both with fieldsslotandkind - rename the type and the constructor, add
isLeaderflag
3rd looked like the least amount of duplication. Which alternative would you propose?
|
|
||
| instance FromJSON Kind where | ||
| parseJSON = Aeson.withObject "Kind" $ \v -> | ||
| Kind <$> v .: "kind" | ||
|
|
||
| getRelevantLeaderSlots :: FilePath -> Int -> H.PropertyT (ReaderT IntegrationState (ResourceT IO)) [Int] | ||
| getRelevantLeaderSlots poolNodeStdoutFile slotLowerBound = do | ||
| getRelevantSlots :: FilePath -> Int -> H.PropertyT (ReaderT IntegrationState (ResourceT IO)) ([Int], [Int]) |
There was a problem hiding this comment.
Returning something like data LeadershipSlots = LeadershipSlots { leaderSlots :: [Int], notLeaderSlots :: [Int] } would make this more readable.
This change introduces additional check for not-leader slots in the tests.
related discussion: https://input-output-rnd.slack.com/archives/CR599HMFX/p1681424630819669 (internal access only)