PHPLIB-459: Support better failure descriptions in DocumentsMatchConstraint

alcaeus · alcaeus · commit 5d9e3e295f24 · 2019-08-14T11:17:15.000+02:00
diff --git a/composer.json b/composer.json
@@ -16,7 +16,8 @@
         "ext-mongodb": "^1.6"
     },
     "require-dev": {
-        "phpunit/phpunit": "^5.7.27 || ^6.4"
+        "phpunit/phpunit": "^5.7.27 || ^6.4",
+        "sebastian/comparator": "^1.0 || ^2.0 || ^3.0"
     },
     "autoload": {
         "psr-4": { "MongoDB\\": "src/" },
diff --git a/tests/SpecTests/DocumentsMatchConstraint.php b/tests/SpecTests/DocumentsMatchConstraint.php
@@ -8,6 +8,8 @@
 use ArrayObject;
 use InvalidArgumentException;
 use RuntimeException;
+use SebastianBergmann\Comparator\ComparisonFailure;
+use SebastianBergmann\Comparator\Factory;
 use stdClass;
 
 /**
@@ -28,6 +30,12 @@ class DocumentsMatchConstraint extends Constraint
     private $sortKeys = false;
     private $value;
 
+    /** @var ComparisonFailure|null */
+    private $lastFailure;
+
+    /** @var Factory */
+    private $comparatorFactory;
+
     /**
      * Creates a new constraint.
      *
@@ -43,26 +51,19 @@ public function __construct($value, $ignoreExtraKeysInRoot = false, $ignoreExtra
         $this->ignoreExtraKeysInRoot = $ignoreExtraKeysInRoot;
         $this->ignoreExtraKeysInEmbedded = $ignoreExtraKeysInEmbedded;
         $this->placeholders = $placeholders;
+        $this->comparatorFactory = Factory::getInstance();
     }
 
-    /**
-     * Returns a string representation of the constraint.
-     *
-     * @return string
-     */
-    public function toString()
+    protected function additionalFailureDescription($other)
     {
-        return 'matches ' . json_encode($this->value);
+        if ($this->lastFailure === null) {
+            return '';
+        }
+
+        return $this->lastFailure->getMessage();
     }
 
-    /**
-     * Evaluates the constraint for parameter $other. Returns true if the
-     * constraint is met, false otherwise.
-     *
-     * @param mixed $other
-     * @return boolean
-     */
-    protected function matches($other)
+    public function evaluate($other, $description = '', $returnResult = false)
     {
         /* TODO: If ignoreExtraKeys and sortKeys are both false, then we may be
          * able to skip preparation, convert both documents to extended JSON,
@@ -73,13 +74,45 @@ protected function matches($other)
          * in all documents and sub-documents. */
         $other = $this->prepareBSON($other, true, $this->sortKeys);
 
+        $success = false;
+        $this->lastFailure = null;
+
         try {
             $this->assertEquals($this->value, $other, $this->ignoreExtraKeysInRoot);
+            $success = true;
         } catch (RuntimeException $e) {
-            return false;
+            $this->lastFailure = new ComparisonFailure(
+                $this->value,
+                $other,
+                $this->exporter->export($this->value),
+                $this->exporter->export($other),
+                false,
+                $e->getMessage()
+            );
         }
 
-        return true;
+        if ($returnResult) {
+            return $success;
+        }
+
+        if (!$success) {
+            $this->fail($other, $description, $this->lastFailure);
+        }
+    }
+
+    protected function failureDescription($other)
+    {
+        return 'two BSON objects are equal';
+    }
+
+    /**
+     * Returns a string representation of the constraint.
+     *
+     * @return string
+     */
+    public function toString()
+    {
+        return 'matches ' . $this->exporter->export($this->value);
     }
 
     /**
@@ -88,19 +121,24 @@ protected function matches($other)
      * @param ArrayObject $expected
      * @param ArrayObject $actual
      * @param boolean     $ignoreExtraKeys
+     * @param string      $keyPrefix
      * @throws RuntimeException if the documents do not match
      */
-    private function assertEquals(ArrayObject $expected, ArrayObject $actual, $ignoreExtraKeys)
+    private function assertEquals(ArrayObject $expected, ArrayObject $actual, $ignoreExtraKeys, $keyPrefix = '')
     {
         if (get_class($expected) !== get_class($actual)) {
-            throw new RuntimeException(sprintf('$expected is %s but $actual is %s', get_class($expected), get_class($actual)));
+            throw new RuntimeException(sprintf(
+                '%s is not instance of expected class "%s"',
+                $this->exporter->shortenedExport($actual),
+                get_class($expected)
+            ));
         }
 
         foreach ($expected as $key => $expectedValue) {
             $actualHasKey = $actual->offsetExists($key);
 
             if (!$actualHasKey) {
-                throw new RuntimeException('$actual is missing key: ' . $key);
+                throw new RuntimeException(sprintf('$actual is missing key: "%s"', $keyPrefix . $key));
             }
 
             if (in_array($expectedValue, $this->placeholders, true)) {
@@ -111,12 +149,53 @@ private function assertEquals(ArrayObject $expected, ArrayObject $actual, $ignor
 
             if (($expectedValue instanceof BSONArray && $actualValue instanceof BSONArray) ||
                 ($expectedValue instanceof BSONDocument && $actualValue instanceof BSONDocument)) {
-                $this->assertEquals($expectedValue, $actualValue, $this->ignoreExtraKeysInEmbedded);
+                $this->assertEquals($expectedValue, $actualValue, $this->ignoreExtraKeysInEmbedded, $keyPrefix . $key . '.');
+                continue;
+            }
+
+            if (is_scalar($expectedValue) && is_scalar($actualValue)) {
+                if ($expectedValue !== $actualValue) {
+                    throw new ComparisonFailure(
+                        $expectedValue,
+                        $actualValue,
+                        '',
+                        '',
+                        false,
+                        sprintf('Field path "%s": %s', $keyPrefix . $key, 'Failed asserting that two values are equal.')
+                    );
+                }
+
                 continue;
             }
 
-            if (gettype($expectedValue) != gettype($actualValue) || $expectedValue != $actualValue) {
-                throw new RuntimeException('$expectedValue != $actualValue for key: ' . $key);
+            // Workaround for ObjectComparator printing the whole actual object
+            if (get_class($expectedValue) !== get_class($actualValue)) {
+                throw new ComparisonFailure(
+                    $expectedValue,
+                    $actualValue,
+                    '',
+                    '',
+                    false,
+                    \sprintf(
+                        'Field path "%s": %s is not instance of expected class "%s".',
+                        $keyPrefix . $key,
+                        $this->exporter->shortenedExport($actualValue),
+                        get_class($expectedValue)
+                    )
+                );
+            }
+
+            try {
+                $this->comparatorFactory->getComparatorFor($expectedValue, $actualValue)->assertEquals($expectedValue, $actualValue);
+            } catch (ComparisonFailure $failure) {
+                throw new ComparisonFailure(
+                    $expectedValue,
+                    $actualValue,
+                    '',
+                    '',
+                    false,
+                    sprintf('Field path "%s": %s', $keyPrefix . $key, $failure->getMessage())
+                );
             }
         }
 
@@ -126,7 +205,7 @@ private function assertEquals(ArrayObject $expected, ArrayObject $actual, $ignor
 
         foreach ($actual as $key => $value) {
             if (!$expected->offsetExists($key)) {
-                throw new RuntimeException('$actual has extra key: ' . $key);
+                throw new RuntimeException(sprintf('$actual has extra key: "%s"', $keyPrefix . $key));
             }
         }
     }
diff --git a/tests/SpecTests/DocumentsMatchConstraintTest.php b/tests/SpecTests/DocumentsMatchConstraintTest.php
@@ -2,6 +2,9 @@
 
 namespace MongoDB\Tests\SpecTests;
 
+use ArrayObject;
+use MongoDB\Model\BSONArray;
+use PHPUnit\Framework\ExpectationFailedException;
 use PHPUnit\Framework\TestCase;
 
 class DocumentsMatchConstraintTest extends TestCase
@@ -16,7 +19,7 @@ public function testIgnoreExtraKeysInRoot()
         $this->assertResult(false, $c, ['x' => 1, 'y' => ['a' => 1, 'b' => 2, 'c' => 3]], 'Extra keys in embedded are not permitted');
         $this->assertResult(true, $c, ['y' => ['b' => 2, 'a' => 1], 'x' => 1], 'Root and embedded key order is not significant');
 
-        // Arrays are always intepretted as root documents
+        // Arrays are always interpreted as root documents
         $c = new DocumentsMatchConstraint([1, ['a' => 1]], true, false);
 
         $this->assertResult(false, $c, [1, 2], 'Incorrect value');
@@ -36,7 +39,7 @@ public function testIgnoreExtraKeysInEmbedded()
         $this->assertResult(true, $c, ['x' => 1, 'y' => ['a' => 1, 'b' => 2, 'c' => 3]], 'Extra keys in embedded are permitted');
         $this->assertResult(true, $c, ['y' => ['b' => 2, 'a' => 1], 'x' => 1], 'Root and embedded Key order is not significant');
 
-        // Arrays are always intepretted as root documents
+        // Arrays are always interpreted as root documents
         $c = new DocumentsMatchConstraint([1, ['a' => 1]], false, true);
 
         $this->assertResult(false, $c, [1, 2], 'Incorrect value');
@@ -55,6 +58,56 @@ public function testPlaceholders()
         $this->assertResult(true, $c, ['x' => '42', 'y' => 42, 'z' => ['a' => 24]], 'Exact match');
     }
 
+    /**
+     * @dataProvider errorMessageProvider
+     */
+    public function testErrorMessages($expectedMessagePart, DocumentsMatchConstraint $constraint, $actualValue)
+    {
+        try {
+            $constraint->evaluate($actualValue);
+            $this->fail('Expected a comparison failure');
+        } catch (ExpectationFailedException $failure) {
+            $this->assertContains('Failed asserting that two BSON objects are equal.', $failure->getMessage());
+            $this->assertContains($expectedMessagePart, $failure->getMessage());
+        }
+    }
+
+    public function errorMessageProvider()
+    {
+        return [
+            'Root type mismatch' => [
+                'MongoDB\Model\BSONArray Object (...) is not instance of expected class "MongoDB\Model\BSONDocument"',
+                new DocumentsMatchConstraint(['foo' => 'bar']),
+                new BSONArray(['foo' => 'bar']),
+            ],
+            'Missing key' => [
+                '$actual is missing key: "foo.bar"',
+                new DocumentsMatchConstraint(['foo' => ['bar' => 'baz']]),
+                ['foo' => ['foo' => 'bar']],
+            ],
+            'Extra key' => [
+                '$actual has extra key: "foo.foo"',
+                new DocumentsMatchConstraint(['foo' => ['bar' => 'baz']]),
+                ['foo' => ['foo' => 'bar', 'bar' => 'baz']],
+            ],
+            'Scalar value not equal' => [
+                'Field path "foo": Failed asserting that two values are equal.',
+                new DocumentsMatchConstraint(['foo' => 'bar']),
+                ['foo' => 'baz'],
+            ],
+            'Scalar type mismatch' => [
+                'Field path "foo": Failed asserting that two values are equal.',
+                new DocumentsMatchConstraint(['foo' => 42]),
+                ['foo' => '42'],
+            ],
+            'Type mismatch' => [
+                'Field path "foo": MongoDB\Model\BSONDocument Object (...) is not instance of expected class "MongoDB\Model\BSONArray".',
+                new DocumentsMatchConstraint(['foo' => ['bar']]),
+                ['foo' => (object) ['bar']],
+            ],
+        ];
+    }
+
     private function assertResult($expectedResult, DocumentsMatchConstraint $constraint, $value, $message)
     {
         $this->assertSame($expectedResult, $constraint->evaluate($value, '', true), $message);
diff --git a/tests/bootstrap.php b/tests/bootstrap.php
@@ -17,3 +17,7 @@ class_alias(PHPUnit_Framework_Error_Warning::class, PHPUnit\Framework\Error\Warn
 if ( ! class_exists(PHPUnit\Framework\Constraint\Constraint::class)) {
     class_alias(PHPUnit_Framework_Constraint::class, PHPUnit\Framework\Constraint\Constraint::class);
 }
+
+if ( ! class_exists(PHPUnit\Framework\ExpectationFailedException::class)) {
+    class_alias(PHPUnit_Framework_ExpectationFailedException::class, PHPUnit\Framework\ExpectationFailedException::class);
+}

Original file line number	Diff line number	Diff line change
`@@ -17,3 +17,7 @@ class_alias(PHPUnit_Framework_Error_Warning::class, PHPUnit\Framework\Error\Warn`
`17`	`17`	`if ( ! class_exists(PHPUnit\Framework\Constraint\Constraint::class)) {`
`18`	`18`	`class_alias(PHPUnit_Framework_Constraint::class, PHPUnit\Framework\Constraint\Constraint::class);`
`19`	`19`	`}`
	`20`	`+`
	`21`	`+if ( ! class_exists(PHPUnit\Framework\ExpectationFailedException::class)) {`
	`22`	`+ class_alias(PHPUnit_Framework_ExpectationFailedException::class, PHPUnit\Framework\ExpectationFailedException::class);`
	`23`	`+}`