Diff of PHPMatcherConstraint ignores patterns

[niklasnatter]

Hey,
we are using this library for asserting the content of JSON responses in our functional tests. Apart from a few minor issues, this is working great. So thanks for your effort!

As we are using the library for testing API responses, we need to use a lot of value-patterns to match values such as autogenerated ids and timestamps. This works great as long as the response matches our response-pattern. Unfortunately, if the response does not match our response-pattern, the PHPMatcherConstraint includes all lines that use a value-pattern in its diff. This makes it hard to find out what line actually contains an error.

For example, the diff of the following test-case includes "name": "@string@" even though "name": "test-2" would match the pattern:

    public function testPattern(): void
    {
        $pattern = <<<EOF
            {
                "name": "@string@",
                "description": "test-1"
            }
EOF;

        $value = <<<EOF
            {
                "name": "test-2",
                "description": "test-2"
            }
EOF;

        TestCase::assertThat(
            $value,
            new PHPMatcherConstraint($pattern)
        );
    }

--- Expected
+++ Actual
@@ @@
 {
-    "name": "@string@",
-    "description": "test-1"
+    "name": "test-2",
+    "description": "test-2"
 }

Is it possible somehow to exclude the lines that do match the pattern from the diff?

Thanks in advance!

[niklasnatter]

Hey,
I just tested this again with 5.0.1 and it looks like the diff of the example above has not changed. Do I need to adjust some configuration? Or is it possible that this was closed by mistake? 🙂

Thanks in advance!

[norberttech]

Hey, sorry, I accidentally closed wrong PR, #212 was the one I wanted to close.

[norberttech]

Hey, so now the diff definitely changed, version 6.0.2 brings totally new, more precise error messages, I'm closing this issue, if you are still using the library don't hesitate to leave some feedback, thanks!

[niklasnatter]

Really like the new error messages! Thanks a lot for your work!

[niklasnatter]

Just a small proposal - the output for the following test case looks like this at the moment:

use Coduo\PHPMatcher\PHPUnit\PHPMatcherConstraint;
use PHPUnit\Framework\TestCase;

class PatternTest extends TestCase
{
    public function testPattern(): void
    {
        $pattern = <<<EOF
            {
                "name": "@string@",
                "description": "test-1"
            }
EOF;

        $value = <<<EOF
            {
                "name": "test-2",
                "description": "test-2"
            }
EOF;

        TestCase::assertThat(
            $value,
            new PHPMatcherConstraint($pattern)
        );
    }
}

1) Namespace\PatternTest::testPattern
Failed asserting that '            {\n
                "name": "test-2",\n
                "description": "test-2"\n
            }' matches given pattern.
Pattern: '            {\n
                "name": "@string@",\n
                "description": "test-1"\n
            }'
Error: Value "test-2" does not match pattern "test-1" at path: "[description]"
Backtrace: 
Empty.
--- Expected
+++ Actual
@@ @@
 {
-    "description": "test-1",
-    "name": "@string@"
+    "description": "test-2",
+    "name": "test-2"
 }

The new error message is great, but it is hard to spot in the middle of the output. What do you think about adjusting the PHPMatcherConstraint class to omit the pattern from the first part of the error and output something like this:

1) Namespace\PatternTest::testPattern
Value "test-2" does not match pattern "test-1" at path: "[description]"
--- Expected
+++ Actual
@@ @@
 {
-    "description": "test-1",
-    "name": "@string@"
+    "description": "test-2",
+    "name": "test-2"
 }

Would you accept a pull request for this?

Thanks for your time!

[norberttech]

hey @nnatter it makes perfect sense now to remove the pattern from that message. I would be happy to merge a pull request with this change 🙌

[niklasnatter]

Just created #226. Not sure if I adjusted the test cases correctly - feel free to add some comments 🙂