Why PHPSpec is stepping into mocked/stubed method?

Question

I'm starting with PHPSpec and I've encounter some problems with it. One of them is that I keep getting

method call: - find(null) on Double\TaskRepositoryInterface\TaskRepositoryInterface\P95 was not expected, expected calls were: - findOneByGoogleId(exact("googleId")) I don't know why phpspec is expecting findOneByGoogleId there.

This is my spec example:

public function it_should_synchronize_new_task_from_google_to_app(
    TaskDTO $taskDTO,
    TaskDTO $taskDTO2,
    TaskListDTO $taskListDTO,
    TaskRepositoryInterface $taskRepository,
    TaskListRepositoryInterface $taskListRepository,
    TaskList $taskList,
    AddTask $addTask
){
    $taskDTO->getGoogleId()->willReturn('googleId');
    $taskRepository->findOneByGoogleId('googleId')->willReturn(null);
    $taskDTO->getTitle()->willReturn('GoogleTitle');
    $taskListDTO->getId()->willReturn(1);
    $taskListRepository->find(1)->willReturn($taskList);
  //$addTask->toTaskList(Prophecy\Argument::type(TaskDTO::class),Prophecy\Argument::type(TaskListDTO::class))->shouldBeCalled();
  //$addTask->toTaskList(Prophecy\Argument::type(TaskDTO::class),Prophecy\Argument::type(TaskListDTO::class))->shouldBeCalled()->willReturn($taskDTO2);
    $addTask->toTaskList(Prophecy\Argument::type(TaskDTO::class),Prophecy\Argument::type(TaskListDTO::class))->willReturn($taskDTO2);
    $this->fromGoogleToApp($taskDTO, $taskListDTO)->shouldReturnAnInstanceOf('Itmore\Core\Entity\TaskDTO');
}

This is SUS:

public function fromGoogleToApp(TaskDTO $taskDTO, TaskListDTO $taskListDTO)
{
    if(!$taskDTO->getGoogleId()){
        throw new \ErrorException('not a google task');
    }
    $task = $this->taskRepository->findOneByGoogleId($taskDTO->getGoogleId());
    if(!$task){
        $task = new Task();
        $task->setTitle($taskDTO->getTitle());
        $taskList = $this->taskListRepository->find($taskListDTO->getId());
        $task->setTaskList($taskList);
        $addTask = new AddTask($this->taskRepository, $this->taskListRepository,$this->userRepository);

        try {
            $addTask->toTaskList(new TaskDTO($task), $taskListDTO);
        } catch (\ErrorException $e) {}

        return new TaskDTO($task);

    }else {
        if ($task->getTitle() != $taskDTO->getTitle()) {
            $task->setTitle($taskDTO->getTitle());
        }
        $this->taskRepository->update();
    }

    return new TaskDTO($task);
}

And this is method PHPSpec is stepping in:

public function toTaskList(TaskDTO $taskDTO, TaskListDTO $taskListDTO)
{
    $taskList = $this->taskListRepository->find($taskListDTO->getId());
    if (!$taskList) {
        throw new \ErrorException('taskList not found');
    }

    $task = $this->taskRepository->find($taskDTO->getId());
    if ($task) {
        throw new \ErrorException('task already exists');
    }

    $task = new Task();
    $task->setTitle($taskDTO->getTitle());
    $task->setGoogleId($taskDTO->getGoogleId());
    $task->setTaskList($taskList);

    $this->taskRepository->add($task);

    return new TaskDTO($task);
}

As you can see I tried different ways to stub/mock this toTaskList method but it doesn't seems to work. When I comment out call to toTaskList test passes. I don't really understand why this is happening.

Answer 1

You are calling $taskListDTO->getId() twice, but only stubbing it once.

I would refactor your SUS to something like this:

public function fromGoogleToApp(TaskDTO $taskDTO, TaskListDTO $taskListDTO)
{
    if(!$taskDTO->getGoogleId()){
        throw new \ErrorException('not a google task');
    }
    $task = $this->taskRepository->findOneByGoogleId($taskDTO->getGoogleId());

    if(!$task){
        $taskList = $this->taskListRepository->find($taskListDTO->getId());

        $task = $this->taskManager->createNew()
            ->setTitle($taskDTO->getTitle())
            ->setTaskList($taskList);

        $addTask = $this->addTaskManager->createNew($this->taskRepository, $this->taskListRepository,$this->userRepository);

        $newTaskDTO = $this->taskDTOManager->createNew($task);

        try {
            $addTask->toTaskList($newTaskDTO, $taskListDTO);
        } catch (\ErrorException $e) {}

        return $newTaskDTO;

    }else {
        if ($task->getTitle() != $taskDTO->getTitle()) {
            $task->setTitle($taskDTO->getTitle());
        }
        $this->taskRepository->update();
    }

    return $this->taskDTOManager->createNew($task);;
}

This makes use of DI and you can stub/mock the calls.

While you spec it you will probably notice that you need to mock and stub quite a few things, that is normally a hint that the method is doing too many things and may be a good idea to delegate some tasks to other classes.