How to unit test that object was passed with correct state?

Question

Lets say there is such function:

function a() 
{
    $entity = $this->getEntity();

    $entity->setSomePrivateVar();

    $service = $this->getService();

    $service->doSomething($entity);

}

I want to test that

 $service->doSomething($entity);

is called with correct $entity.

The $entity calls setSomePrivateVar()

In real application code I have done something like this:

Get mock of entity and test that setSomePrivateVar is called.

Get mock of $service and test that doSomething() is called with parameter $entity.

Looks ok.

But the problem is - if I refactor code and first call doSomething() on service and then setSomePrivateVar() on $entity, test still passes.

But the function is now wrong, because doSomething is depending on $entity private field which is set by setSomePrivateVar().

For example I would refactor to this:

function a() 
{
    $entity = $this->getEntity();

    $service = $this->getService();

    $service->doSomething($entity);

    // this line moved
    $entity->setSomePrivateVar();

}

So it looks like PhpUnit is not checking $entity private fields. If it was for example array, then with() function would see that array passed is not same as expected.

So how do I test that doSomething() gets $entity in correct state (that setSomePrivateVar() was called on entity before passing it to doSomething() )?

Maybe it has something to do that $entity is mocked.

Update with real world example

public function setNotifyUsers(AnnualConsolidation $consolidation, $status)
{
    $consolidation->setNotifyUsers($status);    // if move this method after the flush(), tesst does not fail

    $this->entityManager->persist($consolidation);
    $this->entityManager->flush();
}


public function testNotifyUsers()
{
    $consolidation = $this->getMockBuilder(AnnualConsolidation::class)
        ->setMethods(['setNotifyUsers'])
        ->getMock();

    $consolidation
        ->expects($this->once())
        ->method('setNotifyUsers')
    ;

    $this->entityManager
        ->expects($this->at(0))
        ->method('persist')
        ->with($consolidation)
    ;

    $this->entityManager
        ->expects($this->at(1))
        ->method('flush')
    ;

    /** @var AnnualConsolidation $consolidation */
    $this->consolidationsService->setNotifyUsers($consolidation, true);
}

We were discussing if testing the setNotifyUsers method this way is even good. I was trying to test without hitting the database. One guy thinks that this might be needed to test with hitting database, because if refactoring method without changing logic, test might be needed to refactor. On the other hand - this method is not likely to be refactored that much.

But maybe also there is a way to just test that flush() is called after persist() without telling indexes, because in other examples, the indexes then might be needed to update after adding some call before persist and so might be too much work to keep tests working.

But for this topic - first I want to know how to make test fail - if I move setNotifyUsers after the flush(). Test is not failing. While if we would test with hitting database - we would see that $consolidation status is not updated.

One guy told to check, assert what is passed to the persist method. I did not try yet, but I am not sure if on mocked $consolidation this will be possible. Does mocked $consolidation has some state as real $consolidation would have?

Answer 1

As you say in your question

One guy told to check, assert what is passed to the persist method.

That would be the way to go but your code makes that rather hard and i think you should refactor a bit to make the code testable.

First your method is called "setNotifyUsers" but it does actually do 2 actions it calls the setNotifyUsers on the consolidation object and it saves/persists this data. In my opinion those are 2 different actions that should belong in 2 different methods. It could help your test if you wrote it like :

public function setNotifyUsers(AnnualConsolidation $consolidation, $status) {
  $consolidation->setNotifyUsers($status);    
}

public function persistConsolidation(AnnualConsolidation $consolidation) {
  $this->entityManager->persist($consolidation);
  $this->entityManager->flush();
}

You could test the setNotifyUser and persistConsolidation sepperately and write a functional test for the part that calls these functions (the methods that use the consolidationsService) then you can use the at() functionality to see if those functions are called in the correct order.

But: Second you give both the state as the consolidation to this function with as only reason to add those together. i don't think something like that belongs in the service but rather in the method calling that service. Moving that functionality will again give you trouble as you cannot test the order in which they are called.

But you don't need to use the mockBuilder to make a test double. Instead of using $this->getMockBuilder you can also create a FakeConsolidation that will actually hold the data for you

Then you will also need a mock for the AnnualConsolidation because you want to be able to check if the value was correctly set.

class FakeConsolidation extends AnnualConsolidation {

  protected $id; 
  proteced $status;

  public function getId() {
    return $this->id;
  }

  public function setId($id) {
     $this->id = $id;
  }

  public function setNotifyUsers($status) {
    $this->status = $status;
  }

  public function shouldNotifyUsers() {
    $this->status
  } 
}

Now because you will give an object to the persist that has a state we can check that state in the "with" part.

Of course, I do not know exactly how your code is structured so I made some assumptions just adapt where needed and use the interfaces that you have.

Like this you could even test the code as you presented it in this question:

class SomethingTest extends PHPUnit_Framework_TestCase {
  private $consolidationsService;
  private $entityManager;

  /**
   * {@inheritdoc}
   */
  public function setUp() {
    $this->entityManager = $this->getMockBuilder(EntityManager::class)->getMock();
    $this->consolidationsService = new ConsolidationsService($this->entityManager);
  }

  public function testNotifyUsers() {
    $consolidation = new FakeConsolidation();
    $consolidation->setId(1);
    $this->entityManager
      ->expects($this->at(0))
      ->method('persist')
      ->with($this->callback(
          function($savedConsolidation) {
            return $savedConsolidation->shouldNotifyUsers() === true;
          }
      ));

    $this->entityManager
      ->expects($this->at(1))
      ->method('flush');

    /** @var AnnualConsolidation $consolidation */
    $this->consolidationsService->setNotifyUsers($consolidation, TRUE);
  }

}

Now when you move the setNotifyUsers below the persist

with($this->callback(
              function($savedConsolidation) {
                return $savedConsolidation->shouldNotifyUsers() === true;
              }
          )); 

Your test will fail because the status is not set yet.