PHPLIB-459: Support better failure descriptions in DocumentsMatchConstraint

alcaeus · alcaeus · commit da9fcd79cd82 · 2019-08-20T11:39:18.000+02:00
diff --git a/composer.json b/composer.json
@@ -17,6 +17,7 @@
     },
     "require-dev": {
         "phpunit/phpunit": "^5.7.27 || ^6.4 || ^8.3",
+        "sebastian/comparator": "^1.0 || ^2.0 || ^3.0",
         "symfony/phpunit-bridge": "^4.4@dev"
     },
     "autoload": {
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;
 use Symfony\Bridge\PhpUnit\ConstraintTrait;
 
@@ -31,6 +33,12 @@ class DocumentsMatchConstraint extends Constraint
     private $sortKeys = false;
     private $value;
 
+    /** @var ComparisonFailure|null */
+    private $lastFailure;
+
+    /** @var Factory */
+    private $comparatorFactory;
+
     /**
      * Creates a new constraint.
      *
@@ -45,6 +53,44 @@ public function __construct($value, $ignoreExtraKeysInRoot = false, $ignoreExtra
         $this->ignoreExtraKeysInRoot = $ignoreExtraKeysInRoot;
         $this->ignoreExtraKeysInEmbedded = $ignoreExtraKeysInEmbedded;
         $this->placeholders = $placeholders;
+        $this->comparatorFactory = Factory::getInstance();
+    }
+
+    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,
+         * and compare strings.
+         *
+         * If ignoreExtraKeys is false and sortKeys is true, we still be able to
+         * compare JSON strings but will still require preparation to sort keys
+         * 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) {
+            $this->lastFailure = new ComparisonFailure(
+                $this->value,
+                $other,
+                $this->exporter()->export($this->value),
+                $this->exporter()->export($other),
+                false,
+                $e->getMessage()
+            );
+        }
+
+        if ($returnResult) {
+            return $success;
+        }
+
+        if (!$success) {
+            $this->fail($other, $description, $this->lastFailure);
+        }
     }
 
     /**
@@ -53,19 +99,24 @@ public function __construct($value, $ignoreExtraKeysInRoot = false, $ignoreExtra
      * @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)) {
@@ -76,12 +127,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 (gettype($expectedValue) != gettype($actualValue) || $expectedValue != $actualValue) {
-                throw new RuntimeException('$expectedValue != $actualValue for key: ' . $key);
+            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;
+            }
+
+            // 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())
+                );
             }
         }
 
@@ -91,11 +183,25 @@ 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));
             }
         }
     }
 
+    private function doAdditionalFailureDescription($other)
+    {
+        if ($this->lastFailure === null) {
+            return '';
+        }
+
+        return $this->lastFailure->getMessage();
+    }
+
+    private function doFailureDescription($other)
+    {
+        return 'two BSON objects are equal';
+    }
+
     private function doMatches($other)
     {
         /* TODO: If ignoreExtraKeys and sortKeys are both false, then we may be
@@ -118,7 +224,7 @@ private function doMatches($other)
 
     private function doToString()
     {
-        return 'matches ' . json_encode($this->value);
+        return 'matches ' . $this->exporter()->export($this->value);
     }
 
     /**
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->assertStringContainsString('Failed asserting that two BSON objects are equal.', $failure->getMessage());
+            $this->assertStringContainsString($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`	`+}`