-
Notifications
You must be signed in to change notification settings - Fork 87
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
Static methods of a partial mock return NULL #209
Comments
Any news on this, @mlively? Should I submit a pull request for it? |
Sorry, this ticket actually revealed some fairy serious issues with the You are more than welcome to take a crack at it and I can make sure to take On Mon, Jun 20, 2016 at 6:24 AM, Robbert [email protected] wrote:
|
I forked 2.1.1 and made a change which would allow this to work (basically set a default answer for all static methods of thenCallParent when creating the class). It seems to work in 3.1.6 as well, so when I get a chance I'll put my change up as a sort of Request for Comments. It's not an ideal solution though. |
Hi, I've submitted a fix, that makes partialMock call parent for static methods and disables class caching for partial mocks. It works for me, but may be not optimal, as generates a new class for each partial mock of given base class. |
Hi :-) Could you please submit a pull request ? I see that you created a fix on your repo but you need to submit a pull request. Thanks |
I added some comments on the code review. I have some concerns with memory usage with the crurent implementation.
|
@adoy I don't see any comments on the PR. |
My concern with this PR is that with the proposed implementation, every time a partialMock is created, a new class is declared which increase the memory usage and might be an issue. |
I wrote a quick script to see if there was any noticeable difference in the memory usage (https://gist.github.com/martinbutt/629293e2243675ab166ee2049c70e43f). Running it against both the old code and the new code shows an increase in memory usage: Before: After: In practice with my test suite this is not noticeable, as the mocks are destroyed after every test. However, if the test suite had not been written that way, it could cause memory bloat. |
The problem is that it's a class definition, not a class instance. And PHP will only free this memory when the "request" is finished. So even if your code is super clean, you will still have memory increasing each time you mock an object. Can you provide the memory usage if you remove the object storing each instance ? If i'm not wrong the amount of memory beetween the before and after usecase should be approximativaly the same. |
Without storing the objects: Peak memory before PR: : 4194304 |
Static methods of a partial mock always return
NULL
, unless aPhake::when($partial_mock)->staticMethod()->thenCallParent()
is explicitly defined. I don't think this is expected behaviour - a partial mock should always call the parent by default, right?In
Phake_ClassGenerator_MockClass::generate()
, the static mock info is always defined with aPhake_Stubber_Answers_NoAnswer
as default answer.In
Phake_Facade::mock()
, if we pass on the$defaultAnswer
variable to the$mockGenerator->generate
call, the issue is fixed.The text was updated successfully, but these errors were encountered: