test: more reliable sheduler call for feature_governance_cl.py #7041
Conversation
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
knst
force-pushed
the
fix-governance-cl
branch
from
6437fa0 to
55136b6
Compare
WalkthroughThe changes to Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/functional/feature_governance_cl.py (1)
145-155: Per-iteration block generation before scheduler looks sound; consider documenting intentThe added
generate(..., sync_fun=lambda: self.sync_blocks(self.nodes[0:5]))inside the(delta, expected)loop cleanly advances the chain on non-isolated nodes before each governance-cleanup scheduler run, which matches the goal of making the expected log sequence deterministic. The ordering withself.mocktime += deltaand subsequentmockscheduler(delta)calls is internally consistent and doesn’t introduce obvious race or state issues.Given that this extra block-per-phase is a subtle test stabilization detail, consider adding a brief comment above the loop explaining that the additional generation is required to reliably age and delete old governance triggers across nodes. This would help future readers understand why the extra block is needed here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/functional/feature_governance_cl.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
test/functional/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Functional tests in test/functional/ must be written in Python (minimum version specified in .python-version) and depend on dashd and dash-node
Files:
test/functional/feature_governance_cl.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6926
File: test/functional/feature_governance_cl.py:0-0
Timestamp: 2025-10-28T08:54:00.392Z
Learning: In Dash tests, the scheduler (mockscheduler) operates independently of network state. Isolated nodes should still run scheduler-based cleanup processes like governance cleanup, even if they have different state due to network isolation.
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/governance/**/*.{cpp,h} : Governance implementation must support governance objects (proposals, triggers, superblock management) and on-chain voting with tallying
Applied to files:
test/functional/feature_governance_cl.py
🧬 Code graph analysis (1)
test/functional/feature_governance_cl.py (1)
test/functional/test_framework/test_framework.py (1)
sync_blocks(812-832)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: linux64_multiprocess-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: mac-build / Build source
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: arm-linux-build / Build source
1 participant
Issue being fixed or feature implemented
Fixes intermittent failure in feature_governance_cl.py
This error happens ~10-20% on my localhost in case of multi-parallel runs of this specific test.
What was done?
Generated one more block to trigger scheduler.
I am not sure why it helps, but it drops failure rate to almost 0% (unrelated failures) for my local environment.
See this PR #6926 which improved reliability of feature_governance_cl.py significantly in the past.
How Has This Been Tested?
Run multiple times with and without changes from this PR:
Breaking Changes
N/A
Checklist: