In defence of Mocking Frameworks

Recently Frank de Jonge published a blog post on Testing without mocking frameworks, it's a good read and a well thought out post, but there are a few things I disagree with. This post will be my commentary, you might want to read Frank's post first.

I do have some skin in the game here, I'm one of the maintainers of Mockery, the mocking framework Frank uses in his examples. I maintain it because I use it a lot, but it doesn't provide any income etc, so my defence is purely theoretical. I won't lose my livelyhood if people stop using Mockery.

NB: When I use the word "mock" in this post, I usually use it in the vague way of describing any kind of test double, rather than the by the book definition.

I'm going to focus on the examples Frank uses, how I would see it and were I would go from there.

/**
 * @test
 */
public function testing_something_with_mocks(): void  
{
    $mock = Mockery::mock(ExternalDependency::class);

    $mock->shouldReceive('oldMethodName')
        ->once()
        ->with('some_argument')
        ->andReturn('some_response');

    $dependingCode = new DependingCode($mock);
    $result = $dependingCode->performOperation();

    $this->assertEquals('some_response', $result);
}

Frank argues that performing a rename method refactoring with his IDE, this test would be broken. I have no argument against this, when it happens, I fix the tests manually.

In more recent versions of Mockery, we can remove some of the stringiness, but I still don't think it would make the IDE's any more capable of refactoring without some sort of dyanmic return types for the Mock::allows and Mock::expects methods.

$mock->allows()
    ->oldMethodName('some_argument')
    ->andReturns('some_response');

It won't help much with the refactoring, but it looks a lot better and would give us some extra context when searching the codebase in that we can see it is a method call, rather than just a string matching the old name.

Frank then goes on to show a more concrete example, with an InvoiceService. His first iteration of the test looks like this:

use League\Flysystem\Filesystem;  
use PHPUnit\Framework\TestCase;

class TestInvoiceServiceWithMocksTest extends TestCase  
{
    /**
     * @test
     */
    public function invoicing_a_client(): void
    {
        // Need to work around marking this test as risky
        $this->expectNotToPerformAssertions();

        $mock = Mockery::mock(Filesystem::class);
        $mock->shouldReceive('write')
            ->once()
            ->with('/invoices/abc/2020/3.txt', '{"client_id":"abc","amount":42,"invoice_period":{"year":2020,"month":3}}');

        $invoicePeriod = InvoicePeriod::fromDateTime(DateTimeImmutable::createFromFormat('!Y-m', '2020-03'));
        $invoiceService = new InvoiceService($mock);
        $invoiceService->invoiceClient('abc', $invoicePeriod);
    }

    protected function tearDown(): void
    {
        Mockery::close();
    }
}

First thing I notice is that Frank isn't using Mockery's PHPUnit Integration. We ship a base MockeryTestCase, or you can use a trait. I've just created a PR to add a note to the readme about this.

Pulling in the integration makes the tearDown method and telling PHPUnit to ignore the lack of expectations redundant, so we can remove them.

use MockeryPHPUnitIntegration;

/**
 * @test
 */
public function invoicing_a_client(): void
{
    $mock = Mockery::mock(Filesystem::class);
    $mock->shouldReceive('write')
        ->once()
        ->with('/invoices/abc/2020/3.txt', '{"client_id":"abc","amount":42,"invoice_period":{"year":2020,"month":3}}');

    $invoicePeriod = InvoicePeriod::fromDateTime(DateTimeImmutable::createFromFormat('!Y-m', '2020-03'));
    $invoiceService = new InvoiceService($mock);
    $invoiceService->invoiceClient('abc', $invoicePeriod);
}

Now the PHPUnit awkwardness is out of the way, Frank notes three things:

Amount of mocking code vs other code.
High coupling between test and implementation.
The test does not describe desired behavior, it validates implementation.

Only the first of those items is specific to using mocking frameworks, the second two are possible regardless of mocking frameworks.

Frank's first step in improving this test is to replace the generated mock with a concrete one. This does give him some improved readability with regards to Arrange, Act, Assert, but that is achievable using a mocking framework too and is the first step I would have taken.

/**
 * @test
 */
public function invoicing_a_client(): void
{
    // Arrange
    $mock = spy(Filesystem::class);
    $invoiceService = new InvoiceService($mock);

    // Act
    $invoiceService->invoiceClient(
        'abc'
        InvoicePeriod::fromDateTime(DateTimeImmutable::createFromFormat('!Y-m', '2020-03'))
    );

    // Assert
    $mock->shouldHaveReceived()
        ->write('/invoices/abc/2020/3.txt', '{"client_id":"abc","amount":42,"invoice_period":{"year":2020,"month":3}}');
}

This looks very similar to Frank's first revision. I've replaced the mock with a spy, which allows me to verify that calls were made after the fact, rather than declaring them up front.

The big difference is Frank's test uses a concrete implementation of a FileSystem. Frank gains confidence from this, because he knows the InMemoryFilesystemAdapter runs against the same set of tests as the other more useful versions. I wouldn't have the same confidence as Frank, because I don't trust that library. I don't trust most of the libraries I use and I try not to mock types I don't trust. And by that I mean either mocks generated by a framework or concrete mocks created by hand.

Both my version and Frank's version still have that big horrible JSON string, the high coupling between the test and the implementation. Both versions still don't describe the desired behaviour, they describe what the SUT does and what the outcome is, with explicit detail. We both tackle this next, but in a different way.

The domain experts tell us the Invoice should be submitted to the InvoicePortal, so I'm going to update my test to describe that exactly. I'm also going to make sure the test name describes that.

/**
 * @test
 */
public function invoicing_a_client_submits_the_invoice_to_the_invoice_portal(): void
{
    // Arrange
    $invoicePortal = Mockery::mock(InvoicePortal::class);
    $invoiceService = new InvoiceService($invoicePortal);

    // Act
    $invoicePeriod = InvoicePeriod::fromDateTime(DateTimeImmutable::createFromFormat('!Y-m', '2020-03'))
    $invoiceService->invoiceClient('abc', $invoicePeriod);

    // Assert
    $expectedInvoice = new Invoice('abc', 42, $invoicePeriod);
    $invoicePortal->shouldHaveReceived()
        ->submitInvoice(equalTo($expectedInvoice));
}

At this stage, the InvoicePortal doesn't exist. The author's behind the GOOS book call this programming by wishful thinking. I know the InvoiceService needs to submit an Invoice to an InvoicePortal, so I'm going to describe that in my test and Mockery facilitates it for me. I then would update the SUT accordingly. The astute among you will notice that Mockery isn't really helping me test the SUT here and we will need more tests to actually ensure the system works. What Mockery is doing, is helping me design the SUT. It has perfectly described that I will need an InvoicePortal, and that thing will need a submitInvoice method that takes an Invoice.

I would now move in to the next layer of my application and start building the InvoicePortal, which will have it's own suite of tests, just like in Frank's example. One difference being, I have defined my meaningful boundaries already, by writing an executable example of the way the consumer, would like things to work. By writing the InvoicePortal first, Frank is defining the meaningful boundaries in the inner layer, and once he has built them, the outer layer gets to try them on for size.

In the outside-in methodology, the outer layers describe what is required from the inner layers, and then the programmer builds them. In the inside-out methodology, the programmer builds the inner layer which prescribes what is available and the outer layer works around it. With experienced programmers, the outcome will most often be the same, just a slightly different journey.

Another difference is that I wouldn't have bothered creating a FakeInvoicePortal. I already have a fake generated for me by Mockery and unless I'm going to get a lot of benefit and reuse, I don't take the time to write concrete fakes by default.

What did we achieve?

We still don't have the nice IDE refactoring as mentioned earlier, but I would argue that my example achieves the same other things Frank achieved with his refactoring, but with less effort and more preferable (to me) design guidance.

The tests no longer contain implementation details.

We have the same outcome here, there are no more or no less implementation details in either my test or Frank's. You could argue about the amount of coupling, Frank's are coupled to his manually crafted mock, mine are coupled to Mockery's generated mocks.

Clearer high-level code.

This is irrelevant with regards to the tests and therefore mocking frameworks or not, the code for the SUT is identical for both my way of work and Frank's.

Low-complexity fakes are easy to control.

I personally find Mockery's mocks very easy to control, but I have a bias there. Mockery ships with the ability to stage exceptions and fix responses and if you know how to use them, would save you time writing code. That said, it's far easier to understand and trust code you have written yourself, so I see where Frank is coming from. I personally trust Mockery enough to lean on it and save myself the time.

$invoicePortal = Mockery::mock(InvoicePortal::class);
$invoicePortal
    ->allows()
    ->submit(anyArgs())
    ->andThrows(new \Exception());

Fin.

What did we achieve?

Subscribe to news and updates