phpunit icon indicating copy to clipboard operation
phpunit copied to clipboard

Can not mock interface with addMethods

Open aakashtushar opened this issue 3 years ago • 5 comments

Q A
PHPUnit version 8.5.17
PHP version 7.2.14
Installation Method Composer

Summary

When I try to mock an interface using createMock() method is works fine but with addMethods(), none of the method is mocked.

Current behavior

PHP throws an exception cause mock class does not implement any method:

PHP Fatal error:  Class Mock_SomeInterface_41ecf28a contains 8 abstract methods and must therefore be declared abstract or implement the remaining methods (SomeInterface::abc, SomeInterface::def, ...) in /.../project/vendor/phpunit/phpunit/src/Framework/MockObject/MockClass.php(45) : eval()'d code on line 3
PHP Stack trace:
PHP   1. {main}() /.../project/vendor/phpunit/phpunit/phpunit:0
PHP   2. PHPUnit\TextUI\Command::main() /.../project/vendor/phpunit/phpunit/phpunit:76
PHP   3. PHPUnit\TextUI\Command->run() /.../project/vendor/phpunit/phpunit/src/TextUI/Command.php:195
PHP   4. PHPUnit\TextUI\TestRunner->doRun() /.../project/vendor/phpunit/phpunit/src/TextUI/Command.php:236
PHP   5. PHPUnit\Framework\TestSuite->run() /.../project/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:656
.
.
...

How to reproduce

interface SomeInterface
{
    public function abc();

    public function def();
}

// unit tests
$this->getMockBuilder(SomeInterface::class)
    ->disableOriginalConstructor()
    ->disableOriginalClone()
    ->disableArgumentCloning()
    ->disallowMockingUnknownTypes()
    ->addMethods(['xyz'])
;

Expected behavior

Mocked interface class must implement all the methods from interface as it works with createMock().

aakashtushar avatar Jul 16 '21 06:07 aakashtushar

I do not know whether my attempt to fix this have bad side effects, but otherwise it should be fine

qossmic-training avatar Mar 23 '22 12:03 qossmic-training

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.

sebastianbergmann avatar Mar 23 '22 12:03 sebastianbergmann

Thanks for your fast reply. I will check once again and will provide my fix for 9.5 and master as well. As I'm not familiar with GitHub itself and how to provide valuable contribution I try to explain what I did in my fork instead: I took the code from the bug report and add this as a test - so creating a mock from interface with adding methods xyz, mock to return true on call and call this method on mock and assert if this is being true. Before my fix I got the mentioned error on abstract methods as the bug reporter. The fix is basically to let the generator to create this required methods.

qossmic-training avatar Apr 05 '22 08:04 qossmic-training

@qossmic-training @sebastianbergmann Hello everyone, do you have any updates on this problem ?

NicolasJourdan avatar Aug 11 '22 08:08 NicolasJourdan

An update on this issue would be highly appreciated 🙏

The issue seems to be somewhere down the method stack where $methods gets renamed to $explicitMethods and the default behavior here is that if $explicitMethods is null or empty array then at line \PHPUnit\Framework\MockObject\Generator::generateMock:800 (version 9.5.5) it will get populated with all methods of target class. But if you add a method using addMethods then, unlike what the method name says, then it will add the method to an empty array instead of the default behavior which is all methods which gives unexpected results...

It's kind of the same @qossmic-training has discovered in his PR but sadly his change would be a breaking change.

I have made below dirty fix for now in my own base case class:

/**
 * @param class-string $class
 */
protected function getMockBuilderWithAllMethods($class): MockBuilder
{
    return $this->getMockBuilder($class)->onlyMethods(
        \array_map(fn($method) => $method->getName(), (new \ReflectionClass($class))->getMethods()),
    );
}

On the returned MockBuilder you can now use addMethods with the expected behavior.

A quick non breaking fix could be to introduce a addAllMethods() to replicate what I just did here on the MockBuilder itself.

w3dev-dk avatar Sep 06 '22 19:09 w3dev-dk

$this->getMockBuilder(SomeInterface::class) ->disableOriginalConstructor() ->disableOriginalClone() ->disableArgumentCloning() ->disallowMockingUnknownTypes() ->addMethods(['xyz']) ->getMockForAbstarctClass() ;

The above construct works.

skellamp avatar Nov 02 '22 17:11 skellamp

getMockBuilder() will be soft-deprecated (@deprecated annotation) in PHPUnit 10.1, deprecated (triggering a deprecation warning) in PHPUnit 11, and removed in PHPUnit 12. Therefore no changes should be made to it anymore.

sebastianbergmann avatar Mar 06 '23 06:03 sebastianbergmann