1

I am writing some unit test for some legacy code as a preparation for some refactoring.

One section of the code is calling a class from a third-party library (I cannot change the code in it).

The code is using its constructor and calling some static variable.

May I ask how can I mock the method for testing? (or how do I test the method without mocking?)

I have tried to simplify my code for an example:

/////////////////////////////////////////////////////////////////////
// This is from the library code that I am unable to change

class ResponseMessage 
{
    const STATUS_ERROR = 500;
    const STATUS_SUCCESS = 200;

    function __construct( $responseMessage, $responseCode ) {
        $this->responseMessage = $responseMessage;
        $this->responseCode = $responseCode
    }
    
    public function sendMessage($sendToAdmin = false) {
        if ($sendToAdmin) {
            $targetEmailAddress = $this->emailAddresses['admin'];
            $targetAPISettings = $this->apiSettings['admin'];
        }
        else {
            $targetEmailAddress = $this->emailAddresses['customer'];
            $targetAPISettings = $this->apiSettings['customer'];
        }
        $this->triggerSendEmail($targetEmailAddress);
        $this->triggerAPI($targetAPISettings);
    }
    
    //.... Some other methods and variables
}
/////////////////////////////////////////////////////////////////////
// This is my class to be tested. If possible I would like to avoid
// changing any code, but if it is necessary this code can be changed
// a bit for injecting mock class.

class myClass 
{
    // ... some other methods where I have no problem testing....
    function handle($input) {
        //The actual comparison is much more complicated. Simplified for my question
        if (empty($input)) {
            // Some other error handling, no response message is generated
            return;
        }
        if ( $input == 1 )
            $responseMessage = new ResponseMessage('The input is 1', ResponseMessage::STATUS_SUCCESS );
        else if ( $input == 2 )
            $responseMessage = new ResponseMessage('The input is 2', ResponseMessage::STATUS_SUCCESS );
        else
            $responseMessage = new ResponseMessage('Unexpected input', ResponseMessage::STATUS_ERROR );
        
        $respnoseMessage->sendMessage();

        $responseMessageToAdmin = new ResponseMessage('The comaparison is triggered', RespnoseMessage::STATUS_SUCCESS);
        $responseMessageToAdmin->sendMessage(true);
    }
}
///////////////////////////////////////////////////////////////////
// This is my PHPUnit testing class

class testMyClass 
{
    public function testProcessWithInputOne {
        // how should I mock ResponseMessage here?
        /////////////////////////////////////////////////////////
        // These are not going to work, but tried to write some code to demo what I want to test.
        $mockResponseMessage = Mockery::mock('alias:ResponseMessage');//used alias for simplicity, but mocking it normally and inject the mocked object is also acceptable.
        $mockResponseMessage->setConst('STATUS_SUCCESS', 200);
        $mockResponseMessage->shouldReceive('__construct')->with('The input is 1', 200)->once()->andReturn($mockResponseMessage);
        $mockResponseMessage->shouldReceive('sendMessage')->with(false)->once(); //here with(false) is testing no argument is passed as the default argument value is false
        
        $mockResponseMessage->shouldReceive('__construct')->with('The comaparison is triggered', 200)->once()->andReturn($mockResponseMessage);
        $mockResponseMessage->shouldReceive('sendMessage')->with(true)->once();
        /////////////////////////////////////////////////////////
        $myClass = new MyClass();
        $myClass->handle(1);
    }
}

How to write unit test testMyClass for code (class myClass) calling a class from a library ResponseMessage with public constants (ResponseMessage::STATUS_ERROR, ...::STATUS_SUCCESS) and constructor ResponseMessage::__construct()?

hakre
  • 193,403
  • 52
  • 435
  • 836
cytsunny
  • 4,838
  • 15
  • 62
  • 129
  • Without extending MyClass ResponseMessage absolutely no chance. If it is possible, one could test method calls with a spy test. And then it's a dirty solution. – Bademeister Aug 08 '23 at 15:24
  • @Bademeister myClass can be changed, but ResponseMessage cannot be changed as it is library code. – cytsunny Aug 09 '23 at 07:53
  • Can you provide ResponseMessage in full so I can have a look at it? I find the topic exciting and I had tried something yesterday. I have not yet completely understood what exactly you want to test. Can you please summarize again briefly? Basically you cannot mock ResponseMessage in the case. – Bademeister Aug 09 '23 at 09:08
  • @Bademeister Sorry but ResponseMessage is an internal library (provided by another partner company) and I cannot show the code to outsiders. I have tried to write some fake code to give some more idea. For the test class I have tried adding some codes for demo on what I would like to test. – cytsunny Aug 09 '23 at 09:42
  • How does `MyClass` load `ResponseMessage`? Via autoloading or include? – Olivier Aug 14 '23 at 16:16
  • @Olivier It is loaded by autoloading – cytsunny Aug 14 '23 at 17:01
  • Then you could write your own version of `ResponseMessage` and use it instead of the real class. – Olivier Aug 14 '23 at 17:04
  • @Olivier That is what the current answer done? It works, but I am starting this bounty to see if there are other easier way to test. – cytsunny Aug 14 '23 at 19:23
  • @hakre Sorry, I mean the constant that is defined in the class. Edited the title. – cytsunny Aug 19 '23 at 09:41
  • Alright, guessed that it's not there ;) removed comment + the edit marker in the question (summary at the button). will clean this comment up later, it's not necessary any longer. – hakre Aug 19 '23 at 09:47

5 Answers5

2

Testing myClass::handle() method (as it is described in your question) comes to analyze two local objects instantiated inside of that method. These objects of type ResponseMessage should contain different values for text and status depending on $input argument.

You basically have two approaches:

  • without modification of myClass you can only mock the ResponseMessage class, that's what you did and yes, this is cumbersome, since for every combination of $input needs to rewrite the test sequence...
  • if you can modify myClass, you can use the Dependency Inversion Principle, with one of the possibility proposed in this answer. Another variation is proposed below. You provide a callback function to handle() that creates an object of type ResponseMessage during runtime, but ResponseMessageX during the tests. ResponseMessageX should use the same interface as ResponseMessage, but have no side effects and allow to test the value of text and status variables. For this we will extend ResponseMessage, redeclare $responseMessage, $responseCode and $sendToAdmin as public properties, and force the callback to store the instantiated test object in a place accessible from tests. This way we can analyze the object values after the call to myClass::handle().

Modified handle() method (create objects via injected $makeResponse callback):

<?php 
class myClass {
    function handle($input, callable $makeResponse) {
        //The actual comparision is much more complicated. Simplified for this question
        if (empty($input)) {
            // Some other error handling, no response message is generated
            return;
        }
        if ( $input == 1 )
            $responseMessage = $makeResponse('The input is 1', ResponseMessage::STATUS_SUCCESS );
        else if ( $input == 2 )
            $responseMessage = $makeResponse('The input is 2', ResponseMessage::STATUS_SUCCESS );
        else
            $responseMessage = $makeResponse('Unexpected input', ResponseMessage::STATUS_ERROR );
        
        $responseMessage->sendMessage();

        $responseMessageToAdmin = $makeResponse('The comaparison is triggered', RespnoseMessage::STATUS_SUCCESS);
        $responseMessageToAdmin->sendMessage(true);
    }
}

ResponseMessageX class normally would implement the same interface as ResponseMessage, but since we don't know if one is available, here we extend the latter:

<?php
class ResponseMessageX extends ResponseMessage {
    public ?string $responseMessage;
    public ?int $responseCode;
    public ?bool $sendToAdmin;
    
    public function sendMessage($sendToAdmin = false) {
        $this->sendToAdmin = $sendToAdmin;
    }
}

Callable used in production:

<?php
$makeResponse = function ($text, $code) {
    return new ResponseMessage($text, $code);
}

Callable used in tests:

<?php
$makeResponse = function ($text, $code) use (&$rmStore) {
    $rm = new ResponseMessageX($text, $code);
    $rmStore[] = $rm;
    return $rm;
}

Tests may now be much simpler:

<?php
class MyClassTest extends PHPUnit\Framework\TestCase
{
    /** @dataProvider providerHandle */
    function testHandle($input, $expectedText, $expectedCode)
    {
        $rmStore = [];
        $c = new myClass();
        $c->handle(
            $input,
            function ($text, $code) use (&$rmStore) {
                $rm = new ResponseMessageX($text, $code);
                $rmStore[] = $rm;
                return $rm;
            }
        );

        $this->assertSame($expectedText, $rmStore[0]->responseMessage);
        $this->assertSame($expectedCode, $rmStore[0]->responseCode);
    }

    function providerHandle()
    {
        return [
            [1, 'The input is 1', ResponseMessage::STATUS_SUCCESS],
            [2, 'The input is 2', ResponseMessage::STATUS_SUCCESS],
            [42, 'Unexpected input', ResponseMessage::STATUS_ERROR],
        ];
    }
}

Here in testHandle() we assume that the first created object will be analyzed, not the second (admin) one. If creation order in handle() somehow changes and we'll start generate admin responses before regular ones, the test will start failing. In this case you can rewrite test assertions as follows:

foreach ($rmStore as $rm) {
    if ($rm->sendToAdmin) {
        continue;
    }
    $this->assertSame($expectedText, $rm->responseMessage);
    $this->assertSame($expectedCode, $rm->responseCode);
}
Stas Trefilov
  • 173
  • 1
  • 6
2

You certainly want to make all the problems of new go away:

        if ( $input == 1 )
            $responseMessage = new ResponseMessage('The input is 1', ResponseMessage::STATUS_SUCCESS );
                               ###################
        else if ( $input == 2 )
            $responseMessage = new ResponseMessage('The input is 2', ResponseMessage::STATUS_SUCCESS );
                               ###################
        else
            $responseMessage = new ResponseMessage('Unexpected input', ResponseMessage::STATUS_ERROR );
                               ###################
        
        $respnoseMessage->sendMessage();

        $responseMessageToAdmin = new ResponseMessage('The comaparison is triggered', RespnoseMessage::STATUS_SUCCESS);
                                  ###################
        $responseMessageToAdmin->sendMessage(true);

I found four places.

The first three places create a closure for the ->sendMessage() protocol.

Same for the fourth place.

What your class is certainly missing is a seam for this:

public function responseSendMessage($responseMessage, $responseCode, $sendToAdmin = false)
{
    (new ResponseMessage($responseMessage, $responseCode))->sendMessage($sendToAdmin)
}

Introduce it at the places:

        switch ($input) {
            case 1: $this->responseSendMessage('The input is 1', ResponseMessage::STATUS_SUCCESS); break;
            case 2: $this->responseSendMessage('The input is 2', ResponseMessage::STATUS_SUCCESS); break;
            default: $this->responseSendMessage('Unexpected input', ResponseMessage::STATUS_ERROR);
        }

        $this->responseSendMessage('The comaparison is triggered', RespnoseMessage::STATUS_SUCCESS, true);

Now in testing decide which part you'd like to replace because all those parts that you may want to mock, are yours.

You can introduce the test and you can easily refactor locally with the seam only, then on the large scale later.

And you already have improved your code and this refactoring is local (only one method affected).

If you already have upgraded the PHP version, make use of match expressions instead of the switch/case.

How did I do it?

  • Find the offending parts (e.g. new not in creation only code, static method calls into libraries and third party frameworks etc.)
  • List them, look how they are used
  • Introduce a seam at those hurting places - possible?
  • Only test your code, not the third library code - get library code as far away as possible -> from 4 places to 1 place (75% of the original problem reduced just by that, it is much smaller now).
  • Only you can mock something must not mean that you should mock all that (!) - Train your nose. Otherwise greetings from Mocking Hell.

Yes, your question is simplified. Check if you can apply this or how. But also see what is happening here: Due to the simplification you can find those parts. So this is based on your work asking the question.

The seam idea is from Feathers refactoring legacy code book first edition. If you have some serious parts in front of you, get a couple of copies for your whole team and send everyone in the reading room, then discuss your reading experiences and learning together.


While what was shown is a Refactoring, you refactor pretty specifically here. That is, you need to make this go hand-in-hand with writing the test (not shown here), and best you write the test-code first (e.g. for mocking this one method) and then let it crash as the method not yet exists.

This is important because you want to have a test, too, that responseSendMessage() is working, but technically, you can fully render it void, it's a single method under your control.

The problem is not there any longer.

Happy testing & maintaining.

hakre
  • 193,403
  • 52
  • 435
  • 836
1

If you can modify MyClass, I would do the following. The challenge is that ResponseMessage needs values in the constructor and therefore cannot be mocked the way MyClass is structured.

The ResponseMessageAdapter is more or less a bridge to the ResponseMessage.

You pass the ResponseMessageAdapter to MyClass via the constructor. You can also use it outside the test.

ResponseMessageAdapter::getResponseMessage() returns the instance of ResponseMessage. So you can mock all the methods in ResponseMessage.

class ResponseMessageAdapter {

    private string $responseMessage;
    private int $responseCode;

    public function setResponseMessage(string $responseMessage): void
    {
        $this->responseMessage = $responseMessage;
    }

    public function setResponseCode(int $responseCode): void
    {
        $this->responseCode = $responseCode;
    }

    public function getResponseMessage(): ResponseMessage
    {
        return new ResponseMessage(
            $this->responseMessage,
            $this->responseCode
        );
    }
}

class MyClass {

    private ResponseMessageAdapter $responseMessageAdapter;

    public function __construct(ResponseMessageAdapter $responseMessageProxy)
    {
        $this->responseMessageAdapter = $responseMessageProxy;
    }

    public function handle($input)
    {
        if ($input == 1) {
            $this->responseMessageAdapter->setResponseMessage('The input is 1');
            $this->responseMessageAdapter->setResponseCode(ResponseMessage::STATUS_SUCCESS);
            $responseMessage = $this->responseMessageAdapter->getResponseMessage();
            $responseMessage->sendMessage();
        }
        // ...
    }
}
Bademeister
  • 389
  • 1
  • 10
1

as mentioned by the other answers without having a Dependency injection having a mock object for ResponseMessage will not help because your code is create a new object from the original class so your mocked instance will never be used

however I found this answer from a similar question which you may find helpful https://stackoverflow.com/a/40915836/19249695 but I quote

It is highly not recommended but possible

  • Not only it is not recommended back then, the library it used was not updated since 9 years ago, so I believe it won't be useful now. – cytsunny Aug 21 '23 at 09:08
  • yeah, but I think they mentioned that it's superseded by this https://github.com/krakjoe/uopz which is still getting recent updates – Bassel Hossam Aug 22 '23 at 06:49
-2
class testMyClass extends PHPUnit\Framework\TestCase {
public function testProcessWithInputOne() {
    // Create a mock of the ResponseMessage class
    $mockResponseMessage = $this->getMockBuilder(ResponseMessage::class)
        ->disableOriginalConstructor()
        ->getMock();

    // Set up expectations for the mock object
    $mockResponseMessage->expects($this->once())
        ->method('sendMessage')
        ->with($this->equalTo(false));

    // Replace the original ResponseMessage class with the mock
    $this->getMockBuilder('ResponseMessage')
        ->setMockClassName('ResponseMessage')
        ->setMockObjectConstructorArgs(['The input is 1', ResponseMessage::STATUS_SUCCESS])
        ->getMock();

    // Create an instance of myClass and call the handle() method
    $myClass = new myClass();
    $myClass->handle(1);
}

}

  • The `setMockObjectConstructorArgs()` function you have used is simply not found. I have also done some search on Google but it seems that this is not a default function that PHPUnit has? – cytsunny Aug 16 '23 at 15:07