Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Mocking interface with final return types fails since 6.5.x #388

Closed
frederikbosch opened this issue Dec 1, 2017 · 20 comments
Closed

Mocking interface with final return types fails since 6.5.x #388

frederikbosch opened this issue Dec 1, 2017 · 20 comments

Comments

@frederikbosch
Copy link

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
Copy link
Owner

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

@frederikbosch
Copy link
Author

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

@ghost
Copy link

ghost commented Dec 1, 2017

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.

@ghost
Copy link

ghost commented Dec 1, 2017

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
Copy link
Author

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.

@ghost
Copy link

ghost commented Dec 1, 2017

  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 frederikbosch changed the title Mocking class with method 'provide' fails since 6.5.x Mocking class with final return types fails since 6.5.x Dec 1, 2017
@frederikbosch frederikbosch changed the title Mocking class with final return types fails since 6.5.x Mocking interface with final return types fails since 6.5.x Dec 1, 2017
@frederikbosch
Copy link
Author

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.

@sebastianbergmann
Copy link
Owner

<?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
Copy link
Owner

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
Copy link
Author

frederikbosch commented Dec 1, 2017

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
Copy link
Author

@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
Copy link
Author

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

@sebastianbergmann
Copy link
Owner

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

@frederikbosch
Copy link
Author

@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
Copy link
Owner

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

@sebastianbergmann
Copy link
Owner

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
Copy link
Owner

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

@frederikbosch
Copy link
Author

Thanks for the fast help!

@driesvints
Copy link

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

@frederikbosch
Copy link
Author

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants