From 3a98c223efa60e1760f4ef9dc0a10c686e7439dd Mon Sep 17 00:00:00 2001 From: Alexander Varwijk Date: Wed, 3 Feb 2021 12:36:19 +0100 Subject: [PATCH] Simplify assertResultErrors to avoid closure serialization This commit changes some loops into built-in PHP functions to simplify the code and eliminate the need for closures. Additionally this switches over to an `assertEmpty` from `assertEquals`. This achieves the same test result and in the message still provides the errors. However it prevents test failures with weird serialization complaints that are caused by closures in function arguments of GraphQL Error stacktraces. See https://github.com/sebastianbergmann/phpunit/issues/4371#issuecomment-772094587 The downside to this change is that we may lose some contextual information that the full error in the array comparison may provide. The upside is that we can actually see why tests fail since in a lot of cases this beneficial diff doesn't make it into the test output anyway. So all-in-all this is a step forward. --- .../src/Traits/QueryResultAssertionTrait.php | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/tests/src/Traits/QueryResultAssertionTrait.php b/tests/src/Traits/QueryResultAssertionTrait.php index 21db9363d..a06d70b1e 100644 --- a/tests/src/Traits/QueryResultAssertionTrait.php +++ b/tests/src/Traits/QueryResultAssertionTrait.php @@ -4,7 +4,6 @@ use Drupal\Core\Cache\CacheableMetadata; use Drupal\graphql\GraphQL\Execution\ExecutionResult; -use GraphQL\Error\Error; use GraphQL\Server\OperationParams; /** @@ -145,34 +144,28 @@ private function assertResultData(ExecutionResult $result, $expected): void { * @internal */ private function assertResultErrors(ExecutionResult $result, array $expected): void { - // Retrieve the list of error strings. - $errors = array_map(function (Error $error) { - return $error->getMessage(); - }, $result->errors); - // Initalize the status. $unexpected = []; - $matchCount = array_map(function () { - return 0; - }, array_flip($expected)); + $matchCount = array_fill_keys($expected, 0); // Iterate through error messages. // Collect unmatched errors and count pattern hits. - while ($error = array_pop($errors)) { + foreach ($result->errors as $error) { + $error_message = $error->getMessage(); $match = FALSE; foreach ($expected as $pattern) { - if (@preg_match($pattern, $error) === FALSE) { - $match = $match || $pattern == $error; + if (@preg_match($pattern, $error_message) === FALSE) { + $match = $match || $pattern == $error_message; $matchCount[$pattern]++; } else { - $match = $match || preg_match($pattern, $error); + $match = $match || preg_match($pattern, $error_message); $matchCount[$pattern]++; } } if (!$match) { - $unexpected[] = $error; + $unexpected[] = $error_message; } } @@ -181,8 +174,8 @@ private function assertResultErrors(ExecutionResult $result, array $expected): v return $count == 0; })); - $this->assertEquals([], $missing, "Missing errors:\n* " . implode("\n* ", $missing)); - $this->assertEquals([], $unexpected, "Unexpected errors:\n* " . implode("\n* ", $unexpected)); + self::assertEmpty($missing, "Missing errors:\n* " . implode("\n* ", $missing)); + self::assertEmpty($unexpected, "Unexpected errors:\n* " . implode("\n* ", $unexpected)); } /**