Fix bootstrap unit tests#4383
Merged
dsiganos merged 37 commits intonanocurrency:developfrom Jan 26, 2024
Merged
Conversation
pwojcikdev
reviewed
Jan 22, 2024
d47ce4b to
a2bc018
Compare
clemahieu
previously approved these changes
Jan 26, 2024
It makes it easy to craete a disconnected node and it also makes the intention clear, that the node is meant to be special and disconnected
This is needed for NANO_TEST_BASE_PORT to work.
862e39c to
91dfe35
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes a number of issues with the bootstrap tests.
This work was triggered by #4368, which was flawed.
A common mistake that I have seen before is to convert manually created instances of nano::node (without using
system.add_node). Often people assume that this is just old code and that the manual construction of the object can be tidied up by the add_node call. But in many cases these manual node constructions are there so that the node is not connected to anything, add_node runs code to connect all nodes in a system.Many tests depended on or simply called block_processor.flush(). I removed all instances of block processor flush as it shouldn't be needed and is known to be buggy.
Some tests were supposedly disabling the legacy bootstrap and testing the lazy bootstrap. But since we now have ascending bootstrap, these tests had to modified to disable the ascending bootstrap too.
Many tests were setting the network port semi-automatically but there is no need to do so, it is more flexible to let the OS pick a port number.
I re-enabled some tests that were disabled due to failing intermittedly.
resolves #3559
resolves #3517
resolves #3640
resolves #3613