Skip to content

Support for omitting parameter default values for willReturnMap() #5551

Description

@Schrank

There are at least two ways to tell a test double to return a value: with()->willReturn() and willReturnMap().

While with()->willReturn() kind of works as I expect it, willReturnMap() does not.

with()->willReturn() seems to "ignore" the default values and returns the willReturn() although the default values are not part of the test double configuration.

Example:

// We have a method like this:
public function methodWithDefaultValue(int $return, int $subtract = 0): int
{
        return $return - $subtract;
}

The following code will work as expected and be a valid test and $this->methodWithDefaultValue($a) will return 6.

 $a = 6;

$this->exampleMock
    ->method('methodWithDefaultValue')
    ->with($a)
    ->willReturn($a);

While this does not work:

        $a = 6;

        $this->exampleMock
            ->method('methodWithDefaultValue')
            ->willReturnMap([[$a, $a]]);

It returns an error TypeError: MockObject_Example_5ee46b65::methodWithDefaultValue(): Return value must be of type int, null returned

And this is already more than expected, because in the horrible past, the error would be: Failed asserting that null is identical to 6. if we don't define return types on the methods.

My situation is: I'm writing not enough unit tests, therefore I forget the problem often. And in combination with PHPStorm not autocompleting a method with its optional parameters, I often forget, that the method I double even has more than the required parameter.

To make willReturnMap() work, we need to pass all parameters to the mapping array, looking like this:

$this->exampleMock
            ->method('methodWithDefaultValue')
            ->willReturnMap([[$a, 0, $a]]);

then the code will return our expected 6.

After a nice chat with Sebastian, we thought about how to fix it and there are two location, where it might make sense to do it:

  1. When creating the ReturnValueMap or
  2. when matching against it.

Sebastian already said, 1. makes more sense, because then we know we have to handle default values, because the others are passed to us. While if we do it while matching, we first need to determine wether the values we have are default or not and over all, Sebastian said it is more complex.

@sebastianbergmann I hope I didn't forget anything. The good news: with()->willReturn() works as expected <3

Thanks for all your work!

Activity

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

Metadata

Metadata

Labels

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions