-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Fixes and refactorings related to using mnsync in tests #3136
Conversation
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.
utACK
Rebased to fix merge conflicts after #3127 |
…ntroduce `force_finish_mnsync` for MNs only
…d drop local unused functions Also move the call, `force_finish_mnsync` should be called before `connect_nodes_bi`
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.
re-utACK
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.
utACK
(from test_framework.util import *
makes me cringe a bit, not related to this refactoring though.)
* Drop `get_mnsync_status`, `wait_to_sync` and `sync_masternodes` and introduce `force_finish_mnsync` for MNs only * Use `force_finish_mnsync` from util.py in dip3-deterministicmns.py and drop local unused functions Also move the call, `force_finish_mnsync` should be called before `connect_nodes_bi`
While reviewing #3127 I suggested that we should keep
sync_masternodes
assuming it was helping tests to pass... but after digging a bit I realized that I was wrong and we don't needsync_masternodes
andwait_to_sync
in tests that do not use MNs at all actually. These functions were introduced in #950 back when we had non-deterministic MN list. They are still useful for MN test nodes (MNs won't accept incoming connections while syncing, there is actually a bug related to this indip3-deterministicmns.py
which is fixed in the second commit) but we can drop them everywhere else. Also, there is no need for the "slow" sync mode, it's always the "fast" one now and we can simplify the function.