jrdavidson's avatar

Mocking method test returns no expectations

I'm trying to figure out why my test is saying that Received Mockery_0_App_Models_Player::currentInjury(), but no expectations were specified. I don't understand why this is because it shouldn't even be running the clearInjury method in the test it should just be returning from it.

public function an_injured_player_can_be_cleared_from_an_injury_without_a_date_passed_in()
{
    $recoveryDate = null;
    $playerMock = $this->mock(Player::class);
    $repositoryMock = $this->mock(PlayerRepository::class);
    $strategy = new PlayerClearInjuryStrategy($repositoryMock);

    $playerMock->expects()->canBeClearedFromInjury()->once()->andReturns(true);
    $repositoryMock->expects()->clearInjury($playerMock, $recoveryDate)->once()->andReturns($playerMock);

    $strategy->setInjurable($playerMock)->clearInjury($recoveryDate);
}

Action Class

public function clearInjury(string $recoveryDate = null)
{
    throw_unless($this->injurable->canBeClearedFromInjury(), new CannotBeClearedFromInjuryException);

    $recoveryDate ??= now()->toDateTimeString();

    $this->playerRepository->clearInjury($this->injurable, $recoveryDate);
    $this->injurable->updateStatusAndSave();
}

PlayerRepository

public function clearInjury(Player $player, string $recoveryDate)
{
    return $player->currentInjury()->update(['ended_at' => $recoveryDate]);
}
0 likes
8 replies
martinbean's avatar

@jrdavidson It’s because you’re mocking too much. You’re mocking the repository, but then trying to mock the repository’s implementation itself. You don’t need to. If the repository is mocked, then nothing in it is going to get called, so you don’t need to mock any of the classes it uses, like the model.

This is why you’re getting the “no expectations were specified” error. The player model mock never gets invoked, because the class that calls it (the repository) is mocked itself.

Mock the repository and then just mock any methods you expect to be called on that repository.

$repository = $this->mock(PlayerRepository::class);
$repository->shouldReceive('clearInjury')->once()->with($player, $recoveryDate)->andReturn($player);

// Call repository; mocked method will be invoked instead
$repository->clearInjury($player, $recoverDate);
1 like
jrdavidson's avatar

@martinbean That will be great when I'm trying to mock the actual repository method itself but my intention was to mock the the Action Class clearInjury method there. I know it might have been confusing because of both methods having the same name. That's something I will be changing but I want to go ahead and figure out how I can mock the action class past the throw_unless.

martinbean's avatar

@jrdavidson So what are you actually trying to test? The model? The repository? The action class? Because you seem almost as confused as I am right now.

martinbean's avatar
martinbean
4 years ago
Best Answer
Level 80

I'm trying to test the action class which is referenced as $strategy in the test.

@jrdavidson So test that? If a repository class is injected into your strategy class, then that’s all you need to mock:

$player = new Player();

$repository = $this->mock(PlayerRepository::class, function ($mock) use ($player) {
    $mock->shouldReceive()->once()->with($player, null)->andReturn($player);
});

$strategy = new PlayerClearInjuryStrategy($repository);

$strategy->setInjurable($player)->clearInjury();

But yeah, as @bugsysha says, this seems overkill just for some code that essentially sets a status on a model.

1 like
bugsysha's avatar

This is a great example where things got overengineered and that made code ugly and complex to test.

jrdavidson's avatar

@bugsysha I don't disagree but would appreciate any further ideas that could make this better testable.

bugsysha's avatar
  1. Your strategy variable is not a strategy pattern. So I'm not sure why are you calling it a strategy.
  2. Get rid of the Repository pattern since you are not using it as it should be used.
  3. Simplify code so that the responsibilities are clear.
  4. Your test is very brittle.

To explain all these points, we would have to have a long call.

1 like

Please or to participate in this conversation.