sebastianbergmann/phpunit-mock-objects

Mock expectation changes when the expectation is met

mvscheidt opened this issue · 4 comments

Consider the following example (which is included in testcase.tar.gz - a simple composer install should be enough to set it up and vendor/bin/phpunit to run it)

    protected function setUp()
    {
        $this->bazMock = $this->getMockBuilder(Baz::class)->getMock();
        $this->bazMock->method("output")->with("world")->willReturn("world");

        $this->bar = new Bar($this->bazMock);
    }

    public function testWorksAsExpected()
    {
        $this->assertEquals("Output: world", $this->bar->hello("world"));
    }

    public function testOutputsCorrectExpectationErrorIfExpectationFails()
    {
        //this works as expected - we expected "hello" and pass "world" - the error message describes this mistake properly
        $this->bazMock->method("output")->with("hello")->willReturn("hello");

        $this->assertEquals("Output: world", $this->bar->hello("world"));
    }

    public function testOutputsWrongExpectationErrorIfExpectationSucceeds()
    {
        //this is where thinks get "wonky" - we expect "hello" and pass "hello", but now that the expectation should succeed, phpunit randomly uses the former expectation "world" from setUp()
        //I would expect this to either: succeed OR a proper error message (violates best practices, or whatever is the reason for this behaving unintuitive)
        $this->bazMock->method("output")->with("hello")->willReturn("hello");

        $this->assertEquals("Output: hello", $this->bar->hello("hello"));
    }
  • setUp has the base setup of the mock object including a method expectation.
  • testOutputsCorrectExpectationErrorIfExpectationFails works as expected (fails)
    • by that I mean phpunit detects the unexpected value and outputs the expected one
  • testOutputsWrongExpectationErrorIfExpectationSucceeds does not work as expected (fails aswell)
    • by that I mean phpunit (seemingly) detects the expected value and then decides to test against the expected value we defined in setUp. This is also evident if you look at the error message - suddenly the --- Expected value is different even though both tests use $this->bazMock->method("output")->with("hello")->willReturn("hello");

Using createMock instead of getMockBuilder makes no difference. Using Matchers, e.g. $this-at(0) does not change the outcome either.

I would expect the expectation inside testOutputsWrongExpectationErrorIfExpectationSucceeds to succeed. If this fails by design then the error message should reflect this


Example Output when executing the testcase.

1) Repro\Tests\BarTest::testOutputsCorrectExpectationErrorIfExpectationFails
Expectation failed for method name is equal to <string:output> when invoked zero or more times
Parameter 0 for invocation Repro\Baz::output('world'): string does not match expected value.
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'hello'
+'world'

2) Repro\Tests\BarTest::testOutputsWrongExpectationErrorIfExpectationSucceeds
Expectation failed for method name is equal to <string:output> when invoked zero or more times
Parameter 0 for invocation Repro\Baz::output('hello'): string does not match expected value.
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'world'
+'hello'

Isses that may be related in some way:

Each test method is executed on a fresh instance of the test class, so each $this->bazMock is a fresh test double object. This is expected and will not be changed.

Yes, that is clear - I don't get the connection to the issue though, because $this->bazMock is created only once for each test method (in the setUp) already. I don't want to share data between tests.

This issue was about changing the (default) expectation defined in setUp to something else for certain test methods.

I should've probably used an example like this

<?php
declare(strict_types=1);

namespace Repro\Tests;

use Repro\Bar;
use Repro\Baz;
use PHPUnit\Framework\TestCase;

class Bar2Test extends TestCase
{
    public function testSomething()
    {
        $bazMock = $this->getMockBuilder(Baz::class)->getMock();
        $bazMock->method("output")->with("hello")->willReturn("hello");
        $bazMock->method("output")->with("world")->willReturn("world");

        $bar = new Bar($bazMock);

        $this->assertEquals("Output: world", $bar->hello("world"));
    }
}

This I would expect to succeed (without knowing any specifics about how phpunit works internally when trying to "reconfigure" method expectations) - instead it fails with

1) Repro\Tests\Bar2Test::testWorksAsExpected
Expectation failed for method name is equal to <string:output> when invoked zero or more times
Parameter 0 for invocation Repro\Baz::output('world'): string does not match expected value.
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'hello'
+'world'

Please note what the --- Expected value here is - hello

Now If you change the $bar->hello("world") to $bar->hello("something else") it will output

1) Repro\Tests\Bar2Test::testWorksAsExpected
Expectation failed for method name is equal to <string:output> when invoked zero or more times
Parameter 0 for invocation Repro\Baz::output('something else'): string does not match expected value.
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'world'
+'something else'

Now the line --- Expected is saying something different - namely world. Why did the expectation change?

I already thought that maybe it now expects two calls to output but that is not the case either because
this

        $this->assertEquals("Output: world", $bar->hello("hello"));
        $this->assertEquals("Output: world", $bar->hello("world"));

still causes

1) Repro\Tests\Bar2Test::testSomething
Expectation failed for method name is equal to <string:output> when invoked zero or more times
Parameter 0 for invocation Repro\Baz::output('hello'): string does not match expected value.
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'world'
+'hello'

So it seems that the method expectation was reconfigured (in some way) to from expecting hello to expecting world - but once I actually pass world it suddenly expects hello again.

Or there is some edge case here, which I had not seen in the documentation.

Re: "This issue was about changing the (default) expectation defined in setUp to something else for certain test methods."

That also works as intended and cannot, even if I wanted, be changed for backward compatibility reasons.

Fair enough - just wanted to report it in case it was a bug.