1
\$\begingroup\$

I have a Responder class whose prepare method takes an entity, checks it with a validator, and returns a response or errorResponse based on the results of validator.

class CreatorResponder implements ResponderInterface
{
    private $validator;
    private $repositoryFactory;
    private $httpResponseFactory;

    public function __construct(
        ValidatorInterface $validator,
        RepositoryFactoryInterface $repositoryFactory,
        HttpResponseFactoryInterface $httpResponseFactory
    ){
        $this->validator = $validator;
        $this->repositoryFactory = $repositoryFactory;
        $this->httpResponseFactory = $httpResponseFactory;
    }

    public function prepare(AbstractEntity $entity) : SimpleHttpResponseInterface
    {
        $validatorResponse = $this->validator->validate($entity);
        if ($validatorResponse->isValid()) {

            $repository = $this->repositoryFactory->create($entity);
            $repository->persist($entity);

            return $this->getSuccessResponse($entity);
        }
        return $this->getFailureResponse($validatorResponse->getErrors());
    }

    protected function getSuccessResponse(AbstractEntity $entity): SimpleHttpResponseInterface
    {
        $response = $this->httpResponseFactory->create(200, $entity);
        return $response;
    }

    protected function getFailureResponse(array $errors): SimpleHttpResponseInterface
    {
        $response = $this->httpResponseFactory->create(422, $errors);
        return $response;
    }
}

The code is pretty simple, but its unit test is just pain in the ass. It is too big, hard to read, hard to maintain. That is because of mocking. Even repositoryFactory class is mocked, and it return a repository that is again mocked, so a mock is returning a mock.

class CreatorResponderTest extends ResponderTestBase
{
    public function testPrepare_withValid() : void
    {
        $validator = $this->getValidator(
            true,
            [$this->responseContent]
        );
        $repositoryFactory = $this->getRepositoryFactory();

        $entity = $this->getEntity();

        $httpResponseFactory = $this->getHttpResponseFactory(
            $this->validStatusCode,
            $this->responseContent
        );

        $responder = new CreatorResponder($validator, $repositoryFactory, $httpResponseFactory);
        $response = $responder->prepare($entity);

        $this->assertInstanceOf(SimpleHttpResponseInterface::class, $response);
        $this->assertEquals($this->validStatusCode, $response->getStatusCode());
        $this->assertEquals(
            $this->responseContent,
            $response->getContent()
        );
    }


    public function testPrepare_withInvalid() : void
    {
        $validator = $this->getValidator(
            false,
            [$this->responseContent]
        );
        $repositoryFactory = $this->getRepositoryFactory();
        $entity = $this->getEntity();

        $httpResponseFactory = $this->getHttpResponseFactory(
            $this->invalidStatusCode,
            $this->responseContent
        );

        $responder = new CreatorResponder($validator, $repositoryFactory, $httpResponseFactory);
        $response = $responder->prepare($entity);

        $this->assertInstanceOf(SimpleHttpResponseInterface::class, $response);
        $this->assertEquals($this->invalidStatusCode, $response->getStatusCode());
        $this->assertEquals(
            $this->responseContent,
            $response->getContent()
        );
    }
}

The base class of tests actually creates all these mocks.

class ResponderTestBase extends TestCase
{
    protected $responseContent = 'some random content string!';
    protected $validStatusCode = 200;
    protected $invalidStatusCode = 422;

    protected function getValidator(bool $isValid, $content)
    {
        $validatorWithValidResponseStub = $this->createMock(
            ValidatorInterface::class
        );

        $validatorResponse = new ValidatorResponse($isValid, $content);

        $validatorWithValidResponseStub->method('validate')
            ->willReturn($validatorResponse);

        return $validatorWithValidResponseStub;

    }

    protected function getEntity()
    {
        $entityStub = $this->createMock(
            AbstractEntity::class
        );

        $entityStub->method('getId')
            ->willReturn(1);

        return $entityStub;
    }

    protected function getRepositoryFactory()
    {
        $repositoryMock = $this->createMock(
            RepositoryFactoryInterface::class
        );

        $repositoryMock->expects($this->any())
            ->method('persist')
            ->with($this->isInstanceOf(AbstractEntity::class));

        return $repositoryMock;
    }

    protected function getRepository()
    {
        $repositoryMock = $this->createMock(
            RepositoryInterface::class
        );

        $repositoryMock->expects($this->any())
            ->method('persist')
            ->with($this->isInstanceOf(AbstractEntity::class));

        return $repositoryMock;
    }

    protected function getHttpResponseFactory(int $httpStatusCode, $content)
    {
        $httpResponseFactoryStub = $this->createMock(
            HttpResponseFactoryInterface::class
        );

        if (!is_string($content)) {
            $content = json_encode($content);
        }

        $httpResponseFactoryStub->method('create')
            ->willReturn($this->getSimpleHttpResponse($httpStatusCode, $content));

        return $httpResponseFactoryStub;
    }

    protected function getSimpleHttpResponse(int $httpStatusCode, $content)
    {
        return new SimpleHttpResponse($httpStatusCode, $content);
    }
}

This whole problem is all because of mocking. Everytime I touch something in the class, I have to come to test and find the thing that disturbed it and it takes a lot more time to deal with this test. I think if I just don't mock repositoryFactory and httpResponseFactory here, a lot of my problem will be solved. I will not have mocks returning mocks. My unit tests will get more focused on behaviour instead of implementation. But I will lose isolation and it will turn into an integrated test. What is your take on that and how can I improve it?

I also tried not mocking repositoryFactory, but to create its instance, I will to bring in EntityManager that goes in repositoryFactory constructor. Again, this will need either mocking or accessing service container. So, not mocking is not good either. I think my design is bad. What's better?

\$\endgroup\$
  • \$\begingroup\$ The convention for Code Review is for question titles to state what the code accomplishes, rather than what your primary concern is about the code. See How to Ask. \$\endgroup\$ – 200_success Jun 10 at 19:17
3
\$\begingroup\$

The thing is, there is not much to test in your code. You have added several abstractions to a simple statement:

if (isValid($data)) {
    doSomething($data);
    return makeResponse(200);
} else {
    return makeResponse(422);
}

So, your unit is a single if statement, and all actions -- condition, then branch, else branch -- everything is passed as parameter. There is literally nothing to test.

Still, in attempt to test that if statement, you need to execute it somehow, and thus you need to prepare all these mocks etc. to just make that if work.

Basically, I'm in the same boat as you because the level of abstraction you have demonstrated is what I usually do. It always ends badly...

I have no "cure" for this ATM, but I think starting with famous Mocking is a Code Smell would be nice.

\$\endgroup\$
0
\$\begingroup\$

I ended up writing a builder for the test. The builder made the unit test a lot more readable and maintainable. I am not exactly where I wanted to get, but I am close. Find below the the test with builder.

class CreatorResponderBuilder extends TestCase
{
    private $validator;
    private $repositoryFactory;
    private $httpResponseFactory;

    public static function getInstance(): CreatorResponderBuilder
    {
        return new self();
    }

    public function __construct()
    {
        $this->validator = $this->createMock(ValidatorInterface::class);
        $this->repositoryFactory = $this->createMock(
            RepositoryFactoryInterface::class
        );
        $this->httpResponseFactory = $this->createMock(
            HttpResponseFactoryInterface::class
        );


    }

    public function withValidValidatorResponse() : self
    {
        $validatorResponse = new ValidatorResponse(true, []);

        $this->validator->method('validate')
            ->willReturn($validatorResponse);

        $this->withHttpResponseFactoryForValidResponse();

        return $this;
    }

    private function withHttpResponseFactoryForValidResponse(): void
    {
        $this->httpResponseFactory->method('create')
            ->willReturn(new SimpleHttpResponse(200, 'Test content.'));

    }

    public function withInvalidValidatorResponse(): self
    {
        $validatorResponse = new ValidatorResponse(false, ['An error message.']);

        $this->validator->method('validate')
            ->willReturn($validatorResponse);

        $this->withHttpResponseFactoryForInvalidResponse();

        return $this;
    }

    private function withHttpResponseFactoryForInvalidResponse(): void
    {
        $this->httpResponseFactory->method('create')
            ->willReturn(new SimpleHttpResponse(422, 'An error message.'));
    }

    public function withRepositoryFactory() : self
    {
        $this->repositoryFactory->expects($this->any())
            ->method('create')
            ->with($this->isInstanceOf(AbstractEntity::class))
            ->willReturn(
                $this->createMock(RepositoryInterface::class)
            );

        return $this;
    }

    public function build() : CreatorResponder
    {
        $this->withValidValidatorResponse()
            ->withRepositoryFactory();

        return new CreatorResponder(
            $this->validator,
            $this->repositoryFactory,
            $this->httpResponseFactory
        );
    }
}

And the test:

class CreatorResponderTest extends TestCase
{
    public function testPrepareForValidEntity(): void
    {
        $creatorResponder = CreatorResponderBuilder::getInstance()
            ->withValidValidatorResponse()
            ->build();

        $response = $creatorResponder->prepare(
            $this->createMock(AbstractEntity::class)
        );

        $this->assertInstanceOf(SimpleHttpResponseInterface::class, $response);
        $this->assertEquals(200, $response->getStatusCode());
        $this->assertEquals('Test content.', $response->getContent());
    }

    public function testPrepareForInvalidEntity(): void
    {
        $creatorResponder = CreatorResponderBuilder::getInstance()
            ->withInvalidValidatorResponse()
            ->build();

        $response = $creatorResponder->prepare(
            $this->createMock(AbstractEntity::class)
        );

        $this->assertEquals(422, $response->getStatusCode());
        $this->assertEquals('An error message.', $response->getContent());
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.