ElapseSteady was introduced a while back, but is only used in one place. It makes more sense if this were a context manager, so that mocktime does not leak from one test into the next.
So turn it into a context manager, rename it and allow easy time advancement via e.g. steady_ctx += 1h.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | l0rinc, ismaelsadeeq |
If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.
Reviewers, this pull request conflicts with the following ones:
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.
32+ SteadyClockContext& operator=(const SteadyClockContext&) = delete;
33+
34+ /** Change mocktime by the given duration delta */
35+ void operator+=(std::chrono::milliseconds d)
36 {
37+ Assert(d >= 0s); // Steady time can only increase monotonically.
0 is allowed, which wouldn’t increase it
21- }
22- void operator()(std::chrono::milliseconds d)
23+
24+public:
25+ /** Initialize with INITIAL_MOCK_TIME. */
26+ explicit SteadyClockContext() { (*this) += 0s; }
+1 for explicit, too bad we can’t add [[nodiscard]] since we would want to avoid nameless temporaries like SteadyClockContext{}; (given that the name is unused) - which would self-destruct immediately.
nit: it seemed that the unit is a millisecond here, consider: (*this) += 0ms;
24+public:
25+ /** Initialize with INITIAL_MOCK_TIME. */
26+ explicit SteadyClockContext() { (*this) += 0s; }
27+
28+ /** Unset mocktime */
29+ ~SteadyClockContext() { MockableSteadyClock::ClearMockTime(); }
4@@ -5,16 +5,30 @@
5 #ifndef BITCOIN_TEST_UTIL_TIME_H
6 #define BITCOIN_TEST_UTIL_TIME_H
7
8+#include <util/check.h>
9 #include <util/time.h>
10
11-struct ElapseSteady {
12+
13+/// Helper to initialize the global MockableSteadyClock, let a duration elapse,
ACK facb2aab26dffbc1e46809ac776ed43b9eaa9ad4
It’s a small improvement, mostly in documenting that this sets global state and that the context cleans up after itself (which isn’t really used outside of a single test as far as I can tell, but it’s still cleaner than before)
Labels
Tests