sebastianbergmann/phpunit-mock-objects

Mocking interface with final return types fails since 6.5.x

frederikbosch opened this issue · 20 comments

frederikbosch commented
Q A
PHPUnit version 6.5.0-1
PHP version 7.1.12
Installation Method Composer

Since version 6.5.0 I have problems with mocking a specific class: DomainProviderInterface. It seems to be related to the method provide that is part of the class.

namespace Genkgo\TestFramework;

use PHPUnit\Framework\TestCase;

final class DomainNameCollection {}

interface DomainProviderInterface {
    public function provide(): DomainNameCollection;
    public function persist(DomainNameCollection $domains): void;
}

final class Test extends TestCase {
    public function test() {
        $mock = $this->createMock(DomainProviderInterface::class);
        $this->assertInstanceOf(DomainProviderInterface::class, $mock);
    }
}

Above code works in 6.4.x, but fails in 6.5.x. I tested both 6.5.0 and 6.5.1.

sebastianbergmann commented

The problem is that DomainNameCollection is final. Final classes cannot be stubbed or mocked.

frederikbosch commented

@sebastianbergmann I am not mocking the final class. I am mocking the interface. And this works in PHPUnit 6.4.

Deleted user commented

The problem is that phpunit must implement the method in the mock. The default behavior is that it returns a new mock of the return type hint (if given), wich is impossible as you can't mock final classes.

Deleted user commented

See #386. As I pointed there, it could be possible to mock such a method if it returns the typehint with ?, so it could return null for that method as default.

frederikbosch commented

That leaves me with two questions.

  1. As this works in 6.4.0, and always worked for me in earlier, why does it not work in 6.5.0? It should at least have a major release since it is backward incompatible.

  2. In your reply you are using the default behavior suggesting you can overcome the default behaviour. How would I bypass the default behaviour.

Deleted user commented
  1. It's actually a major version of phpunit-mock-objects v4 to v5
  2. It seems to be impossible now. You are not able to mock interfaces that return final classes. (Which is a problem ;) )
frederikbosch commented

Well that is a problem indeed. I get that the default behaviour of a mock that all its methods return mocks of the return type. However, why would you not create that mock once the method is called. And thereby allowing to override the default behaviour via willReturn.

  • 👍2
sebastianbergmann commented 
<?php
final class Foo
{
}

interface Bar
{
    public function baz(): Foo;
}

class Test extends PHPUnit\Framework\TestCase
{
    public function testOne()
    {
        $stub = $this->createMock(Bar::class);
        
        $this->assertInstanceOf(Bar::class, $stub);

        $this->assertInstanceOf(Foo::class, $stub->baz());
    }
}

phpunit-mock-objects 4.0

PHPUnit 6.4.4 by Sebastian Bergmann and contributors.

W                                                                   1 / 1 (100%)

Time: 50 ms, Memory: 4.00MB

There was 1 warning:

1) Test::testOne
Class "Foo" is declared "final" and cannot be mocked.

WARNINGS!
Tests: 1, Assertions: 1, Warnings: 1.

phpunit-mock-objects 5.0

PHPUnit 6.5.1 by Sebastian Bergmann and contributors.

PHP Fatal error:  Class Mock_Bar_7e536b9d contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Bar::baz) in /usr/local/src/phpunit/vendor/phpunit/phpunit-mock-objects/src/Generator.php(264) : eval()'d code on line 1
PHP Stack trace:
PHP   1. {main}() /usr/local/src/phpunit/phpunit:0
PHP   2. PHPUnit\TextUI\Command::main() /usr/local/src/phpunit/phpunit:53
PHP   3. PHPUnit\TextUI\Command->run() /usr/local/src/phpunit/src/TextUI/Command.php:148
PHP   4. PHPUnit\TextUI\TestRunner->doRun() /usr/local/src/phpunit/src/TextUI/Command.php:195
PHP   5. PHPUnit\Framework\TestSuite->run() /usr/local/src/phpunit/src/TextUI/TestRunner.php:546
PHP   6. Test->run() /usr/local/src/phpunit/src/Framework/TestSuite.php:755
PHP   7. PHPUnit\Framework\TestResult->run() /usr/local/src/phpunit/src/Framework/TestCase.php:894
PHP   8. Test->runBare() /usr/local/src/phpunit/src/Framework/TestResult.php:698
PHP   9. Test->runTest() /usr/local/src/phpunit/src/Framework/TestCase.php:939
PHP  10. ReflectionMethod->invokeArgs() /usr/local/src/phpunit/src/Framework/TestCase.php:1071
PHP  11. Test->testOne() /usr/local/src/phpunit/src/Framework/TestCase.php:1071
PHP  12. Test->createMock() /home/sb/Test.php:15
PHP  13. PHPUnit\Framework\MockObject\MockBuilder->getMock() /usr/local/src/phpunit/src/Framework/TestCase.php:1497
PHP  14. PHPUnit\Framework\MockObject\Generator->getMock() /usr/local/src/phpunit/vendor/phpunit/phpunit-mock-objects/src/MockBuilder.php:118
PHP  15. PHPUnit\Framework\MockObject\Generator->getObject() /usr/local/src/phpunit/vendor/phpunit/phpunit-mock-objects/src/Generator.php:200
PHP  16. PHPUnit\Framework\MockObject\Generator->evalClass() /usr/local/src/phpunit/vendor/phpunit/phpunit-mock-objects/src/Generator.php:221
PHP  17. eval() /usr/local/src/phpunit/vendor/phpunit/phpunit-mock-objects/src/Generator.php:264
sebastianbergmann commented

There are two differences here between phpunit-mock-objects 4.0 and phpunit-mock-objects 5.0:

  • phpunit-mock-objects 4.0 raises a proper PHPUnit warning when trying to call a method on a stub that has a final class as its return type declaration
  • phpunit-mock-objects 5.0 triggers a PHP compiler error when trying to stub a method that has a final class as its return type declaration
    In the end, the result is the same and nothing that worked in phpunit-mock-objects 4.0 stopped working in phpunit-mock-objects 5.0.

Of course, such a PHP compiler error must not be triggered and a PHPUnit warning should be raised instead. But I definitely prefer to raise this issue at the point in time when the stub is created rather then when a problematic method is invoked for the first time.

frederikbosch commented 
namespace Genkgo\TestFramework;

use PHPUnit\Framework\TestCase;

final class DomainNameCollection {}

interface DomainProviderInterface {
    public function provide(): DomainNameCollection;
    public function persist(DomainNameCollection $domains): void;
}

final class Test extends TestCase {
    public function test() {
        $mock = $this->createMock(DomainProviderInterface::class);
        $this->assertInstanceOf(DomainProviderInterface::class, $mock);
    }
}

Case 1: PHPUnit 6.4.4

➜  git:(master) ✗ composer show |grep phpunit
phpunit/php-code-coverage          5.2.4   Library that provides collection, processing, and rendering functionality for PHP code coverage information.
phpunit/php-file-iterator          1.4.5   FilterIterator implementation that filters files based on a list of suffixes.
phpunit/php-text-template          1.2.1   Simple template engine.
phpunit/php-timer                  1.0.9   Utility class for timing
phpunit/php-token-stream           2.0.2   Wrapper around PHP's tokenizer extension.
phpunit/phpunit                    6.4.4   The PHP Unit Testing framework.
phpunit/phpunit-mock-objects       4.0.4   Mock Object library for PHPUnit

➜  git:(master) ✗ vendor/bin/phpunit test/Unit/Test.php 
PHPUnit 6.4.4 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.1.12-1+ubuntu16.04.1+deb.sury.org+1 with Xdebug 2.5.5
Configuration: /workspace/project/phpunit.xml

.                                                                   1 / 1 (100%)

Time: 39 ms, Memory: 4.00MB

OK (1 test, 1 assertion)

Case 2: PHPUnit 6.5.1

➜  git:(master) ✗ composer show |grep phpunit          
phpunit/php-code-coverage          5.2.4   Library that provides collection, processing, and rendering functionality for PHP code coverage information.
phpunit/php-file-iterator          1.4.5   FilterIterator implementation that filters files based on a list of suffixes.
phpunit/php-text-template          1.2.1   Simple template engine.
phpunit/php-timer                  1.0.9   Utility class for timing
phpunit/php-token-stream           2.0.2   Wrapper around PHP's tokenizer extension.
phpunit/phpunit                    6.5.1   The PHP Unit Testing framework.
phpunit/phpunit-mock-objects       5.0.2   Mock Object library for PHPUnit

➜  git:(master) ✗ vendor/bin/phpunit test/Unit/Test.php
PHPUnit 6.5.1 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.1.12-1+ubuntu16.04.1+deb.sury.org+1 with Xdebug 2.5.5
Configuration: /workspace/project/phpunit.xml

PHP Fatal error:  Class Mock_DomainProviderInterface_eab8f31f contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Genkgo\TestFramework\DomainProviderInterface::provide) in /workspace/project/vendor/phpunit/phpunit-mock-objects/src/Generator.php(264) : eval()'d code on line 1
PHP Stack trace:
PHP   1. {main}() /workspace/project/vendor/phpunit/phpunit/phpunit:0
PHP   2. PHPUnit\TextUI\Command::main() /workspace/project/vendor/phpunit/phpunit/phpunit:53
PHP   3. PHPUnit\TextUI\Command->run() /workspace/project/vendor/phpunit/phpunit/src/TextUI/Command.php:148
PHP   4. PHPUnit\TextUI\TestRunner->doRun() /workspace/project/vendor/phpunit/phpunit/src/TextUI/Command.php:195
PHP   5. PHPUnit\Framework\TestSuite->run() /workspace/project/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:546
PHP   6. Genkgo\TestFramework\Test->run() /workspace/project/vendor/phpunit/phpunit/src/Framework/TestSuite.php:755
PHP   7. PHPUnit\Framework\TestResult->run() /workspace/project/vendor/phpunit/phpunit/src/Framework/TestCase.php:894
PHP   8. Genkgo\TestFramework\Test->runBare() /workspace/project/vendor/phpunit/phpunit/src/Framework/TestResult.php:698
PHP   9. Genkgo\TestFramework\Test->runTest() /workspace/project/vendor/phpunit/phpunit/src/Framework/TestCase.php:939
PHP  10. ReflectionMethod->invokeArgs() /workspace/project/vendor/phpunit/phpunit/src/Framework/TestCase.php:1071
PHP  11. Genkgo\TestFramework\Test->test() /workspace/project/vendor/phpunit/phpunit/src/Framework/TestCase.php:1071
PHP  12. Genkgo\TestFramework\Test->createMock() /workspace/project/test/Unit/Test.php:15
PHP  13. PHPUnit\Framework\MockObject\MockBuilder->getMock() /workspace/project/vendor/phpunit/phpunit/src/Framework/TestCase.php:1497
PHP  14. PHPUnit\Framework\MockObject\Generator->getMock() /workspace/project/vendor/phpunit/phpunit-mock-objects/src/MockBuilder.php:118
PHP  15. PHPUnit\Framework\MockObject\Generator->getObject() /workspace/project/vendor/phpunit/phpunit-mock-objects/src/Generator.php:200
PHP  16. PHPUnit\Framework\MockObject\Generator->evalClass() /workspace/project/vendor/phpunit/phpunit-mock-objects/src/Generator.php:221
PHP  17. eval() /workspace/project/vendor/phpunit/phpunit-mock-objects/src/Generator.php:264
frederikbosch commented

@sebastianbergmann However, if I do execute your example, I get the same result as you do. So what makes my case different from yours?

frederikbosch commented

@sebastianbergmann Maybe you can see what you get when you try my case in PHPUnit 6.4.4. Does it pass?

sebastianbergmann commented

At first glance, #388 (comment) seems to make sense. I will have investigate this. However, probably not over the weekend.

frederikbosch commented

@sebastianbergmann Sounds good. You may want to consider to create a new tag 6.5.2 that uses the old phpunit mock objects. Will prevent other people hitting the bug and fixing this might take a while.

sebastianbergmann commented

@frederikbosch Unfortunately, PHPUnit 6.5 is not compatible with phpunit-mock-objects 4.0.

sebastianbergmann commented

With

diff --git a/src/Generator.php b/src/Generator.php
index 643b676..7c28df7 100644
--- a/src/Generator.php
+++ b/src/Generator.php
@@ -1029,39 +1029,7 @@ private function generateMockedMethodDefinition($className, $methodName, $cloneA
      */
     private function canMockMethod(ReflectionMethod $method)
     {
-        return !($method->isConstructor() || $method->isFinal() || !$this->canReturnTypeBeStubbed($method) || $method->isPrivate() || $this->isMethodNameBlacklisted($method->getName()));
-    }
-
-    /**
-     * @param ReflectionMethod $method
-     *
-     * @return bool
-     *
-     * @throws \ReflectionException
-     */
-    private function canReturnTypeBeStubbed(ReflectionMethod $method)
-    {
-        if (!$method->hasReturnType()) {
-            return true;
-        }
-
-        if (PHP_VERSION_ID >= 70100 && $method->getReturnType()->allowsNull()) {
-            return true;
-        }
-
-        $returnType = (string) $method->getReturnType();
-
-        if ($returnType === \Generator::class || $returnType === \Closure::class) {
-            return true;
-        }
-
-        if (\class_exists($returnType)) {
-            $class = new ReflectionClass($returnType);
-
-            return !$class->isFinal();
-        }
-
-        return true;
+        return !($method->isConstructor() || $method->isFinal() || $method->isPrivate() || $this->isMethodNameBlacklisted($method->getName()));
     }
 
     /**

I get the same result for PHPUnit 6.4 and PHPUnit 6.5 with the example from #388 (comment).

sebastianbergmann commented

@frederikbosch @weierophinney Does the patch shown in #388 (comment) solve your problem?

frederikbosch commented

Thanks for the fast help!

driesvints commented

@sebastianbergmann I can confirm that with PHPUnit 5.6.2 this problem has been fixed. Thanks!

frederikbosch commented

@sebastianbergmann My problem is solved too. Thanks again for your fast handling.