Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creating an alias or overload of an interface containing __construct fails #650

Open
chappy84 opened this Issue Dec 11, 2016 · 13 comments

Comments

Projects
None yet
4 participants
@chappy84
Copy link

chappy84 commented Dec 11, 2016

When trying to create a test double of an existing class that implements the correct interface, a BadMethodCallException is thrown (in the case of alias:) or the generated code contains syntax errors (in the case of overload:)

This can be reproduced easily with the following POCs:

In the case of alias, the code:

interface TestInterface
{
    public function __construct();
}

$mock = Mockery::mock('alias:TestClass', 'TestInterface');
$mock->shouldReceive('__construct');

$foo = new TestClass();

Mockery::close();

will produce the following output:

PHP Fatal error:  Uncaught exception 'BadMethodCallException' with message 'Method TestClass::__construct() does not exist on this mock object' in /path/to/test/vendor/mockery/mockery/library/Mockery/Loader/EvalLoader.php(17) : eval()'d code:709
Stack trace:
#0 /path/to/test/vendor/mockery/mockery/library/Mockery/Loader/EvalLoader.php(17) : eval()'d code(761): TestClass->_mockery_handleMethodCall('__construct', Array)
#1 /path/to/test/test.php(13): TestClass->__construct()
#2 {main}
  thrown in /path/to/test/vendor/mockery/mockery/library/Mockery/Loader/EvalLoader.php(17) : eval()'d code on line 709

In the case of overload, the code:

interface TestInterface
{
    public function __construct();
}

$mock = Mockery::mock('overload:TestClass', 'TestInterface');

$foo = new TestClass();

Mockery::close();

will produce the following output:

PHP Fatal error:  Cannot redeclare TestClass::__construct() in /path/to/test/vendor/mockery/mockery/library/Mockery/Loader/EvalLoader.php(17) : eval()'d code on line 788
PHP Stack trace:
PHP   1. {main}() /path/to/test/test.php:0
PHP   2. Mockery::mock() /path/to/test/test.php:10
PHP   3. call_user_func_array() /path/to/test/vendor/mockery/mockery/library/Mockery.php:79
PHP   4. Mockery\Container->mock() /path/to/test/vendor/mockery/mockery/library/Mockery.php:79
PHP   5. Mockery\Loader\EvalLoader->load() /path/to/test/vendor/mockery/mockery/library/Mockery/Container.php:219

These are both present in the latest 0.9.6 release

@robertbasic

This comment has been minimized.

Copy link
Collaborator

robertbasic commented May 6, 2017

@chappy84 when using alias or overload, the class that gets aliased or overloaded, must not be loaded by PHP at the time when Mockery does it's thing.

So I'm guessing that's the issue you are seeing, and we can't really do anything about it.

See the Mocking hard dependencies cookbook entry for a thorough explanation.

If that is really the case, then this issue can be closed, as it's how Mockery works.

@chappy84

This comment has been minimized.

Copy link
Author

chappy84 commented May 6, 2017

@robertbasic unfortunately that isn't the case. The code originally provided, aside from including mockery, was all I ran to produce that output. These were both stand alone test scripts so that I could ensure these were both bugs.

The issues are:

  • In the case of alias, you can't tell Mockery to expect a __construct call when stating the generated class must conform to a provided interface which contains a __construct definition
  • In the case of overload, Mockery generates a class which contains two __construct methods, one by default, the other to match the interface containing a __construct method you're expecting the newly generated class to implement.

In both of the above cases there was no class named TestClass loaded. Obviously If this has been fixed since 0.9.6 please feel free to close the issue.

I hope this clarification helps.

@robertbasic

This comment has been minimized.

Copy link
Collaborator

robertbasic commented May 6, 2017

@chappy84 yes, but having the interface in the same file as the $mock = Mockery::mock('alias:TestClass', 'TestInterface'); line, means that the TestInterface is already loaded, and Mockery can't alias/overload it.

@chappy84

This comment has been minimized.

Copy link
Author

chappy84 commented May 6, 2017

@robertbasic Mockery isn't trying to overload TestInterface here. The second parameter is a list of interfaces the generated class should implement. At-least that's how it works according to both the documentation and the Mockery codebase.

@robertbasic

This comment has been minimized.

Copy link
Collaborator

robertbasic commented May 6, 2017

@chappy84 ah, you're right. I'll investigate some more.

@robertbasic

This comment has been minimized.

Copy link
Collaborator

robertbasic commented May 10, 2017

@chappy84 what is your use case for this? The alias: and overload: "operators" are in Mockery to provide aliasing/overriding already existing classes, so to me adding additional interfaces to those existing classes by "gluing" interfaces on them with Mockery seems weird.

Why don't you just implement the interfaces you want on your existing classes and then alias/overload them?

I would really like to understand your usecase first, before diving deep into this.

Thanks!

@chappy84

This comment has been minimized.

Copy link
Author

chappy84 commented May 12, 2017

@robertbasic Unfortunately both alias: and overload: are unaware of the interfaces implemented by the existing class. To discover this via reflection (which Mockery seems to use) they would have to load the existing class, at which point they cannot perform the functionality to which they have been provided, as the class will now exist with the name they are trying to create.

The interfaces have been implemented on the original classes to ensure that design patterns are adhered to.

@robertbasic

This comment has been minimized.

Copy link
Collaborator

robertbasic commented May 12, 2017

@chappy84 you are right, again, about that part. But I still don't understand you're use case.

@chappy84

This comment has been minimized.

Copy link
Author

chappy84 commented May 14, 2017

@robertbasic I decided to show this in an example. This is obviously a very crude example, but I'd expect a not overly un-expected extension of the the reason overload: was implemented originally.

TestInterface.php

interface TestInterface
{
    public function __construct();
    public function foo();
}

TestArgument.php

class TestArgument implements TestInterface
{
    public function __construct()
    {
        // do stuff
    }

    public function foo()
    {
        // do stuff
    }
}

ClassA.php

class ClassA
{
    protected $classB;

    public function __construct()
    {
        $this->classB = new ClassB();
    }

    public function bar();
    {
        $baz = new TestArgument();
        return $this->classB->qux($baz);
    }
}

ClassB.php

class ClassB
{
    public function qux(TestInterface $baz)
    {
        return $baz->foo();
    }
}

Then this would normally be executed by running the following:

$foo = new ClassA();
$res = $foo->bar();

In a test scenario for ClassA::bar I'd want to overload TestArgument, but the type declaration on ClassB::qux requires TestInterface, thus requiring the interface to be implemented on the mockery created class.

@chappy84

This comment has been minimized.

Copy link
Author

chappy84 commented May 19, 2017

The issue with overload: is stemming from the appending of a __construct method in Mockery\Generator\StringManipulation\Pass\InstanceMockPass thanks to $config->isInstanceMock() returning true. This value defaults to false in the Mockery\Generator\MockConfiguration class and is only set to true by Mockery\Container::mock when finding an argument that defines the class as an overload:

Because the interface also has a __construct method, it adds a method of the same name for the actual mocked method, and thus the parser recognises a conflict when run through exec in Mockery\Loader\EvalLoader

With regards to alias:, Mockery generates the code correctly, without conflicting __construct methods. Unfortunately the __construct entry in the Mockery\Mock::_mockery_expectations class level variable is being removed at some point.

@timothyfisherdev

This comment has been minimized.

Copy link

timothyfisherdev commented Dec 29, 2018

@chappy84 This was more than a year ago, but just wondering if you ever found a workaround for this? I too have a similar situation where a class is depending on overloading to mock a hard dependency, and that dependency implements an interface, so it's not passing a type hint check when it's intercepted. Mine doesn't have any constructor requirements like yours though:

interface MyInterface
{
    public function doIt();
}

class MyImplementation implements MyInterface
{
    public function doIt()
    {
        echo 'did it';
    }
}

class Foo
{
    public function get() : MyInterface
    {
        return new MyImplementation();
    }

    public function go()
    {
        $this->get()->doIt();
    }
}

public function testIt()
{
    $mock = Mockery::mock('overload:' . MyImplementation::class);
    $mock->shouldReceive('doIt');

    $foo = new Foo();
    $foo->go();
}

In my case the class Foo actually extends an abstract class and the get() method is required by it.

@davedevelopment

This comment has been minimized.

Copy link
Collaborator

davedevelopment commented Jan 2, 2019

@timothyfisherdev try

$mock = Mockery::mock('overload:' . MyImplementation::class, MyInterface::class);
@chappy84

This comment has been minimized.

Copy link
Author

chappy84 commented Apr 5, 2019

The above will solve the issue in a normal overloading situation, this specific issue is around overriding the __construct method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.