ETCM-645: Add it-test for checkpointing #931
Conversation
jvdp
left a comment
jvdp
left a comment
There was a problem hiding this comment.
So the tests look OK to me, superficially. (This FakePeer interface seems convenient.) Have you found a way to break them that indicates they're testing the right thing?
| case UnknownBranch => | ||
| val currentBlock = blocks.head.number.min(startingBlockNumber) | ||
| val goingBackTo = currentBlock - syncConfig.branchResolutionRequestSize | ||
| val goingBackTo = (currentBlock - syncConfig.branchResolutionRequestSize).max(0) |
There was a problem hiding this comment.
What is this change for? Reverting this doesn't seem to break anything. (Also goes for the branchResolutionRequestSize = 30 changes.)
There was a problem hiding this comment.
before the change val goingBackTo could be set to negative numbers, this shouldn't be the case as the resolution of the branches can't start from before the genesis block.
I actually had this happening during tests -> hence the fix.
As for branchResolutionRequestSize values changes in tests, I just aligned them with the value we use in application.conf.
not sure what you mean with breaking the tests, but changing the test setup breaks them easily |
By disabling / breaking the feature they're supposed to test, I meant. |
checkpointing is a huuuge piece of functionality, touching a lot of layers, it's not something that can be commented out. I just added the tests that were supposed to be there long time ago |
AnastasiiaL
force-pushed
the
feature/ETCM-645
branch
2 times, most recently
from
7be749c to
6770cfa
Compare
AnastasiiaL
force-pushed
the
feature/ETCM-645
branch
from
6770cfa to
675b893
Compare
Description
Integration tests added to simulate peers exchanging the checkpoints