Added more descriptive errors when verifications fail. It will now te…

…ll you why each recorded call was marked as a failure. No more will the fine users of Phake have to resort to argument captures for debugging their verification. #144
phake · Apr 7, 2015 · 70cc682 · 70cc682
1 parent 2ada2e6
commit 70cc682
Show file tree

Hide file tree

Showing 31 changed files with 317 additions and 172 deletions.
diff --git a/composer.json b/composer.json
@@ -12,7 +12,8 @@
         }
     ],
     "require": {
-        "php": ">=5.3.3"
+        "php": ">=5.3.3",
+        "sebastian/comparator": "~1.1"
     },
     "require-dev": {
         "ext-soap": "*",

diff --git a/src/Phake.php b/src/Phake.php
@@ -423,7 +423,7 @@ public static function createPhake()
      */
     public static function equalTo($value)
     {
-        return new Phake_Matchers_EqualsMatcher($value);
+        return new Phake_Matchers_EqualsMatcher($value, new \SebastianBergmann\Comparator\Factory());
     }
 
     /**

diff --git a/src/Phake/CallRecorder/Verifier.php b/src/Phake/CallRecorder/Verifier.php
@@ -92,11 +92,22 @@ public function verifyCall(Phake_CallRecorder_CallExpectation $expectation)
             if ($call->getObject() === $expectation->getObject()) {
                 $obj_interactions = true;
                 $args             = $call->getArguments();
-                if ($matcher->matches($call->getMethod(), $args)) {
+                try
+                {
+                    $matcher->assertMatches($call->getMethod(), $args);
                     $matchedCalls[] = $this->recorder->getCallInfo($call);
                     $this->recorder->markCallVerified($call);
-                } elseif ($call->getMethod() == $expectation->getMethod()) {
-                    $methodNonMatched[] = $call->__toString();
+                }
+                catch (Phake_Exception_MethodMatcherException $e)
+                {
+                    if ($call->getMethod() == $expectation->getMethod()) {
+                        $message = $e->getMessage();
+                        if (strlen($message))
+                        {
+                            $message = "\n{$message}";
+                        }
+                        $methodNonMatched[] = $call->__toString() . $message;
+                    }
                 }
             }
         }
@@ -109,7 +120,7 @@ public function verifyCall(Phake_CallRecorder_CallExpectation $expectation)
             }
 
             if (count($methodNonMatched)) {
-                $additions .= "\nOther Invocations:\n  " . implode("\n  ", $methodNonMatched);
+                $additions .= "\nOther Invocations:\n===\n  " . implode("\n===\n  ", str_replace("\n", "\n  ", $methodNonMatched)) . "\n===";
             }
 
             return new Phake_CallRecorder_VerifierResult(

diff --git a/src/Phake/Exception/MethodMatcherException.php b/src/Phake/Exception/MethodMatcherException.php
@@ -0,0 +1,36 @@
+<?php
+
+/**
+ * Thrown when a method call doesn't match an expection
+ */
+class Phake_Exception_MethodMatcherException extends Exception
+{
+    private $argument;
+
+    /**
+     * @param string $message
+     * @param Exception $previous
+     */
+    public function __construct($message = "", Exception $previous = null)
+    {
+        parent::__construct($message, 0, $previous);
+        $this->argument = 0;
+    }
+
+    /**
+     * Updates the argument position (used in the argument chain)
+     */
+    public function incrementArgumentPosition()
+    {
+        $this->argument++;
+    }
+
+    /**
+     * Returns the argument's position (0 indexed)
+     * @return int
+     */
+    public function getArgumentPosition()
+    {
+        return $this->argument;
+    }
+}
diff --git a/src/Phake/Matchers/AnyParameters.php b/src/Phake/Matchers/AnyParameters.php
@@ -48,15 +48,12 @@
 class Phake_Matchers_AnyParameters extends  Phake_Matchers_AbstractChainableArgumentMatcher
 {
     /**
-     * Executes the matcher on a given list of argument values. Returns TRUE on a match, FALSE otherwise.
+     * Do nothing, any parameters always matches
      *
      * @param array $arguments
-     *
-     * @return boolean
      */
     public function doArgumentsMatch(array &$arguments)
     {
-        return true;
     }
 
     /**

diff --git a/src/Phake/Matchers/ArgumentCaptor.php b/src/Phake/Matchers/ArgumentCaptor.php
@@ -92,20 +92,26 @@ public function bindAllCapturedValues(&$values)
      * Will bind the argument to the variable passed to the constructor.
      *
      * @param mixed $argument
-     *
-     * @return boolean
+     * @throws Phake_Exception_MethodMatcherException
      */
     protected function matches(&$argument)
     {
         $args = array();
         $args[] =& $argument;
-        if ($this->matcher === null || $this->matcher->doArgumentsMatch($args)) {
-            $this->boundVariable = $argument;
-            $this->allCapturedValues[] = $argument;
-            return true;
-        } else {
-            return false;
+
+        if ($this->matcher !== null)
+        {
+            try
+            {
+                $this->matcher->doArgumentsMatch($args);
+            }
+            catch (Phake_Exception_MethodMatcherException $e)
+            {
+                throw new Phake_Exception_MethodMatcherException(trim("Failed in Phake::capture()->when()\n" . $e->getMessage()), $e);
+            }
         }
+        $this->boundVariable = $argument;
+        $this->allCapturedValues[] = $argument;
     }
 
     /**

diff --git a/src/Phake/Matchers/ChainedArgumentMatcher.php b/src/Phake/Matchers/ChainedArgumentMatcher.php
@@ -84,6 +84,6 @@ public function getAdaptedMatcher()
      */
     protected function matches(&$argument)
     {
-        return $this->adaptedMatcher->matches($argument);
+        $this->adaptedMatcher->matches($argument);
     }
 }
diff --git a/src/Phake/Matchers/EqualsMatcher.php b/src/Phake/Matchers/EqualsMatcher.php
@@ -54,72 +54,37 @@ class Phake_Matchers_EqualsMatcher extends Phake_Matchers_SingleArgumentMatcher
      */
     private $value;
 
+    /**\
+     * @var \SebastianBergmann\Comparator\Factory
+     */
+    private $comparatorFactory;
+
     /**
      * Pass in the value that the upcoming arguments is expected to equal.
      *
      * @param mixed $value
+     * @param \SebastianBergmann\Comparator\Factory $comparatorFactory
      */
-    public function __construct($value)
+    public function __construct($value, \SebastianBergmann\Comparator\Factory $comparatorFactory)
     {
         $this->value = $value;
+        $this->comparatorFactory = $comparatorFactory;
     }
 
     /**
      * Returns whether or not the passed argument matches the matcher.
      */
     protected  function matches(&$argument)
     {
-        return $this->compareValues($this->value, $argument);
-    }
-
-    private function compareValues($val1, $val2, &$tested = array())
-    {
-        if (is_object($val1) && is_object($val2)) {
-            return $this->compareObjects($val1, $val2, $tested);
-        } elseif (is_array($val1) && is_array($val2)) {
-            return $this->compareArrays($val1, $val2, $tested);
-        } else {
-            return $val1 == $val2;
-        }
-    }
-
-    private function compareObjects($obj1, $obj2, &$tested = array())
-    {
-        if (get_class($obj1) != get_class($obj2)) {
-            return false;
-        }
-
-        if ($obj1 === $obj2) {
-            return true;
-        }
-
-        if (in_array(array($obj1, $obj2), $tested, true)) {
-            return true;
-        }
-
-        $tested[] = array($obj1, $obj2);
-        $tested[] = array($obj2, $obj1);
-
-        return $this->compareArrays((array)$obj1, (array)$obj2, $tested);
-    }
-
-    private function compareArrays(array $arr1, array $arr2, array &$tested)
-    {
-        if (count($arr1) != count($arr2)) {
-            return false;
+        try
+        {
+            $compare = $this->comparatorFactory->getComparatorFor($this->value, $argument);
+            $compare->assertEquals($this->value, $argument);
         }
-
-        foreach ($arr1 as $key => $value) {
-            if (!array_key_exists($key, $arr2)) {
-                return false;
-            }
-
-            if (!$this->compareValues($value, $arr2[$key], $tested)) {
-                return false;
-            }
+        catch (\SebastianBergmann\Comparator\ComparisonFailure $e)
+        {
+            throw new Phake_Exception_MethodMatcherException(trim($e->getMessage() . "\n" . $e->getDiff()), $e);
         }
-
-        return true;
     }
 
     /**

diff --git a/src/Phake/Matchers/Factory.php b/src/Phake/Matchers/Factory.php
@@ -72,7 +72,7 @@ public function createMatcher($argument, Phake_Matchers_IChainableArgumentMatche
         } elseif ($argument instanceof Phake_Matchers_IArgumentMatcher) {
             $return = new Phake_Matchers_ChainedArgumentMatcher($argument);
         } else {
-            $return = new Phake_Matchers_EqualsMatcher($argument);
+            $return = new Phake_Matchers_EqualsMatcher($argument, new \SebastianBergmann\Comparator\Factory());
         }
 
         if ($nextMatcher !== null)

diff --git a/src/Phake/Matchers/HamcrestMatcherAdapter.php b/src/Phake/Matchers/HamcrestMatcherAdapter.php
@@ -66,12 +66,20 @@ public function __construct(Hamcrest\Matcher $matcher)
      * Forwards the call to Hamcrest's matches() method.
      *
      * @param mixed $argument
-     *
-     * @return boolean
+     * @throws Phake_Exception_MethodMatcherException
      */
     protected  function matches(&$argument)
     {
-        return $this->matcher->matches($argument);
+        if (!$this->matcher->matches($argument))
+        {
+            $description = new \Hamcrest\StringDescription();
+            $description->appendText("Expected ")
+                ->appendDescriptionOf($this->matcher)
+                ->appendText(' but ');
+
+            $this->matcher->describeMismatch($argument, $description);
+            throw new Phake_Exception_MethodMatcherException($description);
+        }
     }
 
     public function __toString()

diff --git a/src/Phake/Matchers/IChainableArgumentMatcher.php b/src/Phake/Matchers/IChainableArgumentMatcher.php
@@ -48,11 +48,11 @@
 interface Phake_Matchers_IChainableArgumentMatcher
 {
     /**
-     * Executes the matcher on a given list of argument values. Returns TRUE on a match, FALSE otherwise.
+     * Assert the matcher on a given list of argument values. Throws an exception if the matcher doesn't match
      *
      * @param array $arguments
      *
-     * @return boolean
+     * @throw Exception
      */
     public function doArgumentsMatch(array &$arguments);
 

diff --git a/src/Phake/Matchers/IgnoreRemainingMatcher.php b/src/Phake/Matchers/IgnoreRemainingMatcher.php
@@ -48,15 +48,12 @@
 class Phake_Matchers_IgnoreRemainingMatcher extends Phake_Matchers_AbstractChainableArgumentMatcher
 {
     /**
-     * Executes the matcher on a given list of argument values. Returns TRUE on a match, FALSE otherwise.
+     * Do nothing, ignore remaining always matches
      *
      * @param array $arguments
-     *
-     * @return boolean
      */
     public function doArgumentsMatch(array &$arguments)
     {
-        return true;
     }
 
     /**

diff --git a/src/Phake/Matchers/MethodMatcher.php b/src/Phake/Matchers/MethodMatcher.php
@@ -76,29 +76,62 @@ public function __construct($expectedMethod, Phake_Matchers_IChainableArgumentMa
      */
     public function matches($method, array &$args)
     {
-        if ($this->expectedMethod == $method
-            && $this->doArgumentsMatch($args)
-        ) {
+        try
+        {
+            $this->assertMatches($method, $args);
             return true;
-        } else {
+        }
+        catch (Phake_Exception_MethodMatcherException $e)
+        {
             return false;
         }
     }
 
     /**
-     * Determines whether or not given arguments match the argument matchers configured in the object.
+     * Asserts whether or not the given method and arguments match the configured method and argument matchers in this \
+     * object.
      *
+     * @param string $method
      * @param array $args
+     * @return bool
+     * @throws Phake_Exception_MethodMatcherException
+     */
+    public function assertMatches($method, array &$args)
+    {
+        if ($this->expectedMethod != $method)
+        {
+            throw new Phake_Exception_MethodMatcherException("Expected method {$this->expectedMethod} but received {$method}");
+        }
+
+        $this->doArgumentsMatch($args);
+    }
+
+    /**
+     * Determines whether or not given arguments match the argument matchers configured in the object.
      *
-     * @return boolean
+     * Throws an exception with a description if the arguments do not match.
+     *
+     * @param array $args
+     * @return bool
+     * @throws Phake_Exception_MethodMatcherException
      */
     private function doArgumentsMatch(array &$args)
     {
-        if ($this->argumentMatcherChain === null)
+        if ($this->argumentMatcherChain !== null)
         {
-            return count($args) == 0;
+            try
+            {
+                $this->argumentMatcherChain->doArgumentsMatch($args);
+            }
+            catch (Phake_Exception_MethodMatcherException $e)
+            {
+                $position = $e->getArgumentPosition() + 1;
+                throw new Phake_Exception_MethodMatcherException(trim("Argument #{$position} failed test\n" . $e->getMessage()), $e);
+            }
+        }
+        elseif (count($args) != 0)
+        {
+            throw new Phake_Exception_MethodMatcherException("No matchers were given to Phake::when(), but arguments were received by this method.");
         }
-
-        return $this->argumentMatcherChain->doArgumentsMatch($args);
     }
 }