Mocking inherited class where new object is created or how to unsmell my class

Question

Sorry for the messy title, I am trying to add Unit Tests to my (probably) smelly codebase. I have the something like the following class:

namespace App\Service;

use App\Messages\ItemMessage;
use App\Messages\LocationMessage;
use App\Messages\Message;
use App\Settings\Settings;

class MassMessenger
{
    protected static function sendMessagesForDay (int $day, string $content, string $title, Message $messageType): void
    {
        $posts = \App\Repository\Post::getPostForDay($day);
        foreach ($posts as $post) {
            $message = new $messageType($post->getId(), $content, $title);
            $message->triggerMail();
        }
    }

    public static function sendItem (int $day, Settings $settings): void
    {
        $messageType = new ItemMessage(0, '', '');
        self::sendMessagesForDay($day, $settings->getEmailContent(), $settings->getEmailTitle(), $messageType );
    }

    public static function sendLocation (int $day, Settings $settings): void
    {
        $messageType = new LocationMessage(0, '', '');
        self::sendMessagesForDay($day, $settings->getEmailContent(), $settings->getEmailTitle(), $messageType );
    }
}

The Message class is an abstract class that LocationMessage and ItemMessage inherit from. I want to test the public methods, I do not care too much for mocking the Repository so it is not a completely clean unit test. I also, however do not want to create an integration test where I ask the PHPMailer if the message went through.

So now I am in kind of a pickle on how to design these tests, I tried my hand at this:

<?php

namespace Tests\Service;

use App\Messages\ItemMessage;
use App\Service\MassMessenger;
use App\Settings\Settings;
use PHPUnit\Framework\TestCase;

class MassMessengerTest extends TestCase
{
    public function testSendItem()
    {
        $day = 1;
        $settings = $this->createMock(Settings::class);
        $settings->method('getEmailContent')->willReturn('Test Content');
        $settings->method('getEmailTitle')->willReturn('Test Title');

        $messageMock = \Mockery::mock( ItemMessage::class);
        $messageMock->shouldReceive('triggerMail')->once();
        MassMessenger::sendItem($day, $settings);

        // The test will fail if triggerMail is not called
    }


    protected function tearDown(): void
    {
        parent::tearDown();
        \Mockery::close();
    }
}

it (of course) does not detect the mock, probably because I am doing nothing with it

Mockery\Exception\InvalidCountException : Method triggerMail(<Any Arguments>) from Mockery_0_App_Messages_ItemMessage should be called

and when I overload the class (like mocking a hard dependency) I get this error: TypeError : App\Service\MassMessenger::sendMessagesForDay(): Argument #4 ($messageType) must be of type App\Messages\Message, App\Messages\ItemMessage given, called in /xxx/src/Service/MassMessenger.php on line 24

Also, I am open to hear any suggestions how I could unsmell my code, I am still learning software design patterns but haven't found the right pattern to create code that is well testable here.

I have also tried to directly test the sendMessagesForDay function by mocking the Message dependency which created worked in isolation but with the other tests but this failed with

Mockery\Exception\RuntimeException: Could not load mock App\Messages\Message, class already exists

and adding

* @runInSeparateProcess
* @preserveGlobalState disabled

just made the test fail silently. I am thinking that I have these issues because sendMessage creates a new instance of the mocked object. And since it was already so hard to test this I think I might be better off refactoring the class to make it testable better.

Any advice is highly appreciated, thank you!

Answer 1

To "unsmell" and make the class unit testable, you should be using dependency injection for the required classes rather than calling static methods on them. I don't know whether it would make more sense in your application to inject the dependencies when creating the MassMessenger class, or inject them to each of the static methods.

The first example could look like (this is untested)

class MassMessenger
{
    protected $post;
    protected $itemMessage;
    protected $locationMessage;
    public function __construct($post, $itemMessage, $locationMessage = null)
    {
        $this->post = $post;
        $this->itemMessage = $itemMessage;
        $this->locationMessage = $locationMessage;
    }
 
    protected function sendMessagesForDay (int $day, string $content, string $title, Message $message): void
    {
        $posts = $post->getPostForDay($day);
        foreach ($posts as $post) {
            $message->triggerMail($post->getId(), $content, $title);
        }
    }

    public function sendItem (int $day, Settings $settings): void
    {
        $this->sendMessagesForDay($day, $settings->getEmailContent(), $settings->getEmailTitle(), $this->itemMessage );
    }

    public function sendLocation (int $day, Settings $settings): void
    {
        $this->sendMessagesForDay($day, $settings->getEmailContent(), $settings->getEmailTitle(), $this->locationMessage );
    }
}

Then your test could look like

class MassMessengerTest extends TestCase
{
    public function testSendItem()
    {
        $day = 1;
        $settings = $this->createMock(Settings::class);
        $settings->method('getEmailContent')->willReturn('Test Content');
        $settings->method('getEmailTitle')->willReturn('Test Title');

        $messageMock = \Mockery::mock( ItemMessage::class);
        $messageMock->shouldReceive('triggerMail')->once();
        $messenger = new MassMessenger($post, $messageMock);
        $messenger->sendItem($day, $settings);

        // The test will fail if triggerMail is not called
    }


    protected function tearDown(): void
    {
        parent::tearDown();
        \Mockery::close();
    }
}

In general

• calling static methods in functions makes them harder to test
• instantiating objects in functions makes them harder to test