test: Use mocktime to avoid intermittent failure in rpc_tests #21676

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:2104-testFixMocktime changing 6 files +7 −16

MarcoFalke commented at 8:08 AM on April 14, 2021: member

See #21602 (review)
test: Use mocktime to avoid intermittent failure fa78590a8f
MarcoFalke cross-referenced this on Apr 14, 2021 from issue rpc: add additional ban time fields to listbanned by jarolrod
fanquake added the label Tests on Apr 14, 2021
fanquake commented at 8:23 AM on April 14, 2021: member

Concept ACK
jonatack commented at 8:45 AM on April 14, 2021: contributor

tACK fabc39f3563b027f5750cad2f41ecbde80dd7f5f with light code review, some comments

jonatack commented at 9:05 AM on April 14, 2021: contributor

What about these

src/test/miner_tests.cpp:512:    SetMockTime(0);
src/wallet/test/wallet_tests.cpp:299:    SetMockTime(0);
src/wallet/test/wallet_tests.cpp:382:    SetMockTime(0);

in src/test/util/setup_common.cpp:123 in fabc39f356 outdated

 119 | @@ -120,6 +120,7 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve
 120 |  
 121 |  BasicTestingSetup::~BasicTestingSetup()
 122 |  {
 123 | +    SetMockTime(0s);

jonatack commented at 9:06 AM on April 14, 2021:

    SetMockTime(0s); // Reset mock time for other tests.

MarcoFalke commented at 3:38 PM on April 14, 2021:

Thanks, fixed

DrahtBot commented at 11:30 AM on April 14, 2021: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #21618 (WIP: Reduce MinRelayFee slower when Mempool utilised and faster when needed. by rebroad)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
DrahtBot cross-referenced this on Apr 14, 2021 from issue WIP: Reduce MinRelayFee slower when Mempool utilised and faster when needed. by rebroad

test: Reset mocktime in the common setup

Doing it there will reduce code bloat and also ensure no test can "forget" to reset it

fa40d6a1c4

MarcoFalke force-pushed on Apr 14, 2021
jarolrod commented at 4:20 AM on April 15, 2021: member

ACK fa40d6a1c47ac7f3dc6c11a2e6642cfef95422c1

This gives us tighter control of time and will prevent any time slippage making this test fail. I couldn't make the banned time test fail in practice, but this prevents that.
jonatack commented at 7:31 AM on April 15, 2021: contributor

Code review ACK fa40d6a1c47ac7f3dc6c11a2e6642cfef95422c1
MarcoFalke merged this on Apr 15, 2021
MarcoFalke closed this on Apr 15, 2021
MarcoFalke deleted the branch on Apr 15, 2021
MarcoFalke commented at 8:15 AM on April 15, 2021: member
As predicted, this happens in practice:
```
test/rpc_tests.cpp(292): error: in "rpc_tests/rpc_ban": check time_remaining == banned_until - now has failed [200 != 199]
```
https://cirrus-ci.com/task/5877648200040448?logs=ci#L7931
sidhujag referenced this in commit cad4555475 on Apr 15, 2021
bitcoin locked this on Aug 16, 2022

Contributors

Labels

Linked (view graph)

#21602 rpc: add additional ban time fields to listbanned

test: Use mocktime to avoid intermittent failure in rpc_tests #21676

Conflicts