fix: intermittent error in p2p_invalid_block.py #6709
Conversation
Mocktime here is misused here `t = int(time.time())` Functional test p2p_invalid_block.py is changed are adjusted for Dash Core
WalkthroughThe test in ✨ 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: 0
🧹 Nitpick comments (1)
test/functional/p2p_invalid_block.py (1)
141-141: Fix line length issue.The line exceeds the 100-character limit (110 characters). Consider breaking it into multiple lines for better readability.
- peer.send_blocks_and_test([block], node, force_send=True, success=False, reject_reason='time-too-new') + peer.send_blocks_and_test([block], node, force_send=True, success=False, + reject_reason='time-too-new')🧰 Tools
🪛 Pylint (3.3.7)
[convention] 141-141: Line too long (110/100)
(C0301)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/functional/p2p_invalid_block.py(1 hunks)
🧰 Additional context used
🪛 Pylint (3.3.7)
test/functional/p2p_invalid_block.py
[convention] 141-141: Line too long (110/100)
(C0301)
🔇 Additional comments (2)
test/functional/p2p_invalid_block.py (2)
135-135: Excellent fix for mocktime consistency!Using
self.mocktimeinstead ofint(time.time())ensures the test uses the controlled mock time rather than real system time, which eliminates the intermittent failures caused by time synchronization issues between the test framework and system clock.
142-142: Proper time advancement using framework helper.Using
self.bump_mocktime(1)is the correct way to advance mock time in the test framework, ensuring consistent time management and avoiding direct manipulation of time variables.
knst
force-pushed
the
fix-p2p_invalid_block
branch
from
547fbec to
c0cb338
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/functional/p2p_invalid_block.py (1)
140-140: Consider breaking the long line for better readability.The line exceeds the 100-character limit. Consider reformatting for better readability:
- peer.send_blocks_and_test([block], node, force_send=True, success=False, reject_reason='time-too-new') + peer.send_blocks_and_test([block], node, force_send=True, success=False, + reject_reason='time-too-new')🧰 Tools
🪛 Pylint (3.3.7)
[convention] 140-140: Line too long (110/100)
(C0301)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/functional/p2p_invalid_block.py(1 hunks)
🧰 Additional context used
🪛 Pylint (3.3.7)
test/functional/p2p_invalid_block.py
[convention] 140-140: Line too long (110/100)
(C0301)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: linux64_multiprocess-build / Build source
- GitHub Check: mac-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: arm-linux-build / Build source
🔇 Additional comments (2)
test/functional/p2p_invalid_block.py (2)
134-134: Excellent fix for test determinism!Replacing
int(time.time())withself.mocktimeproperly aligns with the test framework's mocktime mechanism. This prevents the intermittent failures that occur when mixing real time with mocktime, ensuring consistent and predictable test behavior.
141-141: Good use of the test framework's time management API.Using
self.bump_mocktime(1)instead of manually setting the mocktime is the recommended approach. This method properly updates all internal state and ensures consistency across the test framework, making the code more maintainable and reliable.
3 participants
Issue being fixed or feature implemented
#6694
What was done?
Mocktime here is misused here
t = int(time.time())Use instead helper
bump_mocktimeand current mocktime asself.mocktimeHow Has This Been Tested?
Run multiple times locally
Breaking Changes
N/A
Checklist: