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

Static methods of a partial mock return NULL #209

Open
mollierobbert opened this issue Dec 11, 2015 · 12 comments
Open

Static methods of a partial mock return NULL #209

mollierobbert opened this issue Dec 11, 2015 · 12 comments
Labels

Comments

@mollierobbert
Copy link

Static methods of a partial mock always return NULL, unless a Phake::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 a Phake_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.

@mollierobbert
Copy link
Author

Any news on this, @mlively? Should I submit a pull request for it?

@mlively
Copy link
Collaborator

mlively commented Jun 21, 2016

Sorry, this ticket actually revealed some fairy serious issues with the
entire concept of how static mocking works that I have to sort through.
Static mocks can't easily be repeatedly mocked without busting the caching
which then results in fairly hefty memory usage. The reason why you are
having difficulty is because you really only get 1 shot to set the default
answer for a static mock. That's the fundamental thing that has to be fixed
and I've been tied up with some other projects.

You are more than welcome to take a crack at it and I can make sure to take
time to review. It is a bit more involved then I would expect anyone to
offer their free time for, but I would gladly accept and appreciate any
help you'd offer :).

On Mon, Jun 20, 2016 at 6:24 AM, Robbert notifications@github.com wrote:

Any news on this, @mlively https://github.com/mlively? Should I submit
a pull request for it?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#209 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAKwFlDn578ei_xFY3dnJ1KbFqgE7ZR2ks5qNpSEgaJpZM4Gz9KZ
.

@theilig
Copy link

theilig commented Nov 7, 2019

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.

fkalinski pushed a commit to fkalinski/phake that referenced this issue Nov 25, 2021
@fkalinski
Copy link

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.

@adoy
Copy link
Member

adoy commented Nov 26, 2021

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

@martinbutt
Copy link

@adoy looks like the pull request is open. #305

Any update on merging it in?

@adoy
Copy link
Member

adoy commented Jan 13, 2022 via email

@martinbutt
Copy link

@adoy I don't see any comments on the PR.

@adoy
Copy link
Member

adoy commented Jan 14, 2022

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 wonder if there is a possible solution to reduce the number of generated classes.

@martinbutt
Copy link

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:
Memory allocated: 3939384
Memory used: 4194304

After:
Memory allocated: 20497912
Memory used: 20971520

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.

@adoy
Copy link
Member

adoy commented Jan 14, 2022

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.

@martinbutt
Copy link

Without storing the objects:

Peak memory before PR: : 4194304
Peak memory after PR: 17987256

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

No branches or pull requests

6 participants