Do not mock what you do not own

Do not mock what you do not own

It sounds simple: test units of code without their real dependencies, use stubs or mocks instead. But if you don't control the replaced dependencies, things can quickly get out of control.

Web applications typically process HTTP requests. Commonly, objects are used to encapsulate request data. Depending on the framework, we may have an interface such as

1 2 3 4 5 6
interface   HttpRequest
{
     public   function   get ( string   $name ) :   string ;

     // ...
}

or even a concrete class such as

1 2 3 4 5 6 7 8 9
class   HttpRequest
{
     public   function   get ( string   $name ) :   string
     {
         // ...
     }

     // ...
}

that we can (and should) use to access request data.

Symfony, for instance, has Request::get(). For the sake of example, we will not worry about which type of HTTP request we process (GET, POST, or another one).

Let us instead focus on implicit APIs such as HttpRequest::get() and the problems they create.

When we need to look at request data, for instance in a controller, then we need to use the same get() method for the different information we need to query. There is no specific method with an explicit name for the individual piece of request data. Instead, the name is only passed as a string argument to the generic get() method:

1 2 3 4 5 6 7 8 9 10 11
class   SomeController
{
     public   function   execute ( HttpRequest   $request ) :   HttpResponse
     {
         $id       =   $request -> get ( 'id' ) ;
         $amount   =   $request -> get ( 'amount' ) ;
         $price    =   $request -> get ( 'price' ) ;

         // ...
     }
}

We will not argue about whether a controller should have one action method, or multiple ones (hint: it should have just one). The point here is that the controller needs to retrieve and process data from an HTTP request.

When we replace the HttpRequest object with a test stub or mock object to test SomeController in isolation from the web and from the framework we use to abstract from the web then we face the problem of multiple calls to the same method, get(), with different arguments that are just strings: 'id', 'amount', and 'price'.

We must ensure sensible return values for each call, otherwise the data will not pass validation, and we will not make it through the happy path of our controller action method.

For testing SomeController in isolation from the real HttpRequest object we can use a test stub in a unit test with PHPUnit like so:

1 2 3 4 5 6 7 8 9 10 11 12
$request   =   $this -> createStub ( HttpRequest :: class ) ;

$request -> method ( 'get' )
         -> willReturnOnConsecutiveCalls (
               '1' ,
               '2' ,
               '3' ,
           ) ;

$controller   =   new   SomeController ;

$controller -> execute ( $request ) ;

If we also want to verify the communication between the SomeController and HttpRequest object, then we need a mock object on which we have to configure expectations in our test:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18
$request   =   $this -> createMock ( HttpRequest :: class ) ;

$request -> expects ( $this -> exactly ( 3 ) )
         -> method ( 'get' )
         -> withConsecutive (
             [ 'id' ] ,
             [ 'amount' ] ,
             [ 'price' ]
         )
         -> willReturnOnConsecutiveCalls (
             '1' ,
             '2' ,
             '3' ,
         ) ;

$controller   =   new   SomeController ;

$controller -> execute ( $request ) ;

The code shown above is a bit difficult to read, which is a smell.

However, we express that HttpRequest::get() must be called three times: first with the argument 'id', then with 'amount', and finally with 'price'.

If we change the implementation of SomeController::execute() to perform the calls to HttpRequest::get() in a different order, our test will fail. This tells us that we have coupled our test code too tightly to the production code. This is another smell.

The real problem is that we query the HTTP request using an implicit API, where we pass a string argument specifying the name of an HTTP parameter to a generic get() method. And to make matters worse, we mock a type that we do not own: HttpRequest is provided by the framework and not under our control.

The wisdom "do not mock what you do not own" has its origin in the "London School of Test-Driven Development" community. As Steve Freeman and Nat Pryce wrote 2009 in "Growing Object-Oriented Software Guided by Tests":

We find that tests that mock external libraries often need to be complex to get the code into the right state for the functionality we need to exercise. The mess in such tests is telling us that the design isn't right but, instead of fixing the problem by improving the code, we have to carry the extra complexity in both code and test.

If we should not mock what we do not own, then how should we isolate our code from third-party code? Steve Freeman and Nat Pryce continued:

We [...] design interfaces for the services our objects need – which will be defined in terms of our objects' domain, not the external library. We write a layer of adapter objects [...] that uses the third-party API to implement these interfaces [...]

Let us do exactly that:

1 2 3 4 5 6 7 8
interface   SomeRequestInterface
{
     public   function   getId ( ) :   string ;

     public   function   getAmount ( ) :   string ;

     public   function   getPrice ( ) :   string ;
}

Rather than just returning string, we could now use even more specific types, or even value objects. For the purposes of this example, we will stick to string, however.

Creating a test double for SomeRequestInterface is easy and straightforward:

1 2 3 4 5 6 7 8 9 10
$request   =   $this -> createStub ( SomeRequestInterface :: class ) ;

$request -> method ( 'getId' )
         -> willReturn ( 1 ) ;

$request -> method ( 'getAmount' )
         -> willReturn ( 2 ) ;

$request -> method ( 'getPrice' )
         -> willReturn ( 3 ) ;

From a framework's point of view, a generic HTTP request object is the right abstraction, because it is the framework's job to represent the incoming HTTP request.

This should not stop us from doing the right thing, though. We can map the framework's generic HTTP request object to our specific request object. We do not even need a separate mapper. We can just wrap the generic request:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24
class   SomeRequest   implements   SomeRequestInterface
{
     private   HttpRequest   $request ;

     public   function   __construct ( HttpRequest   $request )
     {
         $this -> request   =   $request ;
     }

     public   function   getId ( ) :   string
     {
         return   $this -> request -> get ( 'id' ) ;
     }

     public   function   getAmount ( ) :   string
     {
         return   $this -> request -> get ( 'amount' ) ;
     }

     public   function   getPrice ( ) :   string
     {
         return   $this -> request -> get ( 'price' ) ;
     }
}

This is how we make things work together:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24
class   SomeController
{
     private   SomeHandler   $handler ;

     public   function   __construct ( SomeHandler   $handler )
     {
         $this -> handler   =   $handler ;
     }

     public   function   execute ( HttpRequest   $request )
     {
         return   $this -> handler -> process (
             new   SomeRequest ( $request )
         ) ;
     }
}

class   SomeHandler
{
     public   function   process ( SomeRequest   $request )
     {
         // ...
     }
}

Even if SomeController is a subclass of a Controller base class supplied by the framework, your actual code will stay independent of the framework's HTTP abstraction.

You will, of course, have to map information as needed, specific for each controller. Your code needs certain headers? Create a method to just get those. Your code needs an uploaded file? Create a method to retrieve just that.

A complete HTTP request can contain headers, values, maybe uploaded files, a POST body, etc. Configuring a test stub or mock object for all that, while you do not own the interface, keeps you from getting the job done. Defining your own interface makes things a lot easier.

Update: We improved the naming in the last example, based on feedback we had received via E-Mail and on Twitter. Thanks to everybody who got in touch.