-
Notifications
You must be signed in to change notification settings - Fork 636
fix: make TestMempoolProgressInHigherRound less flaky #1953
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
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.
Thanks @faddat ❤️ Unsure how changing/adding comments fixes the test's flackiness though. Could you please explain?
wrangling.... will mark this as draft. |
That's right draft is a good way for this one. |
privval/socket_listeners_test.go
Outdated
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.
Are these changes somehow related with the mempool test?
ensureNewRound(newRoundCh, height, round) // first round at first height | ||
ensureNewEventOnChannel(newBlockCh) // first block gets committed | ||
// Ensure new round at first height and block commit | ||
ensureNewRound(newRoundCh, height, round) |
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.
The problem here is that the new round event takes more than 200ms?
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.
These are both good questions that I had missed. I will review this tomorrow and thank you for your review.
Also, yes I think so.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This PR should make TestMempoolProgressInHigherRound less flaky and a teensy big more rigorous.
The critical change is the order of the steps in the test itself, which does seem to make the test more reliable.
If the order should not have changed then we should probably just close this PR and try to find another way to make the test more reliable.
closes: #1932
found in: #1954
PR checklist
.changelog
(we use unclog to manage our changelog)docs/
orspec/
) and code comments