Previously, interface_bitcoin_cli.py measured the -rpcwaittimeout wait floor with time.time(). bitcoin-cli arms that deadline with std::chrono::steady_clock, so a CI wall-clock step backward can make the elapsed time look short and cause the assert to fail.
Fix this by using time.monotonic() instead and using -rpcwaittimeout=1. The log line also now says "at least" so it matches the floor assert.
h/t ViniciusCestarii for suggesting time.monotonic() here.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline and AI policy for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | maflcko, ViniciusCestarii |
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
477 | @@ -478,7 +478,8 @@ def run_test(self): 478 | self.stop_node(0) # stop the node so we time out 479 | start_time = time.time() 480 | assert_raises_process_error(1, "Could not connect to the server", self.nodes[0].cli('-rpcwait', '-rpcwaittimeout=5').echo) 481 | - assert_greater_than_or_equal(time.time(), start_time + 5) 482 | + # Allow small scheduling slack. rpcwaittimeout is best effort wall clock. 483 | + assert_greater_than_or_equal(time.time() + 0.5, start_time + 5)
seems fine, but the 5 can probably be reduced to 1, to speed up the test
lgtm ACK 7445428eb851ab5314dab2cbe2dbe34fdd58200f
<!--meta-tag:bot-skip-->
start_time is taken before spawning bitcoin-cli and bitcoin-cli uses monotonic time, so the flake can only come from wall clock adjustment. Wouldn't replacing time.time() with time.monotonic() fix this without the slack?
Good catch, not sure how I missed this. I presume I was confused by the LHS and RHS both doing addition. Maybe it would be clearer to understand and to read the assertion error message, when the LHS was monotonic()-start_time, so that only the duration is printed. Steps to reproduce:
faketime --exclude-monotonic -f "+0 x0.5" ./bld-cmake/test/functional/interface_bitcoin_cli.py
Require the `-rpcwait` command to wait at least the configured timeout
period before raising a process error. Measure the wait using
`time.monotonic()` so that wall-clock discrepancies do not cause
intermittent test failures.
Also, lower the timeout from 5 to 1 second.
Updates:
Addressed @ViniciusCestarii's feedback (use time.monotonic())
Addressed @maflcko's feedback (assert elapsed duration on the LHS)
lgtm ACK dc27539d42bbfc04705d9ae1ea00cf70b046d8fa
ACK dc27539d42bbfc04705d9ae1ea00cf70b046d8fa