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

shouldIgnoreMissing() not working on instance mocks #177

Closed
david0 opened this issue Jun 17, 2013 · 10 comments
Closed

shouldIgnoreMissing() not working on instance mocks #177

david0 opened this issue Jun 17, 2013 · 10 comments
Labels
Feature Request request a new feature

Comments

@david0
Copy link
Contributor

david0 commented Jun 17, 2013

shouldIgnoreMissing() does not work on mock objects created using instance mocks.

$m = m::mock('overload:A');
$m->shouldIgnoreMissing();

$a = new A();
$a->doSomething();

leads to
BadMethodCallException: Method ::doSomething() does not exist on this mock object

I think it this function would be pretty useful, especially for instance mock objects. Also it looks a little inconsistent, since one can set expectations using shouldReceive on all instances, but shouldIgnoreMissing only affects the current mock object.

@aik099
Copy link

aik099 commented Jun 17, 2013

Are you proposing to throw an exception in case if call is made to a method, that is undefined in mocked class?

If so, then this can be already done via global configuration: https://github.com/padraic/mockery#mockery-global-configuration

@david0
Copy link
Contributor Author

david0 commented Jun 17, 2013

@aik099 I think i did not clearly make my point. I think, a mock which has been created using overload affects all new instances of the overloaded class. My point is, in the example above, an exception is thrown. I would expect that the doSomething()-call would be successful and return null.

@davedevelopment
Copy link
Collaborator

I think for consistency, you're probably right, but it's not drastically broken now, so I'll tag this as a feature request and I'll try and get on to it at some point.

@igorsantos07
Copy link

I also got to this... First time trying mocks in unit tests and I found the behaviour quite inconsistent, since the documentation tells about partial mocks and instance mocks, but it's not said they don't work together.

It does look like something is broken since the methods shouldIgnoreMissing, shouldDeferMissing and makePartial are documented but don't work as expected in some cases (like instance mocks).

@robertbasic
Copy link
Collaborator

shouldIgnoreMissing and shouldDeferMissing (and other such flags present and future) do work, but not on the mock, but on the instance mock.

Sounds confusing, but it's because of internals.

When you do a m::mock('overload:Foo'); mockery goes and creates a new class called Foo based on the Mock.php "template". And that template has the _mockery_ignoreMissing flag set to false by default.

Once the mock method gets to the end, it creates an overloaded Foo object and returns it to us. The overloaded Foo class is also available to instantiate new Foo objects.

Calling $m->shouldIgnoreMissing() sets the ignore missing flag on the object that was returned by the mock method.

Calling new Foo() creates a new instance of the Foo object that has the Mock.php templates default of false for _mockery_ignoreMissing.

If you want the flag set to true on the new instance, you need to call shouldIgnoreMissing() on that instance, and not on $m.

Obviously this is far from perfect.

One idea I have is to have a global configuration for this, I have a proof of concept in this commit robertbasic@32d70b7

Thoughts?

/ping @padraic

@padraic
Copy link
Member

padraic commented Feb 13, 2015

@davedevelopment Thoughts?

See how well I can delegate? 😆

@davedevelopment
Copy link
Collaborator

When you do a new ClassThatHasBeenOverloaded, we have a miracle constructor that copies the expectations over from the master object, we should probably copy over the relevant flags as well?

@robertbasic
Copy link
Collaborator

Hm I was looking in there, but didn't see anything useful. Will try again that then.

@davedevelopment
Copy link
Collaborator

This is a complete hack and I haven't tested it, but should work in theory

diff --git a/library/Mockery/Generator/StringManipulation/Pass/InstanceMockPass.php b/library/Mockery/Generator/StringManipulation/Pass/InstanceMockPass.php
index ec370f3..e3bbf7d 100644
--- a/library/Mockery/Generator/StringManipulation/Pass/InstanceMockPass.php
+++ b/library/Mockery/Generator/StringManipulation/Pass/InstanceMockPass.php
@@ -14,6 +14,13 @@ class InstanceMockPass
     {
         \$this->_mockery_ignoreVerification = false;
         \$associatedRealObject = \Mockery::fetchMock(__CLASS__);
+
+        foreach (get_object_vars(\$this) as \$attr => \$val) {
+            if (\$attr !== "_mockery_ignoreVerification" && \$attr !== "_mockery_expectations") {
+                \$this->\$attr = \$associatedRealObject->\$attr;
+            }
+        }
+
         \$directors = \$associatedRealObject->mockery_getExpectations();
         foreach (\$directors as \$method=>\$director) {
             \$expectations = \$director->getExpectations();

@robertbasic
Copy link
Collaborator

@davedevelopment pull request was merged, ticket can be closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request request a new feature
Projects
None yet
Development

No branches or pull requests

6 participants