0

I have a singleton class B which contains a certain method RunTimer() that I'd like to unit test which is a wrapper around an external utility. For unit test, I want to exclude the testing of the timer and rely on other parameters (not shown here)

Fake is what I'd use in unit test which will contain additional ways for the sake of testing.

Since there's only ever a single instance of a Singleton, does the idea of switching out the std::unique_ptr<ITimer> _ptr for Fake in unit test make sense for the sake of testing?

class ITimer
{
    public:
    virtual void foo() = 0;
    virtual ~Base() = default;
};

class Real : public ITimer
{
    public:
    void foo() override {}
};

// For unit test
class Fake : public ITimer
{
    public:
    void foo() override {}
};

class B
{
    std::unique_ptr<ITimer> _ptr;

    // default ctor creates a Real
    B() : B(std::make_unique<Real>())
    {

    }

    B(std::unique_ptr<ITimer> ptr) : _ptr(std::move(ptr))
    {

    }

    public:
    static B& get()
    {
        static B b;
        return b;
    }
 
    void RunTimer()
    {
       Timer::Run(); // external utility
    }
  
    void change(std::unique_ptr<Base> ptr)
    {
        _ptr = std::move(ptr);
    }
};

int main()
{
    B::get();       // use Real in actual code
    
    // use Fake - for testing (eventually to be in TEST_F())
    auto mk = std::make_unique<Fake>();
    B::get().change(std::move(mk));
    // do testing on B::_ptr
}
xyf
  • 664
  • 1
  • 6
  • 16
  • 1
    This seems like a good opportunity to ask yourself "Does my class really _need_ to be a singleton?" Making a `B` instance with a `Real` time and then throwing it away and using a `Fake` timer instead seems like more work than just allowing yourself to make a `B` instance with a `Fake` timer in the first place. – Nathan Pierson Jan 12 '23 at 22:49
  • @xyf Of course you can do it that way. – Gunther Jan 12 '23 at 22:53
  • In the production there's only ever going to be a single instance of `B`. It's just for the sake of testing I am trying to see what fits in with the design...something that's practical and feasible – xyf Jan 12 '23 at 22:53
  • Singletons are notorious when it comes to testing. Here is some worthwhile reading: https://stackoverflow.com/questions/2085953/unit-testing-with-singletons – paddy Jan 12 '23 at 23:13
  • Not really sure I understand the problem. The unit test should not use `B::get()` at all (since presumably the real external utility code is not available in the test environment), it should use something like `B b(std::make_unique())` instead (obvious, the test environment will need access to the constructor) and then do tests on `b` only. Also, I would expect `Timer::Run();` to be `_ptr->foo()` instead. – Remy Lebeau Jan 12 '23 at 23:31
  • 1
    This will work and is better than not writing tests, but this goes against some best practices. To make tests robust and easier to write, you should avoid singletons and instead use dependency injection (with a framework or something manual). Just because there is a single instance of the type in your application, that doesn't mean you must make it a singleton. Using dependency injection can be more work than it is worth - it depends on many factors (code size / complexity, testing requirements, etc.). – Dean Johnson Jan 12 '23 at 23:38
  • @RemyLebeau so basically you're implying `B` shouldn't be a singleton eh? – xyf Jan 13 '23 at 00:45
  • 1
    First: is `change` part of actual the interface, is it going to be added just for the sake of testing? If the latter, I'd say that's a bad practice as it gives the client in production code extra possibilities which are not really intended. If the class has to be singleton, I'd try to replace the "change" with templates, still better than no tests at all. Second: still, consider making the class non-singleton and simply passing it by reference instead of calling `get()` in client's code. – alagner Jan 13 '23 at 07:13
  • @alagner perhaps I could remove `change` since it was mainly for testing and rather stick to `FRIEND_TEST` to access the private member for the sake of testing. That way the original class doesn't expose `_ptr` – xyf Jan 13 '23 at 17:01
  • @xyf the main question is: are you adding tests to existing code, designing the new one, or what exactly are you doing? Because befriending class with the test code is also something to be avoided. On the other hand: it's sometimes the lesser evil, i.e. better that than on testing at all. But again: what is the broader context? – alagner Jan 15 '23 at 14:24

0 Answers0