Mockable clock meeting std::chrono TrivialClock requirement and interface

Question

I'm using the time for e.g. ping and timeout calculation as well as animations in a game. So my (big) codebase uses std::chrono::steady_clock::now() in many places. For testing I'd like to test those without having to resort to sleep and friends (timeout of >=5mins will be hard to test this way...)

For this I created a custom Clock class that uses a singleton-like instance which can be changed to provide a custom clock. A default of steady_clock is used for production code. This Clock can be used as a drop-in replacement of std::chrono::steady_clock and can be mocked in test code.

I'd like a review with suggestions how to improve this or if it has any flaws. Pointers to similar/better implementations are also welcome. I'm mostly concerned about the multiple indirections (and related performance penalties) this causes: Lookup of a "constructed" flag, the impl and the vtable (3 pointers) although I only have 1 function. See godbold

#include <chrono>
#include <memory>

struct BaseClock
{
    using Clock = std::chrono::steady_clock;
    using time_point = Clock::time_point;
    virtual ~BaseClock() = default;
    virtual time_point now(){ return std::chrono::steady_clock::now(); }
};

class Clock
{
    static std::unique_ptr<BaseClock>& inst(){
        static std::unique_ptr<BaseClock> clock(new BaseClock);
        return clock;
    }
public:
    using rep = BaseClock::Clock::rep;
    using duration = BaseClock::Clock::duration;
    using time_point = std::chrono::time_point<Clock>;

    static time_point now(){ return time_point(inst()->now().time_since_epoch()); }
    static void set(std::unique_ptr<BaseClock> clock){inst() = std::move(clock);}
};

int main()
{
    return Clock::now().time_since_epoch().count();
}

Example usage in test code:

struct MockClock: public BaseClock{
    static time_point current;
    time_point now() override { return current; }
};

void test_method(){
    Clock::set(new MockClock);
    MockClock::current = MockClock::time_point(100);
    testClass.methodUsingClock();
    MockClock::current += std::chrono::seconds(10);
    REQUIRE(testClass.checkTimeout());
}

Answer 1

While I think the underlying idea is quite nice, the actual implementation and design has some issues.

Wrong Abstraction Level

Let's start with the less obvious one: How would you actually use Clock or BaseClock with std::chrono::high_resolution_clock or std::chrono::system_clock?

The simplest approach would be something akin to this:

struct HighResClock : BaseClock {
    time_point now() override { return std::chrono::high_resolution_clock::now(); }
};

It seems so clean, so simple, and so easy. Except that it doesn't compile! That's because std::chrono::high_resolution_clock::time_point is not the same as BaseClock::time_point (and cannot easily be converted, if at all possible).

But: Do we actually need time_points in the public interface?

The only reason to expose time_point values is to extract time differences between them. But that only matters if arbitrary time_points are to be compared.

Technically, the time_point could be stored in some form. However, for many clocks, like std::chrono::steady_clock or std::chrono::high_resolution_clock, the epoch from when the clock are measuring their time offset can change between different executions of the same program (e.g. because the computer got rebooted).

This makes storing time_points, especially those not obtained from std::chrono::system_clock, rather useless. In that case, you'll likely need a calendar library (or similar) to get points of time in a storable format.

But in most cases, a simple Timer abstraction can fulfill all clock needs (comparing some time_points with some relation). A simple Timer interface could look like this:

struct timer {
    // choose an appropriate duration type
    using duration = std::chrono::duration<double>;

    virtual ~timer() = default;
    
    virtual void start() = 0;
    virtual duration stop() = 0;
    virtual duration tick() = 0; // to obtain multiple measurements from the same baseline
};

class steady_timer : public timer {
    using clock = std::chrono::steady_clock;
    using time_point = clock::time_point;

    time_point start_time;
    bool running;

public:
    steady_timer() = default;

    void start() override
    {
        start_time = clock::now();
        running = true;
    }

    duration tick() override
    {
        return duration(clock::now() - start_time);
    }

    duration stop() override
    {
        auto elapsed = tick();
        running = false;
        return elapsed;
    }
};

Now the only exposed part of the interface is the duration. And it is easily extensible to other time sources (e.g. Win32 QueryPerformanceCounter) or mockable.

Singleton

I really don't like the Clock singleton. Yes, it is easy to just ask a global clock. Yes, it is also easy to screw all code depending on this clock by changing the underlying instance.

For example, a test setting Clock to a mock but not restoring the original clock breaks all other tests that assume the default clock implementation - making test failure dependent on test execution order.

Instead, take a reference or pointer to a timer as parameter. This allows you to pass in a clock where needed, without changing (or corrupting) everyone elses timer.

Rewriting your test case:

class mock_timer : public timer {
    std::vector<duration> measurements;
    int counter = 0;

public:
    mock_timer(std::initializer_list<duration> il) : measurements(il) {}

    void start() override {}

    duration tick() override
    {
        if(counter < measurements.size()) return measurements[counter++];
        return measurements.back();
    }

    duration stop() override
    {
        return measurements.back(); // just example
    }
};

void test_method(){
    using namespace std::literals::chrono_literals;
    mock_timer my_timer{ 10s };

    testClass.methodUsingClock(my_timer);
    
    // or, more likely:
    testclass inst{my_timer};
    inst.methodUsingClock();

    REQUIRE(testClass.checkTimeout());
}

Answer 2

The use of a singleton will limit your possibilities, as hoffmale's review points out. It completely prevents concurrent testing. However, you'll find that getting the Clock instance to the code that needs it can easily add lots of "tramp data" to intermediated method signatures. I try to limit that by passing a factory/registry interface that allows classes to access the system interactions they need - time, filesystem, network, etc. In the tests, populate a mock-factory with the necessary mock objects, and in production, pass a concrete-factory that gives out objects with real system access.

One aspect that's missing is that this interface only gives access to now() - it doesn't handle the other clock-related actions that can cause slow tests. In particular, you'll want sleeps and network- or mutux-related timeouts to respect the mock clock's idea of time. To achieve that, you'll need to redirect those timeouts to mockable methods. That's a bigger job, but will give you a much more useful test regime.

I think that when I made something like that (many years ago, and in a different language), I ended up with the MockTime storing an ordered list of future events; whenever control entered its code, it could advance the time to match the next event (which could be the end of a sleep, release of a mutex, or an interrupt from an external (mocked) thread, for example).

Answer 3

If you care about performance, you can use templates to provide arguments at compile time.

Example

template <typename Clock>
class TestClass {
    Clock clock;

    public:
    void testMethod() {
        auto start = clock.currentTime();
        doStuff();
        auto end = clock.currentTime();
    }
}

Now you can use any class which has currentTime method

Edit: The downside of this approach, is if you have a lot of template instantiations, it will cripple your compile times. Do not optimize prematurely. Profile First!!!