phpspec / prophecy

Highly opinionated mocking framework for PHP 5.3+
MIT License
8.53k stars 241 forks source link

Using argument callback in combination with willReturn does not work #593

Closed ChristianVermeulen closed 1 year ago

ChristianVermeulen commented 1 year ago

I'm writing a test which uses the Argument::that() callback and then sets a willReturn. However, the willReturn is somehow ignored. The callback works and is called (validated with a var_dump()) and when I use Argument::type() the willReturn() does work. So it seems to specifically be in combination with an argument callback.

$eventBus
    ->dispatch(Argument::that(function(MemberAdded $event){
        $this->assertSame(IdBuilder::ID, $event->id);
        $this->assertSame(CompanyIdBuilder::ID, $event->companyId);
        $this->assertSame(ProjectIdBuilder::ID, $event->projectId);
        $this->assertSame(EmployeeIdBuilder::EMPLOYEE_ID, $event->employeeId);
        $this->assertSame(FirstNameBuilder::FIRST_NAME, $event->firstName);
        $this->assertSame(LastNameBuilder::LAST_NAME, $event->lastName);
        $this->assertTrue($event->isOwner);
        var_dump('test');
    }))
    ->willReturn(new Envelope($eventBus))
    ->shouldBeCalledOnce()
;

Result (notice the var dump showing up and the stub returning null instead of my willReturn):

Testing started at 09:33 ...
PHPUnit 9.6.3 by Sebastian Bergmann and contributors.

Testing /application/tests/ProjectManagement/Application/Command/Member
string(4) "test"

TypeError : Double\MessageBusInterface\P1::dispatch(): Return value must be of type Symfony\Component\Messenger\Envelope, null returned

But without the callback:

$eventBus
    ->dispatch(Argument::type(MemberAdded::class))
    ->willReturn(new Envelope($eventBus))
    ->shouldBeCalledOnce()
;

Result:

Testing started at 09:34 ...
PHPUnit 9.6.3 by Sebastian Bergmann and contributors.

Testing /application/tests/ProjectManagement/Application/Command/Member

Time: 00:00.389, Memory: 16.00 MB

OK (1 test, 7 assertions)
stof commented 1 year ago

The callback of Argument::that must return a boolean indicating whether the argument matched the expectation. When it returns a falsy value, we consider that this was not a match, and we'll try the next signature being registered.

ChristianVermeulen commented 1 year ago

@stof Ah I see. That fixes it indeed. That was... Unexpected...😅 Thanks for the quick reply!

stof commented 1 year ago

Note that those assertSame are a bad idea in that callback (unless you catch their failure exception to return false), as they would break cases where the callback would not match the argument and another call definition should be used.

ChristianVermeulen commented 1 year ago

@stof

Note that those assertSame are a bad idea in that callback (unless you catch their failure exception to return false), as they would break cases where the callback would not match the argument and another call definition should be used.

In that case I would probably do a type check first and early return false before doing any assertions so it can move on to the next callback token. If for some reason we do add an extra call without updating the test, either the assertions will fail or the callback's typehinted parameter will break due to the wrong type of object being injected. Either way it will have been caught I guess. :-)

stof commented 1 year ago

but those callbacks are not meant to throw. Otherwise, you cannot configure a second allow call (as the first candidate call would throw when attempting to match it).

ChristianVermeulen commented 1 year ago

but those callbacks are not meant to throw. Otherwise, you cannot configure a second allow call (as the first candidate call would throw when attempting to match it).

That's exactly what I want... Because if I add something now to my class without updating the test, I want the test to fail so I know I need to update the test for the new code... In which case I will add that second call and provide both with early false returns so they are skipped when not applicable.


$eventBus
    ->dispatch(Argument::that(function($event){
        if ($event instanceof MemberAdded === false) {
            return false;
        }

        $this->assertSame(IdBuilder::ID, $event->id);
        // ...

        return true;
    }))
    ->willReturn(new Envelope($eventBus))
    ->shouldBeCalledOnce()
;

$eventBus
    ->dispatch(Argument::that(function($event){
        if ($event instanceof MemberUpdated === false) {
            return false;
        }

        $this->assertSame(IdBuilder::ID, $event->id);
        // ...

        return true;
    }))
    ->willReturn(new Envelope($eventBus))
    ->shouldBeCalledOnce()

Or am I missing something here?

stof commented 1 year ago

Unexpected calls are already making Prophecy trigger a failure. But your checks based on assertSame throwing forbid you to have multiple expected calls with MemberUpdated objects for different ids for instance.