sebastianbergmann / phpunit-mock-objects

Mock Object library for PHPUnit
https://phpunit.de/
Other
4.98k stars 154 forks source link

callback() for parameter called too many times #400

Closed Olsm closed 6 years ago

Olsm commented 6 years ago

I have the same issue as #261. This still occurs in PHPUnit version 6.5.5 and PHPUnit Mock Objects version 5.0.6 I noticed that in this issue you link a commit where this was fixed in version 5.0.5. Somehow I have this issue with version 5.0.6. Or did I do something incorrectly?

$this->invoiceHandMock()
    ->expects($this->exactly(8))
    ->method("saveInvoice")
    ->withConsecutive([$this->withEquals($expectedInvoice1)], [$this->withEquals($expectedInvoice1C)],
        [$this->withEquals($expectedInvoice2)], [$this->withEquals($expectedInvoice2C)],
        [$this->withEquals($expectedInvoice3)], [$this->withEquals($expectedInvoice3C)],
        [$this->withEquals($expectedInvoice4)], [$this->withEquals($expectedInvoice4C)])
    ->willReturn([]);

/**
 * Tests that actual parameter in a method equals expected.
 *
 * @param $expected
 * @return callable
 */
public function withEquals($expected) {
    return $this->callback(function ($actual) use ($expected) {
        if (is_array($expected) && is_array($actual) && sizeof($expected) == sizeof($actual)) {
            $equals = true;
            foreach ($expected as $value) {
                $equals = $equals && $this->assertContains($value, $actual);
            }
            return $equals;
        }
        return $this->assertEquals($expected, $actual);
    });
}

/**
 * Asserts that array contains an object.
 * Override assertContains to enable custom equals methods in app objects
 *
 * @param mixed $needle
 * @param mixed $haystack
 * @param string $message
 * @param bool $ignoreCase
 * @param bool $checkForObjectIdentity
 * @param bool $checkForNonObjectIdentity
 * @return bool
 * @internal param mixed $expected
 * @internal param mixed $actual
 * @internal param float $delta
 * @internal param int $maxDepth
 * @internal param bool $canonicalize
 */
public static function assertContains($needle, $haystack, $message = '', $ignoreCase = false, $checkForObjectIdentity = true, $checkForNonObjectIdentity = false) {
    if (method_exists($needle, "equals") && !empty($haystack) && method_exists(current($haystack), "equals")) {
        $contains = false;
        foreach ($haystack as $item) {
            $contains = $contains || $needle->equals($item);
        }
        if (!$contains) {
            $exporter = new SebastianBergmann\Exporter\Exporter();
            $expected = $exporter->export($needle);
            $actual = $exporter->export($haystack);
            $message = "Failed asserting that array contains object.";
            $comparisonFailure = new \SebastianBergmann\Comparator\ComparisonFailure($expected, $actual, $expected, $actual, false, $message);
            throw new \PHPUnit\Framework\ExpectationFailedException($message, $comparisonFailure);
        }
        return $contains;
    }
    parent::assertContains($needle, $haystack, $message, $ignoreCase, $checkForObjectIdentity, $checkForNonObjectIdentity);
    return true;
}

This issue is fixed with the workaround of Kherge https://github.com/sebastianbergmann/phpunit-mock-objects/issues/261#issuecomment-209479226

In my withEquals method I change callback method name to callOnce. Result is like this:

/**
 * Tests that actual parameter in a method equals expected.
 *
 * @param $expected
 * @return callable
 */
public function withEquals($expected) {
    return $this->callOnce(function ($actual) use ($expected) {
        if (is_array($expected) && is_array($actual) && sizeof($expected) == sizeof($actual)) {
            $equals = true;
            foreach ($expected as $value) {
                $equals = $equals && $this->assertContains($value, $actual);
            }
            return $equals;
        }
        return $this->assertEquals($expected, $actual);
    });
}

/**
 * Override callback to make sure its called only once
 * Fixes a bug in phpunit, temporary workaround
 * https://github.com/sebastianbergmann/phpunit-mock-objects/issues/261#issuecomment-209479226
 * @param Closure $closure
 * @return callable
 */
private function callOnce(Closure $closure) {
    return $this->callback(
        function (...$arguments) use (&$closure) {
            static $invoked = false;

            if (!$invoked) {
                $invoked = true;

                return $closure(...$arguments);
            }

            return true;
        }
    );
}
stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had activity within the last 60 days. It will be closed after 7 days if no further activity occurs. Thank you for your contributions.

stale[bot] commented 6 years ago

This issue has been automatically closed because it has not had activity since it was marked as stale. Thank you for your contributions.

leonexcc commented 6 years ago

I have the same problem and looking at #261 and #311, it seems to be only fixed for PHPUnit_Framework_MockObject_Matcher_Parameters not for PHPUnit_Framework_MockObject_Matcher_ConsecutiveParameters.

ConsecutiveParameters also checks all old invocations again on verify():

https://github.com/sebastianbergmann/phpunit-mock-objects/blob/f9756fd4f43f014cb2dca98deeaaa8ce5500a36e/src/Matcher/ConsecutiveParameters.php#L84

If any parameter changed since the original invocation the verification will fail.