Allow direct calling of protected methods for mocks #158

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

Projects

None yet

3 participants

@duclet
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
Owner
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
duclet commented Oct 28, 2014

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

@mlively
Owner
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
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

+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 added the Enhancement label Feb 23, 2015
@mlively mlively modified the milestone: v2.1.0 Feb 23, 2015
@mlively mlively added a commit that referenced this issue Apr 26, 2015
@mlively Allowing the ability to call protected and private methods on mocks u…
…sing Phake::makeVisible() and Phake::makeStaticsVisible()

Fixes #158
aaeca1e
@mlively
Owner
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 Apr 26, 2015
@mollierobbert

Great work Mike! Cheers.

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