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

suggested fix for issue #1785: overloading and __set bug #1800

Merged
merged 6 commits into from Apr 2, 2015

Conversation

EVODelavega
Copy link

The Stub::* methods all tend to set a __mocked property on the mocked classes. When attempting to mock a class that implements a __set method that doesn't allow (as PHP calls it) "overloading", this results in a PHPUnit exception.

This PR addresses this issue by checking the class for an existing __set method. If such a method exists, the __mocked flag is not set.
This check is also used when attempting to add a non-existent property to classes with a __set method. These properties will be added, but a PHPUnit_Framework_Exception will be thrown if this fails.

The __mocked flag was primarily (only?) used in the Stub::update call (to change the mock's behavior on the fly). the __mocked property check has been replaced with an instanceof PHPUnit_Framework_MockObject_MockObject check. If this check fails, the behavior remains unchanged: calling Stub::update on anything that is not an instance of the MockObject class cause a LogicException to be thrown.

Elias Van Ootegem added 3 commits March 25, 2015 15:16
…hen a non-existing property is being added to the mock + use instanceof instead of __mocked check when update is called
@EVODelavega EVODelavega changed the title issue #1785 overloading and __set bug suggested fix for issue #1785: overloading and __set bug Mar 25, 2015
@DavertMik
Copy link
Member

Thanks. This looks like a really serious work done to fix this issue. Probably we can remove __mocked at all. Anyway, I'm really glad to get this merged

DavertMik added a commit that referenced this pull request Apr 2, 2015
suggested fix for issue #1785: overloading and __set bug
@DavertMik DavertMik merged commit 64628cc into Codeception:2.0 Apr 2, 2015
@DavertMik
Copy link
Member

Actually the second reason to implement __mocked was to show mocked class name in console output. Probably this can be improved as well
https://github.com/Codeception/Codeception/blob/2.0/src%2FCodeception%2FStep.php#L69

@DavertMik DavertMik added this to the 2.0.12 milestone Apr 2, 2015
@EVODelavega
Copy link
Author

Happy to see my PR merged, If I find the time, I'll set about removing the __mocked property entirely, and look into an alternative approach to display the mocked class name (which probably won't be that hard of a thing to do, there's probably some method on the PHPUnit_Framework_MockObject_MockObject instance I could use, or keep a reference to the ReflectionClass instance somewhere. If I get 'round too it, I'll make a PR.

PS: I think that, having merged this PR, you can close issue #1785 too

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

Successfully merging this pull request may close these issues.

None yet

2 participants