Stub yields error when unexpected method call is made

[sebastianbergmann]

I have a class named Foo which has two methods bar() and baz():

<?php
class Foo
{
    public function bar()
    {
    }

    public function baz()
    {
    }
}

When I create a stub for the Foo class, set up the bar() method to return a specific value, ...

<?php
class Test extends PHPUnit_Framework_TestCase
{
    public function testOne()
    {
        $foo = $this->prophesize(Foo::class);
        $foo->bar()->willReturn('...');

        $stub = $foo->reveal();

        $stub->bar();
        $stub->baz();
    }
}

... and (also) call another method on the stub then I get the following error:

phpunit Test
PHPUnit 4.6.4-2-gbc3ce9c by Sebastian Bergmann and contributors.

E

Time: 56 ms, Memory: 5.75Mb

There was 1 error:

1) Test::testOne
Prophecy\Exception\Call\UnexpectedCallException: Method call:
  - baz()
on Double\Foo\P1 was not expected, expected calls were:
  - bar()

/home/sb/Test.php:23

FAILURES!
Tests: 1, Assertions: 0, Errors: 1.

Not sure whether this is a bug or just a misunderstanding on my part. I expect a stub not to care about method calls for which no behavior is configured.

[sebastianbergmann]

Don't know why I did not see this earlier but I found the

"As a matter of fact, after you define your first promise (method call), Prophecy will force you to define all the communications - it throws the UnexpectedCallException for any call you didn't describe with object prophecy before calling it on a stub."

section in the documentation now. So Prophecy's behavior is expected and intentional (TBH, I did not expect this to be a bug).

However, this conflicts with my understanding of a stub and I need to figure out how to resolve that conflict. IMO, a stub should simply ignore method calls that have no configured behavior.

[stof]

@sebastianbergmann see https://github.com/phpspec/prophecy/issues/23#issuecomment-18411542

[everzet]

@sebastianbergmann that is a tricky one :) The reasoning is the exact one @stof provided. Idea is that Prophecy does some things purely from the perspective of promoting better design. Based on my experience this particular feature prevents more problems than it creates - every single time I got in the case where the call been made that my stub didn't expect, the solution was to refactor and the outcome was objectively better design.

[everzet]

That said, I'm quite open if you could come up with an example where this feature doesn't lead to better design, but only makes things worse.

[lieut-data]

I like this behaviour, but too found it initially confusing. Though, I'm still confused about why I would need to bother using shouldBeCalled() if I'm forced to define the communication anyway.

[sebastianbergmann]

It is even more confusing because it is not consistent. When I create a stub for the Foo class, set up the bar() method to return a specific value, and do not call the bar() method ...

<?php
class Test extends PHPUnit_Framework_TestCase
{
    public function testOne()
    {
        $foo = $this->prophesize(Foo::class);
        $foo->bar()->willReturn('...');

        $stub = $foo->reveal();
    }
}

... then no error is yielded:

PHPUnit 4.6.4-2-gbc3ce9c by Sebastian Bergmann and contributors.

.

Time: 95 ms, Memory: 5.00Mb

OK (1 test, 0 assertions)

[stof]

@sebastianbergmann Defining ->willReturn does not mean that the method must be called (this is what shouldBeCalled() is about).

[dontub]

I also stumbled upon this. What about treating stubs as dummies if there's no method prophecy and only throw the UnexpectedCallException for mocks missing a matching method prophecy? Probably this behaviour should be made optional for backward compatibility.

[issei-m]

Also this fact seems to be against the following explanation of shouldHaveBeenCalled in the document:

$em = $prophet->prophesize('Doctrine\ORM\EntityManager');

$controller->createUser($em->reveal());

$em->flush()->shouldHaveBeenCalled();

This doesn't work well because flush method has not been expected to be get called, so UnexpectedCallException will be thrown. But it does work well if you $em->flush()->shouldBeCalled() before you do $controller->createUser($em->reveal()); but this is a bit verbose to me.