fix: intermittent error in feature_governance.py due to exceed amount of votes #6710
Conversation
WalkthroughThe functional test in test/functional/feature_governance.py was updated to accommodate situations where the vote count may sometimes exceed the expected value of 24. The test logic now waits for the vote count to reach at least 24 and asserts that it remains below 25, allowing for occasional occurrences of a 25th vote. No changes were made to the declarations of exported or public entities. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📥 CommitsReviewing files that changed from the base of the PR and between 850606155bdefdbf7f9cbc563a868c6f05b3e472 and c33f34c. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (6)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 09aa42e and 850606155bdefdbf7f9cbc563a868c6f05b3e472.
📒 Files selected for processing (1)
test/functional/feature_governance.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
test/functional/feature_governance.py (2)
test/functional/test_framework/test_framework.py (1)
wait_until(900-901)test/functional/test_framework/util.py (1)
assert_greater_than(72-74)
🪛 Ruff (0.11.9)
test/functional/feature_governance.py
287-287: Undefined name assert_greater_than
(F821)
🪛 Pylint (3.3.7)
test/functional/feature_governance.py
[error] 287-287: Undefined variable 'assert_greater_than'
(E0602)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: x86_64-apple-darwin / Build depends
- GitHub Check: x86_64-pc-linux-gnu_multiprocess / Build depends
- GitHub Check: x86_64-pc-linux-gnu_nowallet / Build depends
- GitHub Check: x86_64-pc-linux-gnu / Build depends
- GitHub Check: x86_64-w64-mingw32 / Build depends
- GitHub Check: arm-linux-gnueabihf / Build depends
🔇 Additional comments (1)
test/functional/feature_governance.py (1)
286-287: LGTM! Logic change effectively handles the intermittent 25th vote.The modification correctly implements a range check allowing 24-25 votes instead of requiring exactly 24. This addresses the intermittent test failure described in the PR objectives while maintaining test reliability.
🧰 Tools
🪛 Ruff (0.11.9)
287-287: Undefined name
assert_greater_than(F821)
🪛 Pylint (3.3.7)
[error] 287-287: Undefined variable 'assert_greater_than'
(E0602)
Good catch! 👍 pls see #6712 |
|
Closed in favor of #6712 |
bd2aa80 refactor: use named constants, tweak and explain chosen numbers (UdjinM6) c16a62f test: adjust feature_governance.py (UdjinM6) 4bcacc5 fix: request governance votes from more nodes on regtest (UdjinM6) Pull request description: ## Issue being fixed or feature implemented We only ask `nPeersPerHashMax` (3) nodes for governance votes for the same governance object when syncing. However on regtest we also isolate nodes to create conflicting triggers and since we have 5 nodes to sync from asking 3 of them often results in asking "non-isolated" nodes only (24 votes) yet sometimes we do ask previously isolated node too (25 votes). Should fix `feature_governance.py` flakiness. Alternative to #6710. ## What was done? Bump `nPeersPerHashMax` for regtest. Add more asserts in tests to fail earlier if smth isn't right, check votes on all nodes. ## How Has This Been Tested? run `feature_governance.py` ## Breaking Changes n/a ## Checklist: - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ Top commit has no ACKs. Tree-SHA512: 10885df4d1252d09051b053703c097f9522c79a0f3fa744ea1287aeb23ff47d24371dc38478255378898fc1b7e87785d0a852f7315562d1e19089637c15e03f3
2 participants
Issue being fixed or feature implemented
What was done?
This PR is follow-up for #6646
It has been introduced wait_until to wait until governance votes will reach exactly 24, but sometimes it could be 25 of them. This PR relax condition for amount of votes to be 24 or 25.
How Has This Been Tested?
Run multiple times with changes from #6631 + extra changes from local branch; no more failures.
Breaking Changes
N/A
Checklist: