Testing chained method call in Mockery

Question

I'm trying to properly mock a chained call to an Eloquent model in a controller. In my controller I'm using dependancy injection to access the model so that it should be easy to mock, however I'm not sure how to test the chained calls and make it work right. This is all in Laravel 4.1 using PHPUnit and Mockery.

Controller:

<?php

class TextbooksController extends BaseController
{
    protected $textbook;

    public function __construct(Textbook $textbook)
    {
        $this->textbook = $textbook;
    }

    public function index()
    {
        $textbooks = $this->textbook->remember(5)
            ->with('user')
            ->notSold()
            ->take(25)
            ->orderBy('created_at', 'desc')
            ->get();

        return View::make('textbooks.index', compact('textbooks'));
    }
}

---

Controller test:

<?php

class TextbooksControllerText extends TestCase
{
    public function __construct()
    {
        $this->mock = Mockery::mock('Eloquent', 'Textbook');
    }

    public function tearDown()
    {
        Mockery::close();
    }

    public function testIndex()
    {
        // Here I want properly mock my chained call to the Textbook
        // model.

        $this->action('GET', 'TextbooksController@index');

        $this->assertResponseOk();
        $this->assertViewHas('textbooks');
    }
}

---

I've been trying to achieve this by placing this code before the $this->action() call in the test.

$this->mock->shouldReceive('remember')->with(5)->once();
$this->mock->shouldReceive('with')->with('user')->once();
$this->mock->shouldReceive('notSold')->once();
$this->app->instance('Textbook', $this->mock);

However, this results in the error Fatal error: Call to a member function with() on a non-object in /app/controllers/TextbooksController.php on line 28.

I've also tried a chained alternative hoping it would do the trick.

$this->mock->shouldReceive('remember')->with(5)->once()
    ->shouldReceive('with')->with('user')->once()
    ->shouldReceive('notSold')->once();
$this->app->instance('Textbook', $this->mock);

What is the best approach I should take to testing this chained method call with Mockery.

Caenogenesis

Please read the documentation github.com/padraic/… – Helfant

Answer 1

I'm quite new to testing myself, and this whole answer may be wrong in most people's eyes, but I do see a prevalence of people testing the wrong thing. If you test exactly everything a method does then you're not testing, but just writing a method twice.

You should think of your code as something of a black box - don't presume to know what's going on inside when you write your tests. Call a method with a given input, expect an output. Sometimes you need to ensure that certain other effects have happened, and that's when the shouldReceive stuff comes in. But again it's more high level than this collection chain testing - you should be testing that the code to do what this code does is done, but exactly that the code itself happens. As such, the collection chain should be extracted to some other method somehow, and you should simply test that that method is called.

The more you're testing the actual written code (rather than the purpose of the code) the more problems you will have. For example, if you need to update the code to do the same thing a different way (maybe remember(6) not remember(5) as part of that chain or something), you also have to update your test to ensure that remember(6) is now called, when you shouldn't be testing that at all.

This advice doesn't just go for chained methods of course, it's for any time you ensure that various objects have various methods called on them when testing a given method.

As much as I dislike the term 'red, green, refactor' you should consider it here as there are two points where your testing method is failing:

• Red/Green: When you first write the failing test, your code shouldn't have all these shouldReceives (maybe one or two if it makes sense to, see above) - if it does, then you're not writing a test but you're writing the code. And really, it's an indication that you wrote the code first then the test to fit the code, which is against test-first TDD.
• refactor: Assuming you have written the code first, then the test to fit the code (or hey somehow managed to guess exactly what shouldReceives to write in your test that the code just magically worked out). That's bad, but let's say you did it, as it's not the end of the world. You now need to refactor, but you can't without changing your test. You test is so closely coupled to the code, that any refactoring will break the test. That is, again, against the idea of TDD.

Even if you don't follow test-first TDD, you should at least realise that the refactor step should be doable without breaking your tests.

Anyway, that's just my tuppence.

Ackler

Also I'm aware this doesn't answer the question directly as asked, but I think it's a good answer to have as it answers to the broader point of your code and is helpful to the community at large. – Ackler

Yes, this is a great answer. Actually reading through it made me feel a little stupid because it seems much more obvious now; test that the end result is as expected and only test the higher levels of the code if they are critical to that process. I'll leave my answer below, as it does technically achieve the result I was initially looking for, but it is the wrong approach. – Caenogenesis

Together we answer both sides of the question :) – Ackler

Instead of directly using the models in your controller, use a repository class and check that the controller is calling it as it should: culttt.com/2013/07/08/… Then, the repository class should be tested with an integration test (rather than with a unit test, due to the reasons explained by @Ackler in his answer). – Halcyon

Answer 2

Originally a comment but moved to answer to make the code legible!

I lean toward @alexrussell's answer too, though a middle ground would be:

$this->mock->shouldReceive('remember->with->notSold->take->orderBy->get')
    ->andRe‌​turn($this->collection);

Iodine

this works, but i noticed that it causes code coverage to fail (in case you are using that) – Catto

I didn't know that, so thanks for pointing it out. Another reason not to get sucked into the innards of the unit being tested :) – Iodine

@Iodine 3 years later, but $this->mock isn't available. How to instantiate? – Prance

The OP sets up $this->mock in a constructor in his (slightly misnamed) TextbooksControllerTest, that's probably to avoid it being done repeatedly in a setUp method, though I'd personally still prefer the latter. – Iodine

Answer 3

I've discovered this technique, but I don't love it. It's very verbose. I think there must be a cleaner/simpler method to achieving this.

In the constructor:

$this->collection = Mockery::mock('Illuminate\Database\Eloquent\Collection')->shouldDeferMissing();

In the test:

$this->mock->shouldReceive('remember')->with(5)->andReturn($this->mock);
$this->mock->shouldReceive('with')->with('user')->andReturn($this->mock);
$this->mock->shouldReceive('notSold')->andReturn($this->mock);
$this->mock->shouldReceive('take')->with(25)->andReturn($this->mock);
$this->mock->shouldReceive('orderBy')->with('created_at', 'DESC')->andReturn($this->mock);
$this->mock->shouldReceive('get')->andReturn($this->collection);

Caenogenesis

Answer 4

Try like below

$this->mock
->shouldReceive('remember->with->notSold->take->orderBy->get')
->once()
->andReturn('Any desired data');

Hope this will work. Thanks.

Psoriasis

Answer 5

You can also pass the methods as arguments to mock object:

$this->mock
    ->shouldReceive('remember', 'with', 'notSold', 'take', 'get', 'orderBy')
    ->withAnyArgs()
    ->andReturnSelf();

Antagonism

Thats wrong answer. this is testing if every single of these methods are being executed instead of testing if methods are being executed in the chain – Vulcanize