Make CachedResult test more deterministic by using steady_clock

src/tech/CMakeLists.txt

@@ -12,7 +12,6 @@ target_include_directories(coincenter_tech PUBLIC include)
 
 add_unit_test(
     cachedresult_test
-    src/cachedresultvault.cpp
     test/cachedresult_test.cpp
     LIBRARIES
     spdlog

src/tech/include/cachedresult.hpp

@@ -11,29 +11,34 @@
 #include "timedef.hpp"
 
 namespace cct {
-
-class CachedResultOptions {
+template <class DurationT>
+class CachedResultOptionsT {
  public:
-  explicit CachedResultOptions(Duration refreshPeriod) : _refreshPeriod(refreshPeriod) {}
+  explicit CachedResultOptionsT(DurationT refreshPeriod) : _refreshPeriod(refreshPeriod) {}
 
-  CachedResultOptions(Duration refreshPeriod, CachedResultVault &cacheResultVault)
+  CachedResultOptionsT(DurationT refreshPeriod, CachedResultVaultT<DurationT> &cacheResultVault)
       : _refreshPeriod(refreshPeriod), _pCacheResultVault(std::addressof(cacheResultVault)) {}
 
  private:
-  template <class T, class... FuncTArgs>
-  friend class CachedResult;
+  template <class, class, class...>
+  friend class CachedResultT;
 
-  Duration _refreshPeriod;
-  CachedResultVault *_pCacheResultVault = nullptr;
+  DurationT _refreshPeriod;
+  CachedResultVaultT<DurationT> *_pCacheResultVault = nullptr;
 };
 
+using CachedResultOptions = CachedResultOptionsT<Duration>;
+
 /// Wrapper of an object of type T (should be a functor) for which the underlying method is called at most once per
 /// given period of time. May be useful to automatically cache some API calls in an easy and efficient way.
-template <class T, class... FuncTArgs>
-class CachedResult : public CachedResultBase {
+template <class ClockT, class T, class... FuncTArgs>
+class CachedResultT : public CachedResultBase<typename ClockT::duration> {
  public:
   using ResultType = std::remove_cvref_t<decltype(std::declval<T>()(std::declval<FuncTArgs>()...))>;
+  using TimePoint = typename ClockT::time_point;
+  using Duration = typename ClockT::duration;
   using ResPtrTimePair = std::pair<const ResultType *, TimePoint>;
+  using State = typename CachedResultBase<Duration>::State;
 
  private:
   using TKey = std::tuple<std::remove_cvref_t<FuncTArgs>...>;
@@ -42,18 +47,18 @@ class CachedResult : public CachedResultBase {
 
  public:
   template <class... TArgs>
-  explicit CachedResult(CachedResultOptions opts, TArgs &&...args)
-      : CachedResultBase(opts._refreshPeriod), _func(std::forward<TArgs &&>(args)...) {
+  explicit CachedResultT(CachedResultOptionsT<Duration> opts, TArgs &&...args)
+      : CachedResultBase<Duration>(opts._refreshPeriod), _func(std::forward<TArgs &&>(args)...) {
     if (opts._pCacheResultVault) {
       opts._pCacheResultVault->registerCachedResult(*this);
     }
   }
 
-  CachedResult(const CachedResult &) = delete;
-  CachedResult &operator=(const CachedResult &) = delete;
+  CachedResultT(const CachedResultT &) = delete;
+  CachedResultT &operator=(const CachedResultT &) = delete;
 
-  CachedResult(CachedResult &&) noexcept = default;
-  CachedResult &operator=(CachedResult &&) noexcept = default;
+  CachedResultT(CachedResultT &&) noexcept = default;
+  CachedResultT &operator=(CachedResultT &&) noexcept = default;
 
   /// Sets given value associated to the key built with given parameters,
   /// if given timestamp is more recent than the one associated to the value already present at this key (if any)
@@ -71,18 +76,18 @@ class CachedResult : public CachedResultBase {
   template <class... Args>
   const ResultType &get(Args &&...funcArgs) {
     TKey key(std::forward<Args &&>(funcArgs)...);
-    TimePoint t = Clock::now();
+    auto t = ClockT::now();
     auto flattenTuple = [this](auto &&...values) { return _func(std::forward<decltype(values) &&>(values)...); };
-    if (_state == State::kForceUniqueRefresh) {
+    if (this->_state == State::kForceUniqueRefresh) {
       _cachedResultsMap.clear();
-      _state = State::kForceCache;
+      this->_state = State::kForceCache;
     }
     auto it = _cachedResultsMap.find(key);
     if (it == _cachedResultsMap.end()) {
       TValue val(std::apply(flattenTuple, key), t);
       it = _cachedResultsMap.insert_or_assign(std::move(key), std::move(val)).first;
-    } else if (_state != State::kForceCache && _refreshPeriod != Duration::max() &&
-               it->second.second + _refreshPeriod < t) {
+    } else if (this->_state != State::kForceCache && this->_refreshPeriod != Duration::max() &&
+               it->second.second + this->_refreshPeriod < t) {
       it->second = TValue(std::apply(flattenTuple, std::move(key)), t);
     }
     return it->second.first;
@@ -102,4 +107,7 @@ class CachedResult : public CachedResultBase {
   MapType _cachedResultsMap;
 };
 
+template <class T, class... FuncTArgs>
+using CachedResult = CachedResultT<Clock, T, FuncTArgs...>;
+
 }  // namespace cct

src/tech/include/cachedresultvault.hpp

@@ -4,38 +4,60 @@
 #include "timedef.hpp"
 
 namespace cct {
+template <class DurationT>
 class CachedResultBase {
  protected:
-  friend class CachedResultVault;
+  template <class>
+  friend class CachedResultVaultT;
 
   enum class State { kStandardRefresh, kForceUniqueRefresh, kForceCache };
 
-  explicit CachedResultBase(Duration refreshPeriod) : _refreshPeriod(refreshPeriod) {}
+  explicit CachedResultBase(DurationT refreshPeriod) : _refreshPeriod(refreshPeriod) {}
 
   void freeze() noexcept { _state = State::kForceUniqueRefresh; }
 
   void unfreeze() noexcept { _state = State::kStandardRefresh; }
 
-  Duration _refreshPeriod;
+  DurationT _refreshPeriod;
   State _state = State::kStandardRefresh;
 };
 
 /// Represents an Observer of CachedResults.
 /// It can be used to launch queries on all objects listening to this observer.
-class CachedResultVault {
+template <class DurationT>
+class CachedResultVaultT {
  public:
-  CachedResultVault() noexcept = default;
-
-  void registerCachedResult(CachedResultBase &cacheResult);
-
-  void freezeAll();
-
-  void unfreezeAll() noexcept;
+  CachedResultVaultT() noexcept = default;
+
+  void registerCachedResult(CachedResultBase<DurationT> &cacheResult) {
+    _cachedResults.push_back(std::addressof(cacheResult));
+  }
+
+  void freezeAll() {
+    if (!_allFrozen) {
+      for (CachedResultBase<DurationT> *p : _cachedResults) {
+        p->freeze();
+      }
+      _allFrozen = true;
+    }
+  }
+
+  void unfreezeAll() noexcept {
+    if (_allFrozen) {
+      for (CachedResultBase<DurationT> *p : _cachedResults) {
+        p->unfreeze();
+      }
+      _allFrozen = false;
+    }
+  }
 
  private:
-  using CachedResultPtrs = vector<CachedResultBase *>;
+  using CachedResultPtrs = vector<CachedResultBase<DurationT> *>;
 
   CachedResultPtrs _cachedResults;
   bool _allFrozen = false;
 };
+
+using CachedResultVault = CachedResultVaultT<Duration>;
+
 }  // namespace cct

src/tech/include/timedef.hpp

@@ -5,6 +5,7 @@
 namespace cct {
 /// Alias some types to make it easier to use
 /// The main clock is system_clock as it is the only one guaranteed to provide conversions to Unix epoch time.
+/// It is not monotonic - thus for unit tests we will prefer usage of steady_clock
 using Clock = std::chrono::system_clock;
 using TimePoint = Clock::time_point;
 using Duration = Clock::duration;

src/tech/test/cachedresult_test.cpp

@@ -2,6 +2,7 @@
 
 #include <gtest/gtest.h>
 
+#include <chrono>
 #include <thread>
 
 namespace cct {
@@ -23,24 +24,35 @@ struct Incr {
   int nbCalls{};
 };
 
+// We use std::chrono::steady_clock for unit test as it is monotonic (system_clock is not)
+constexpr std::chrono::steady_clock::duration kCacheTime = std::chrono::milliseconds(2);
+constexpr auto kCacheExpireTime = 2 * kCacheTime;
+
+template <class T, class... FuncTArgs>
+using CachedResultSteadyClock = CachedResultT<std::chrono::steady_clock, T, FuncTArgs...>;
+
+using CachedResultOptionsSteadyClock = CachedResultOptionsT<std::chrono::steady_clock::duration>;
+
+using CachedResultVaultSteadyClock = CachedResultVaultT<std::chrono::steady_clock::duration>;
+
 }  // namespace
 
 class CachedResultTestBasic : public ::testing::Test {
  protected:
-  CachedResultTestBasic() : vault(), cachedResult(CachedResultOptions(std::chrono::milliseconds(1), vault)) {}
+  CachedResultTestBasic() : cachedResult(CachedResultOptionsSteadyClock(kCacheTime, vault)) {}
 
   virtual void SetUp() {}
   virtual void TearDown() {}
 
-  CachedResultVault vault;
-  CachedResult<Incr> cachedResult;
+  CachedResultVaultSteadyClock vault;
+  CachedResultSteadyClock<Incr> cachedResult;
 };
 
 TEST_F(CachedResultTestBasic, GetCache) {
   EXPECT_EQ(cachedResult.get(), 1);
   EXPECT_EQ(cachedResult.get(), 1);
   EXPECT_EQ(cachedResult.get(), 1);
-  std::this_thread::sleep_for(std::chrono::milliseconds(2));
+  std::this_thread::sleep_for(kCacheExpireTime);
   EXPECT_EQ(cachedResult.get(), 2);
   EXPECT_EQ(cachedResult.get(), 2);
 }
@@ -51,17 +63,17 @@ TEST_F(CachedResultTestBasic, Freeze) {
   EXPECT_EQ(cachedResult.get(), 2);
   EXPECT_EQ(cachedResult.get(), 2);
   EXPECT_EQ(cachedResult.get(), 2);
-  std::this_thread::sleep_for(std::chrono::milliseconds(2));
+  std::this_thread::sleep_for(kCacheExpireTime);
   EXPECT_EQ(cachedResult.get(), 2);
   vault.unfreezeAll();
   EXPECT_EQ(cachedResult.get(), 3);
 }
 
 class CachedResultTest : public ::testing::Test {
  protected:
-  using CachedResType = CachedResult<Incr, int, int>;
+  using CachedResType = CachedResultSteadyClock<Incr, int, int>;
 
-  CachedResultTest() : cachedResult(CachedResultOptions(std::chrono::milliseconds(1))) {}
+  CachedResultTest() : cachedResult(CachedResultOptionsSteadyClock(kCacheTime)) {}
 
   virtual void SetUp() {}
   virtual void TearDown() {}
@@ -73,16 +85,16 @@ TEST_F(CachedResultTest, GetCache) {
   EXPECT_EQ(cachedResult.get(3, 4), 7);
   EXPECT_EQ(cachedResult.get(3, 4), 7);
   EXPECT_EQ(cachedResult.get(3, 4), 7);
-  std::this_thread::sleep_for(std::chrono::milliseconds(2));
+  std::this_thread::sleep_for(kCacheExpireTime);
   EXPECT_EQ(cachedResult.get(3, 4), 14);
 }
 
 TEST_F(CachedResultTest, SetInCache) {
-  TimePoint t = Clock::now();
+  auto t = std::chrono::steady_clock::now();
   cachedResult.set(42, t, 3, 4);
   EXPECT_EQ(cachedResult.get(3, 4), 42);
   EXPECT_EQ(cachedResult.get(3, 4), 42);
-  std::this_thread::sleep_for(std::chrono::milliseconds(2));
+  std::this_thread::sleep_for(kCacheExpireTime);
   EXPECT_EQ(cachedResult.get(3, 4), 7);
   cachedResult.set(42, t, 3, 4);  // timestamp too old, should not be set
   EXPECT_EQ(cachedResult.get(3, 4), 7);
@@ -96,7 +108,7 @@ TEST_F(CachedResultTest, RetrieveFromCache) {
   RetrieveRetType ret = cachedResult.retrieve(-5, 3);
   ASSERT_NE(ret.first, nullptr);
   EXPECT_EQ(*ret.first, -2);
-  EXPECT_GT(ret.second, TimePoint());
+  EXPECT_GT(ret.second, std::chrono::steady_clock::time_point());
   EXPECT_EQ(cachedResult.retrieve(-4, 3), RetrieveRetType());
 }
 }  // namespace cct