Missing method argument in MockBuilder for getMockForAbstractClass #93

Closed
qubit05 opened this Issue Aug 10, 2012 · 3 comments

Comments

Projects
None yet
4 participants
@qubit05

qubit05 commented Aug 10, 2012

Environment:
Mac OS X Lion
Zend Server CE 5.1.0
PHP 5.3.5

Versions:
PHPUnit 3.6.12
PHPUnit_MockObject 1.1.1

Description:
When creating a mock object for an abstract class using the Mock Builder, the $methods argument isn't passed along.

Steps to reproduce:

<?php
abstract class AbstractToString {
    public function __toString() {
        return 'foo';
    }
}
class AbstractToStringTest extends PHPUnit_Framework_TestCase {
    public function test__toString() {
        $mock = $this->getMockBuilder('AbstractToString')
            ->setMethods(array('__toString'))
            ->getMockForAbstractClass();
        $mock->expects($this->any())
            ->method('__toString')
            ->will($this->returnValue('bar'));
        $this->assertEquals('bar', (string)$mock);
    }
}

Expected output:

OK (1 test, 2 assertions)

Actual output:

Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'bar'
+'foo'

Needed change:
PHPUnit_Framework_MockObject_MockBuilder::getMockForAbstractClass (line 134)
Is currently -

        return $this->testCase->getMockForAbstractClass(
          $this->className,
          $this->constructorArgs,
          $this->mockClassName,
          $this->originalConstructor,
          $this->originalClone,
          $this->autoload
        );

Should be -

        return $this->testCase->getMockForAbstractClass(
          $this->className,
          $this->constructorArgs,
          $this->mockClassName,
          $this->originalConstructor,
          $this->originalClone,
          $this->autoload,
          $this->methods
        );
@rgazelot

This comment has been minimized.

Show comment
Hide comment
@rgazelot

rgazelot Nov 13, 2012

Please add this information in the documentation and the PHPUnit API.

Please add this information in the documentation and the PHPUnit API.

@dharkness

This comment has been minimized.

Show comment
Hide comment
@dharkness

dharkness Nov 13, 2012

Is there any reason not to add all abstract methods when building a mock for a class that has at least one? In my base test case class I override getMock to merge in all abstract methods to get around the above problem. Since you cannot instantiate a class with any abstract methods, it seems like there's no need for getMockForAbstractClass at all. Just mock all abstract methods no matter what.

I suppose you can avoid instantiation by not calling the original constructor and thus deserializing the object into existence. However, it just seems easier and more correct to include all abstract methods. It's worked for us for over two years now. I wonder if I'm just missing something.

Is there any reason not to add all abstract methods when building a mock for a class that has at least one? In my base test case class I override getMock to merge in all abstract methods to get around the above problem. Since you cannot instantiate a class with any abstract methods, it seems like there's no need for getMockForAbstractClass at all. Just mock all abstract methods no matter what.

I suppose you can avoid instantiation by not calling the original constructor and thus deserializing the object into existence. However, it just seems easier and more correct to include all abstract methods. It's worked for us for over two years now. I wonder if I'm just missing something.

@edorian

This comment has been minimized.

Show comment
Hide comment
@edorian

edorian Nov 24, 2012

Collaborator

This issue was fixed with PHPUnit 3.7 (possibly before, but it works with that for sure).

Sorry for not getting back to you earlier.

@dharkness I never had a use case for "getMockForAbstractClass" except for it being more specific. All in all I don't use it (as I mostly don't test or mock abstract classes) but the different default behavior makes sense in a couple of cases

Collaborator

edorian commented Nov 24, 2012

This issue was fixed with PHPUnit 3.7 (possibly before, but it works with that for sure).

Sorry for not getting back to you earlier.

@dharkness I never had a use case for "getMockForAbstractClass" except for it being more specific. All in all I don't use it (as I mostly don't test or mock abstract classes) but the different default behavior makes sense in a couple of cases

@edorian edorian closed this Nov 24, 2012

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