sebastianbergmann / phpunit

The PHP Unit Testing framework.
https://phpunit.de/
BSD 3-Clause "New" or "Revised" License
19.65k stars 2.2k forks source link

createPartialMock called with method(s) from that do not exist can be valid use case #4240

Closed zeleznypa closed 3 years ago

zeleznypa commented 4 years ago
Q A
PHPUnit version 8.5.4
PHP version 7.3.11
Installation Method Composer

Summary

When I tried to mock an object Dibi/Fluent method from, the PhpUnit return the following message: createPartialMock called with method(s) from that do not exist in Dibi\Fluent. This will not be allowed in future versions of PHPUnit.

Unfortunately the method select is valid callable method based on the magic __call.

Current behavior

When I create a mock with method that is valid and callable via __call but not exactly defined, the PhpUnit generate a warning.

How to reproduce

    public function testFoo(): void
    {
        $fluent = $this->createFluentMock(['from']);
        // ...
    }

    /**
     * Fluent mock factory
     *
     * @param string[] $methods [OPTIONAL] List of mocked method names
     * @return \Dibi\Fluent|MockObject
     */
    protected function createFluentMock(array $methods = []): MockObject
    {
        $mock = $this->createPartialMock(Fluent::class, $methods);
        return $mock;
    }

Expected behavior

Don't generate this warning, when the class contain __call magic.

sebastianbergmann commented 4 years ago

Thank you for your report.

Please provide a minimal, self-contained, reproducing test case that shows the problem you are reporting.

Without such a minimal, self-contained, reproducing test case I will not be able to investigate this issue.

zeleznypa commented 4 years ago

Hello @sebastianbergmann, thanks for your fast reply. I hope that this code helps you to understand better: https://github.com/zeleznypa/phpunit-4240/blob/master/tests/DibiTest.php

This is the typical usage of the Dibi/Fluent class: https://phpfashion.com/dibifluent-tekute-sql-prikazy

sebastianbergmann commented 4 years ago

Sorry, but that is not self-contained.

zeleznypa commented 4 years ago

@sebastianbergmann I'm sorry, but I don't know what does the self-contained mean.

The repository https://github.com/zeleznypa/phpunit-4240 contain everything to reproduce the bug.

Can you please send me an example of bug-report with self-contained example?

sebastianbergmann commented 4 years ago

"self-contained" means no dependencies.

zeleznypa commented 4 years ago

@sebastianbergmann Thanks for the patience.

The dependency on Dibi was replaced with minimal code example.

Hope that this is what you want :)

wilson1000-MoJ commented 4 years ago

Hi, was there an outcome to satisfy the issue described here?

ls-chloe-schoreisz commented 3 years ago

+1 ? Running into this issue while testing post method from the GuzzleHttp\Client class

sebastianbergmann commented 3 years ago

Instead of configuring methods that do not exist, configure the method that does exist: __call().

PivitParkour94 commented 1 year ago

Is the "right" way to unit test Magic methods to actually have a complex return callback for the underlying __call method?

I can make a different issue to promote a discussion if the answer is not "yes" or "no"

In my case I'm working on making my existing codebase which is rife with magic methods more "unit-testable". From my understanding defining specific methods rather than relying on the magic methods is the best practice, but requires lots of code changes to implement. Before I spend many hours removing all the magic methods for testability sake, is it possible to simply mock the return value of __call in order to exercise those magic methods? I have done extensive research into the best way and haven't found an authoritative "yes" or "no" answer.

zeleznypa commented 1 year ago

@PivitParkour94 it is true "unit" testing. But when you need to test the application "end to end”, it is (from my point of view) correct to call the "magic" method. And it is also more "transparent".

PivitParkour94 commented 1 year ago

I came up with a somewhat ok option, I still think testing magic methods should not throw an error My "Solution"

        $magicMock = $this->createPartialMock(MyMagicClass::class,
            [
                '__call',
            ]
        );
        $magicMock->method('__call')->will(
            $this->returnCallback(
                function($data)
                {
                    $args = func_get_args();

                    if($args[0] == 'getName')
                        return 'wizard guy';
                    if($args[0] == 'getSpell')
                        return 'avada kedavra';
                }
            )
        );

To extend this instead of writing each method expected, you would then add to the anonymous function in the returnCallback. I don't love it, but it works well :)

zeleznypa commented 1 year ago

@PivitParkour94 For some cases it can be the way. Unfortunately not the „definitive“ one, because there is no possibility to test for example that „setter” is called „just once“, because you „reimplement“ the getter elsewhere, so the test will pass even when code will be changed...