Correct unit testing

Question

I started using unit and functionality test with this project and because of this I have some questions:

Im working with the symfony php framework. And I have a doctrine like LDAP ORM service.

Furthermore I have a user repository (as a service) which depends on the LDAP ORM service, the logger and a validation service.

Now I want to write a unit test for the addUser function of the UserRepo. It will internally call: getNewUidNumber, userToEntities, doesUserExist and getUserByUid.

My question is: Should I mock all these internal function to just test the addUser function? Would that be against the unit test idea (just test the API).

Or should I just mock the LDAP ORM service, the Logger, and the validation service, so that the class calls all internal functions? But this would cause a huge test function with a lot of mocking because I have to mock the repositories for all internal repositories calls.

Or should I start the symfony kernel and use the ServiceContainer to use the ORM LDAP service with a real test database. But wouldn't that be a functionally test and not a unit test? I heard that its bad to have so many dependencies in a test. So I thought it would be bad to use the whole serviceContainer.

Adduser:

public function addUser(User $user)
{
    $pbnlAccount = $this->userToEntities($user);

    if(!$this->doesUserExist($user)) {
        $pbnlAccount->setUidNumber($this->getNewUidNumber());
        $this->ldapEntityManager->persist($pbnlAccount);
        $this->ldapEntityManager->flush();
    }
    else {
        throw new UserAlreadyExistException("The user ".$user->getUid()." already exists.");
    }

    return $this->getUserByUid($user->getUid());
}

For more code, like the internal functions: https://gist.github.com/NKPmedia/4a6ee55b6bb96e8af409debd98950678

Thanks Paul

Answer 1

First, I would like to rewrite the method a tiny bit, if I may.

public function addUser(User $user)
{
    if ($this->doesUserExist($user)) {
        throw new UserAlreadyExistException("The user ".$user->getUid()." already exists.");
    }

    // ... shortened for brevity
    $pbnlAccount = $this->userToEntities($user);
    $this->ldapEntityManager->persist($pbnlAccount);
}

The other relevant method is:

private function doesUserExist(User $user)
{
    $users = $this->ldapRepository->findByUid($user->getUid());
    return count($users) === 1;
}

Immediately we can see that we basically have two tests:

• We test that the method throws when the user exists
• We test that the method persists a PbnlAccount if the user does not exist.

If you do not see why we have these two tests, note that there are 2 possible "flows" in this method: one where the block inside the if statement is executed, and one where it is not executed.

Lets tackle the first one:

public function testAddUserThrowsWhenUserExistsAlready()
{
    $user = new User();
    $user->setUid('123');

    $ldapRepositoryMock = $this->createMock(LdapRepository::class);
    $ldapRepositoryMock
        ->method('findByUid')
        ->expects($this->once())
        ->with('123')
        ->willReturn(new PbnlAccount());

    $userRepository = new UserRepository($ldapRepositoryMock);

    $this->expectException(UserAlreadyExistException::class);
    $userRepository->addUser($user);    
}

The second test is left as an exercise for the reader :)

Yes you will have to do some mocking in your case. You wil need to mock the LdapRepository and LdapEntityManager both in this case.

Note 1: this code probably is not runnable, since I do not know the exact details of your code base (and I wrote this off the top of my head), but that is beside the point. The point is that you want to test for the exception.

Note 2: I would rename your function to createNewPbnlAccountForUser(User $user) which is longer, but more descriptive of what it actually does.

Note 3: I am not sure why you are returning $this->getUserByUid() since that seems redundant (you already have the User right there), so I am ommitting that case.

Answer 2

You need to mock ldapEntityManager and all repository services but not the internal function.And as you said, don't boot kernel in unit test. So, you should test all cases with success and throwing exception (make sure to check all behaviour)