chrisblackwell's avatar

Why is this Mock object not working in the controller?

I'm trying to make a unit test to test my controller. This keeps failing because the test is trying to call the method in the controller, rather then the Mock.

This is my test:

public function test_index_returns_all_contacts()
{
    $mock = m::mock(ContactService::class)->shouldReceive('getAllContacts')->once()->andReturn('testing');
    $response = $this->call('GET', 'contacts');
    $this->assertResponseOk();
}

and this is my controller index method

public function index()
{
    return $this->contact->getAllContacts();
}

If anyone has any idea what is wrong, I would greatly appreciate it.

0 likes
8 replies
bjones2015's avatar

Try mocking with the full path at least for me it never seems to work without the full path.

You'll probably need to use App::instance or $this->app->instance() (assuming you are using 5.1 with the built in testing) as you made the mock, but you didn't tell the framework to use it.

e.g below (this is from when I had to mock my billing gateway, but you get the idea)


$stripeGateWay = Mockery::mock('App\Contracts\BillingGatewayInterface');

        $stripeGateWay->shouldReceive('cancelSubscription')
            ->once()
            ->withArgs(['cus_123', 'sub_345'])
            ->andReturn();

        $this->app->instance('App\Contracts\BillingGatewayInterface', $stripeGateWay);

JeffreyWay's avatar

If you're writing a test for your controller, just hit the database.

2 likes
chrisblackwell's avatar

@JeffreyWay @bjones2015 The problem with hitting the database, is how many classes it goes through to get there. My controller calls my ContactService, which calls my ContactRepository, which then calls the model, to finally hit the database. My manager says he wants strict unit tests.

bjones2015's avatar

Well if he wants you to strictly compartmentalize everything then just revert back to my other answer and try something like this: (note - untested, obviously)

public function test_index_returns_all_contacts()
{
    $mock = m::mock(Path\To\ContactService::class)->shouldReceive('getAllContacts')->once()->andReturn('testing');
     App::instance('Path\To\ContractService', $mock);

    $response = $this->call('GET', 'contacts');
    $this->assertResponseOk();
}

You might already be doing it, but make sure to call Mockery::close() after each test.

chrisblackwell's avatar

@bjones2015 I got the index() and show() methods working, but the Mock doesn't want to work on the store() method

public function test_store_method_creates_and_returns_the_contact()
{
    $requestMock = m::mock(CreateContactRequest::class);
    $requestMock->shouldReceive('json->all')->andReturn(['name' => 'Foo Bar']);
    $this->app->instance(CreateContactRequest::class, $requestMock);

    $this->serviceMock->shouldReceive('create')->with($requestMock)
            ->once()->andReturn(Mockery::self());

    $response = $this->call('POST', 'contacts', [ 'name' => 'Foo Bar' ]);
    $this->assertResponseOk();
}

and the store method

public function store(CreateContactRequest $request)
{
    return $this->contact->create($request->json()->all());
}
bjones2015's avatar

TL;DR try this

$requestMock = m::mock(CreateContactRequest::class)->makePartial();

Quick Disclaimer: I'm still newish at testing so I may be completely off about the following

When you mock a class by default it assumes you will be providing expectations (shouldReceive) for everything that you expect to be called. In your case you are covering the case of it it being called with json()->all(), but you are not setting up the receives where the CreateContactRequest validates the form data it is passed.

So you'll either need to mock that out or use a passive partial mock. That way if a method is called that does not have an expectation set up it will revert to the using the concrete class method.

ifpingram's avatar

I am a bit confused about your code for the store method, and perhaps the reasons are why you are finding it difficult to make work. It seems that you are adding a lot of complexity that can be avoided.

Why would you have a specific CreateContactRequest class to essentially pass in a request object? Why not just have a Contact class, that you pass in a request object to, and then let the create() method work out what it needs to use? This way, you won't end up littering the system with lots of different CreateXXXXXRequest classes. It looks to me like you may have an unnecessary dependency here.

You say that your manager wants strict unit tests, but your store() test is more of a functional test. You also use an "and" in the naming, which indicates you are trying to test two things at once; one being the storing of data and one being the returning of data. This is a smell that you are trying to do too much at once - it's certainly no longer a unit test. Perhaps you should be looking at writing a few tests to drive out the functionality from different levels. Here's an example ideas:

  1. test of controller (outermost layer):
public function test_the_contacts_name_is_shown_upon_successful_creation()
{
    $request_mock = m::mock(Request::class);
    $request_mock->shouldReceive('json->all')->andReturn(['name' => 'Foo Bar']);
    $this->app->instance(Request::class, $request_mock);

    $create_contact = new CreateContact($request_mock);
    $this->app->instance(CreateContact::class, $create_contact);

    $this->call('POST', 'contacts', [ 'name' => 'Foo Bar' ]);
    $this->assertResponseOk();
    $this->assertContains('Foo Bar', $this->response->getContent());
}

this will fail at first (as we want), then it will help write the controller code something like:

public function store(CreateContact $create_contact)
{
    $create_contact->create($request->json()->all()); // Storing the data in one method

    return $create_contact->getName(); // Returning the output we want in another
}

which in turn will then help to delve in and drive out your inner unit tests of CreateContact->create() and getName():

class CreateContactTest
{
    use DatabaseMigrations; // to isolate each test with a migration to ensure the model's tables are empty to begin with

    // 1. unit test of CreateContact->create() method
    public function test_it_creates_the_contact_when_supplied_with_a_json_request_of_contact_details()
    {
        $request_mock = m::mock(Request::class);
        $request_mock->shouldReceive('json->all')->andReturn(['name' => 'Foo Bar']);
        $create_contact = new CreateContact(request_mock);

        $create_contact->create(); 

        $contacts = Contact::all(); //assuming using Eloquent, we pull in all the models

        $this->assertEquals('Foo Bar', $contacts[0]->name); // test first model to check name is as expected
        $this->assertCount(1, $contacts); // ensure that there is only one record; if there are more, we have a problem!
    }

    // 2. unit test of CreateContact->getName() method
    public function test_the_name_can_be_returned_after_the_contact_is_created()
    {
        $request_mock = m::mock(Request::class);
        $request_mock->shouldReceive('json->all')->andReturn(['name' => 'Foo Bar']);
        $create_contact = new CreateContact(request_mock);

    $create_contact->create(); 

        $this->assertEquals('Foo Bar', $create_contact->getName()); 
    }
}

I hope this gives you an idea about how to better organise the tests and the architecture. Please note, I have only written this in the message window and not tested it for syntax accuracy etc. as it's only to give an idea about where I would go with this.

Finally, the big glaring thing to me when I write the above code is that there is a CreateContact class with a create() method. This is tautological and looks to me like there should just be a Contact class with a create() method, which in turns looks to me like all you really want to do is to test that you are saving an Eloquent model (or other ORM model) after all of this? Perhaps there is some deeper architecture in the system that your posts don't allude to?

Good luck!

Please or to participate in this conversation.