Phpunit: PHPUnit_Framework_TestCase::createConfiguredMock() doesn't mock protected methods

[morozov]

| Q | A
| --------------------| ---------------
| PHPUnit version | 6.0.8
| PHP version | 7.1.2
| Installation Method | Composer

Currently, PHPUnit\Framework\TestCase::createConfiguredMock() doesn't mock protected methods, and judging by the error message it's not the expected behavior. It's also unexpected as technically it is possible to mock a protected method if it's specified during mock initialization.

Consider the following test:

<?php

use PHPUnit\Framework\TestCase;

class SUT
{
    public function foo()
    {
        return $this->bar();
    }

    protected function bar()
    {
        return 'bar';
    }
}

class ConfiguredMockTest extends TestCase
{
    public function testMockProtectedMethodUsingPartialMock()
    {
        $mock = $this->createPartialMock(SUT::class, ['bar']);
        $mock->method('bar')->willReturn('baz');
        $this->assertEquals('baz', $mock->foo());
    }

    public function testMockProtectedMethodUsingConfiguredMock()
    {
        $mock = $this->createConfiguredMock(SUT::class, [
            'bar' => 'baz'
        ]);
        $this->assertEquals('baz', $mock->foo());
    }
}

It produces the following output:

There was 1 warning:

1) ConfiguredMockTest::testMockProtectedMethodUsingConfiguredMock
Trying to configure method "bar" which cannot be configured because it does not exist, has not been specified, is final, or is static

/home/morozov/Projects/phpunit/src/Framework/TestResult.php:701
/home/morozov/Projects/phpunit/src/Framework/TestCase.php:867
/home/morozov/Projects/phpunit/src/Framework/TestSuite.php:739
/home/morozov/Projects/phpunit/src/Framework/TestSuite.php:739
/home/morozov/Projects/phpunit/src/Framework/TestSuite.php:739
/home/morozov/Projects/phpunit/src/TextUI/TestRunner.php:514
/home/morozov/Projects/phpunit/src/TextUI/Command.php:207
/home/morozov/Projects/phpunit/src/TextUI/Command.php:136

However, SUT::bar() exists, was specified, it's not final or static.

[localheinz]

@morozov

TestCase::createConfiguredMock() creates a _full_ mock. That is, all (!) methods are mocked, and the configured methods will return the specified return values.

How much sense does it make to configure a protected method of a _full_ mock, when _all_ of the public methods are mocked, and _none_ of them actually have a chance to invoke the protected method?

Adjusting TestCase::createConfiguredMock() to mock the specified methods only, instead of all of them, will break existing behaviour.

For reference, see #2596.

[sebastianbergmann]

Thank you for investigating this, @localheinz. I am closing this now.

[morozov]

@localheinz thank you for the explanation. Besides backward compatibility, what's the reason for createConfiguredMock() to create a full mock?

I see that #2596 breaks a unit test which makes sure that a non-specified method is also mocked and thus returns null, but I'm curious if this behavior is deliberately designed and thus should not be altered, or it's happened to be implemented like this.

[alex-pravdin-sn]

@morozov This is a religious position of PHPUnit. I also tried to convince to leave a legitimate way to mock protected methods but I was told that I can't code and my program is totally wrong if I'm trying to do that. PHPUnit is not only a testing framework, it's also a school of coding that punches you in your face if you code the wrong way :)

[ozanakc]

@alex-pravdin-sn which means that, i cannot test internal functions in a class. I have to trust it's behaviour or have to switch it to public in order to test it.

[alex-pravdin-sn]

@ozanakc Yup, you can create your own workarounds to extend the target class by a temporary class that will make all protected methods public and then test them. This can be solved pretty easily by very few changes in PHPUnit. But... This particular functionality is roughly denied and can not be discussed.

PHPUnit is for simple code tests only. If you have a complex business logic split into reusable methods and want to test those methods separately testing edge cases without repeating the same testing code lines hundreds of times - PHPUnit is not your friend. Either use your own band-aids or not use PHPUnit.

[alex-pravdin-sn]

Any non-public method is an implementation detail of a public method. Test that public method and you are fine. Testing non-public methods in isolation from the public method(s) that use(s) them is a bad idea.

Sebastian knows better what is good and what is bad for any of your code.

[alex-pravdin-sn]

@localheinz

How much sense does it make to configure a protected method of a full mock, when all of the public methods are mocked, and none of them actually have a chance to invoke the protected method?

Is it possible to extend this class and make protected methods public without changing logic? Does it make sense to mock these methods now? If so, why forcing developers to write endless band-aids instead of giving them a proper tool and let them decide how to use it? Stating it doesn't make sense or this is a bad idea you're judging categorically instead of developers while knowing nothing about why they really came to the idea of testing protected methods. Why do you take responsibility for the users' code judging what makes sense for them?

The functionality asked many times is pretty simple and requires very little time to implement and minor changes. You even may ask to suggest a PR for that. I'm wondering why do you reject this possibility so categorically?

[localheinz]

@alex-pravdin-sn

If phpunit/phpunit allowed mocking protected methods, we would need to document it. If we documented this behavior, this documentation would suggest that the makers of phpunit/phpunit endorse mocking protected methods.

I can only speak for myself, but I do not endorse it, and I would not recommend it.

Besides, there is no need to use the built-in mocking facilities to create test doubles. If you insist on mocking protected methods, you can easily create your test doubles yourself:

<?php

$double = new class extends ClassWithProtectedMethod {
    protected function methodINeedToMock() 
    {
        // whatever needs to be done here
    }
};

[ozanakc]

@localheinz from your comments, it's like you think protected messages don't need to be tested. Only what we need is just testing the main functions that are visible to other classes. But since unit testing suggests every unit to be tested independently, aren't protected functions units in terms of unit-testing?

[alex-pravdin-sn]

@localheinz It still looks like a bandaid, isn't it? If we change a little the base class we would have to change the corresponding double, adding more useless work. I know that testing protected methods separately seems like a not so good idea. But sometimes I think it's relly needed.

Here is my example:

class OrderProcessing
{
    public method createOrder($customer, $cartItems, $orderParameters)
    {
        $this->checkCustomerBalance($customer);
        $this->checkOrderItemsAvailability($customer, $cartItems);
        if ($orderParameters->isInstant()) {
            $order = $this->doCreateInstantOrder($customer, $cartItems, $orderParameters);
        } else {
            $order = $this->doCreateRegularOrder($customer, $cartItems, $orderParameters);
        }
        $this->updateCustomerBalance($customer, $order);
        $this->sendOrderCreatedNotifications($order);
    }
    ...
}

Imagine if we need to test the last method of sending notifications with multiple edge cases? For example, multiple mailing settings. If not allowed to call protected methods directly, we will have to copy many lines of code to pass previous methods. And they will have no relation to the test itself - testing notifications. Orders creation can also be very complicated with many steps and passing them may take a lot of lines of the test code.

I was working on a project with the big and complex business logic of creating orders: checking customers, checking balances, checking availability, calculating prices and discounts, calculation shipping and delivery options. All in one big public method exposed to the frontend. Internally, this logic was split into multiple methods, some of them were reused across a few public methods of creating orders (regular retail order, wholesale order, partner order, etc). In such cases testing separate parts of the bigger logic saving a lot of time and makes tests very simple and clear. Nowhere except tests these parts will be called separately.

So I'm voting for having a possibility to do that without band-aids and kindly asking you to consider adding it.

[localheinz]

@alex-pravdin-sn

The pain you are experiencing is essential.

It is a pain the vast majority of developers will never experience because they do not test their code.

You experience this pain because you are probably trying to write tests for poorly designed software. It is not impossible to write these tests, but it isn't easy. Often, though, it is easier to make changes to your code first before writing these tests.

However, the lack of features in phpunit/phpunit is not the root cause of the pain. Perhaps you and everyone else in the team are better off discussing your software's design instead of requesting features for phpunit/phpunit that would enable developers, in particular users of phpunit/phpunit, to create poorly designed software?

[alex-pravdin-sn]

@localheinz Your arguments absolutely right if PHPUnit would be a code-style verification tool, not just a unit-testing tool. How do you think? Is it a function of the good-design unit-testing tool to judge the design patterns of the tested code?

Of course, any code can be refactored to anything. But how would you be able to convince business/product owners of working and profitable products that the code needs to be refactored because the unit test tool told the existing code is poor?

I totally agree with you that it could be not a good idea. But I totally not agreed that a unit test tool should apply any code design restrictions. I think that it should test existing code no matter what it is, not teach coding.

[alex-pravdin-sn]

Imagine if the OS will prevent poorly designed programs from running because it decided the developers should make some redesign.

If you want to control the tested code design, please describe that clearly and fully on the main page of the PHPUnit website. To make everyone clearly understand that this tool is not only tests, it also controls the design. But I think this also will require to be consistent in this intention and implement a lot more features to control not only protected methods but even worse designs. There can be a code from the hell but without protected methods. Why protected methods become a stumbling block? I don't see the consistency in the declaration of fighting for good design when it applies to protected methods only, a very small piece from the large variety of poor designs (as you think they are poor).

Of course, you may implement what you want, not we. But when users have common problems and asking for minor improvements to solve their problems efficiently, why do you tell them that they... can't code good design? The teaching tone is not an appropriate choice to convince others in your position why you don't wish to implement the requested feature and even not ready to discuss it.

[ozanakc]

I totally agree with every word of @alex-pravdin-sn. It’s very important for the community to be able to use unit testing in as much software as we can. And we kindly request from php-unit to support us.