Add unittests for ledger publisher and bug fixes #859
Conversation
Inject cache
| @@ -97,6 +102,7 @@ class LedgerPublisher | |||
|
|
|||
| if (!range || range->maxSequence < ledgerSequence) | |||
| { | |||
| ++numAttempts; | |||
There was a problem hiding this comment.
Fix bug, when numAttemps is x, we actually accessed db x+1 times.
| void | ||
| SetUp() override | ||
| { | ||
| MockBackendTest::SetUp(); |
There was a problem hiding this comment.
Do you actually need to do this manually? Or do you need specific sequence?
There was a problem hiding this comment.
1 We do need to call these manually. It inherits multiple classes.
2 Keeping the setup and teardown order does not matter for this usercase. Just my preference to keep such construction/ destruction actions symmetrical.
| dummyState.isWriting = true; | ||
| // age is -10 | ||
| ripple::LedgerInfo dummyLedgerInfo = CreateLedgerInfo(LEDGERHASH, SEQ, 0); | ||
| auto const now = duration_cast<seconds>(system_clock::now().time_since_epoch()); |
There was a problem hiding this comment.
It would be better to fix the time point here. Tests shouldn't depend on time.
There was a problem hiding this comment.
This is deliberately create a ledger with the future timestamp to trigger the error handling in lastCloseAgeSeconds function.
1 Add unittests to ledger publisher (100%)
2 Inject cache module, remove backend->cache().
3 Bug fix: Use SubscriptionManagerType; maxAttempts calculation