php Question posted in
Official manual can be found here.

How to correctly create a mock for class dependence with phpunit?

I have a service class with directly created object of TypeSaveLogRecord in it.

class TypeSaveLogService
{
    public function store(string $type, string $smId): TypeSaveLogRecord
    {
        $log = new TypeSaveLogRecord([
            'mdm_id' => $smId,
            'type' => $type
        ]);

        $saveResult = $log->save();
        if (!$saveResult) {
            throw new Exception('Failed to save type log');
        }
    }
}

I want to create a test for TypeSaveLogService to test store functionality.

Here is my version:

public function testStoreFailed()
{
    $Mock = $this->createMock(TypeSaveLogRecord::class);
    $Mock->method('save')->willReturn(false);

    $service = new TypeSaveLogService();
    $actualRes = $service->store($type, $smId);

    $this->expectException('Exception');
}

But it doesn’t work I can’t create Mock for TypeSaveLogRecord in this way. Could you tell what I do wrong ? I can’t change the code of TypeSaveLogService unfortunately.

Tags:

2

Answers


  1. 0

    The reason why this is not working is that you create a mock of the TypeSaveLogRecord in your test but basically it’s not used at all.

    If you call $Mock->save() in your test you will realize that this mock (not the original class though) will actually return false (meaning – the "willReturn-Type").
    Just creating a mock is generally not enough. You need to do something with this mock somehow. In this case you will still create a normal instance of TypeSaveLogRecord in your TypeSaveLogService class even though you have created a mock.

    This is exactly the reason, why you should put your dependencies (in this case TypeSaveLogService depends on TypeSaveLogRecord) in the constructor. That way you could inject your mocked class like this:

    class TypeSaveLogService
{
    public function __construct(public TypeSaveLogRecord $log) {}

    public function store(string $type, string $smId): TypeSaveLogRecord
    {
        $this->log->set([
            'mdm_id' => $smId,
            'type' => $type
        ]);

        $saveResult = $this->log->save();
        if (!$saveResult) {
            throw new Exception('Failed to save type log');
        }
    }
}

public function testStoreFailed()
{
    $Mock = $this->createMock(TypeSaveLogRecord::class);
    $Mock->method('save')->willReturn(false);

    $service = new TypeSaveLogService($Mock);
    $actualRes = $service->store($type, $smId);

    $this->expectException('Exception');
}

    If you cannot change the code of TypeSaveLogService I don’t see any solution to this problem because you won’t be able to interfere in the process from "outside" that way.

    Login or Signup to reply.
  2. 0

    This is typical for "implementing backwards", which we normally don’t do, we do it forwards, but not when it is the first time. This is often easy to improve:

    The store() method is part of the protocol of TypeSaveLogService and has been implemented to inherit (and hide) the detail of forwarding parameters to the TypeSaveLogRecord save() protocol.

    This is an implementation detail which you do not change to (unit-) test the implementation, especially not yet while you still design the protocol and the method is not even finished. Instead of testing for the expectation of calling the method, you already test for a detail in the method (hidden), which means your test will always break when you change the implementation. That is too early. Defer the details, just call the method in the test and expect the outcome of the happy path.

    Only then start to introduce error handling if even needed. Most often at this stage it is not even clear yet what is an error or if it even matters. ("define errors out of existence".)

    The return type should already guard this, so you are basically writing too much code, both within the method and within the test.

    A good program works with as little code as necessary, writing the test is not the task to add more code and solve a puzzle with the implementation (how do I test this?), but you use the test first of all to execute the method, in a very profane way. Just execute it, don’t think so much. When the test finally runs (with as little code as necessary), then try to break that by providing parameter values that break it. If you can’t break it than that’s it.

    Suggestions:

    1. Remove the creation and configuration of the mock object in the test, effectively removing the mock from the test.
    2. Remove the if clause in the method that checks the internal return value.
    3. Remove the variable and the assignment to it of the return value.
    4. Finish the method implementation with as little code as necessary.

    If you feel unsafe to not check for a false return value, short-circuit it and care about it later, right now the main goal is that it runs, not that there is error handling. that comes later. If you look closely, the return type already guards to not let a false pass which will automatically throw:

    public function testStoreFailed()
{
    $service = new TypeSaveLogService();

    $service->store('type-value', 'smId-value');

    $this->addToAssertionCount(1);
}

    You only test first if calling the method works. As little as necessary.

        public function store(string $type, string $smId): TypeSaveLogRecord
    {
        $log = new TypeSaveLogRecord([
            'mdm_id' => $smId,
            'type' => $type
        ]);

        return $log->save();
    }

    This allows you to finish the implementation of the method itself also as quickly as possible and with the least code necessary to have it pass.

    Now step back a bit from the monitor, also stand up. view both, test and implementation next to each other. do you spot any patterns? think about what you wanted to implement before you started, and in which code it did result afterwards. did you expect this? the test supports you to write the implementation and helps you to proof what not needs to be written.

    Now you can think about whether you can already improve the code, but without changing it (e.g. names, missing typehints, parameters that can be removed as effectively not in use etc.).

    Then add another test, and try to break that implementation but without changing it. You do not know yet what the error condition is. You only assumed there is one, but yet we have not even understood what constitutes an error, let alone the handling.

    The implementation already throws, that code was superfluous. Or in short: You have typed too much 😉 That’s implementing backwards, therefore you only had to build back. Happens to the best of us, so I’m glad you’ve been asking.

    Login or Signup to reply.
Please signup or login to give your own answer.

Back To Top
Search