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.
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)
notremember(5)
as part of that chain or something), you also have to update your test to ensure thatremember(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:
shouldReceive
s (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.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.
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:
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:
In the test: