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

Allow direct calling of protected methods for mocks #158

Closed
duclet opened this issue Oct 27, 2014 · 7 comments
Closed

Allow direct calling of protected methods for mocks #158

duclet opened this issue Oct 27, 2014 · 7 comments
Milestone

Comments

@duclet
Copy link

duclet commented Oct 27, 2014

We like to test our protected methods because some of them do contain logic. Now, if that method contain calls to other methods, we also want to verify that the call happened correctly. Verification though as far as I can tell only works with mock objects. We tried using PHPUnit_Extensions_Helper_AccessibleObject but it does not allow making the method public on these mocks. Can we add it, either by default or via a flag, the ability to automatically make a method public? So basically, we needed the ability to do the following (in very simple terms):

class MyClass {
    ...
    protected function doWork($raw) {
        $data = $this->processRaw($raw);
        return array('result' => $data);
    }
    ...
}

class MyClassTest {
    public function testDoWork() {
        $mock = Phake::partialMock('MyClass');
        self::assert(array('result' => 'BLAH'), $mock->doWork('blah'));

        Phake::verify($mock)->processRaw(...);
    }
}
@mlively
Copy link
Collaborator

mlively commented Oct 28, 2014

This should work just fine on master

class PhakeTest_MockedClass
{

    public function callInnerFunc()
    {
        return $this->innerFunc();
    }

    protected function innerFunc()
    {
        return 'test';
    }
}

class Test extends PHPUnit_Framework_TestCase
{
    public function testPartialMockCallsInnerMethods()
    {
        $pmock = Phake::partialMock('PhakeTest_MockedClass');
        $this->assertEquals('test', $pmock->callInnerFunc());

        Phake::verify($pmock)->innerFunc();
    }
}

This is on master branch of phake.

@duclet
Copy link
Author

duclet commented Oct 28, 2014

You are calling a public method though. I'm looking to be able to call a protected method.

@mlively
Copy link
Collaborator

mlively commented Oct 28, 2014

I see, I misread your example. I am sorry about that. I can look at providing functionality similar to etsy's extension. The problem with using etsy's extension is my generated files don't have the annotation, so you are probably getting a reflection exception. A short term solution while I sort out a work around can be to create an extension of the class in question and override the methods you want to test to be public. Extending classes can make methods more visible.

I won't go into a big diatribe about testing protected methods since I don't know your exact situation. However, I would feel bad if I didn't at least say I think you'd realize some benefit from moving these methods out into their own classes if they provide services outside of a typical abstract class setup (concrete public method calling a protected abstract method.) Either way, I know this isn't always a practical solution so I'll see about a decent work-around.

@duclet
Copy link
Author

duclet commented Oct 28, 2014

We like to break down our code into small pieces. So if there is a complicated method, rather than have it do everything, we split it out into smaller methods doing a specific task. So while yes, those smaller methods can be encapsulated via a different object, in most cases, it doesn't make much sense for us to split it out when it isn't going to be use elsewhere.

I have already written a utility class much like Etsy's that will make this work so this isn't a high priority. As mentioned though, I think it would be nice if this was built into the base framework itself since I don't see a particular reason why in the realm of unit testing, we need to follow the strict guidelines of the visibility of the methods (this wouldn't be much of an issue if PHP have the package visibility support like Java but I digress). So, my suggestion is to update the __call magic method (I'm assuming this is used somewhere) so that when a method that can't be invoke, the code simply change its visibility before calling it.

@mollierobbert
Copy link

+1 - I'd also like to see this as native Phake functionality. Right now we're using an extra layer to accomplish this, e.g. $this->createDummy(Phake::mock(SomeClass::class))->callProtectedMethod(). It ain't pretty.

@mlively mlively modified the milestone: v2.1.0 Feb 23, 2015
mlively added a commit that referenced this issue Apr 26, 2015
…sing Phake::makeVisible() and Phake::makeStaticsVisible()

Fixes #158
@mlively
Copy link
Collaborator

mlively commented Apr 26, 2015

This is now doable in the 2.1 branch. It's not really that much different from what mollierobbert posted, but it is at least code you don't have too write now. Documentation can be found here: http://phake.readthedocs.org/en/2.1/mocks.html#calling-private-and-protected-methods-on-mocks

@mlively mlively closed this as completed Apr 26, 2015
@mollierobbert
Copy link

Great work Mike! Cheers.

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

No branches or pull requests

3 participants