Improve assertion traits: DX, type safety, and performance (#235)

Author: TavoNiievez

.github/workflows/main.yml

@@ -55,8 +55,9 @@ jobs:
 
           if [ "${{ matrix.symfony }}" = "7.4" ]; then
             composer require codeception/module-rest --dev
+            git -C framework-tests checkout -- composer.json
             git -C framework-tests apply resetFormatsAfterRequest_issue_test.patch
-            composer -d framework-tests install --no-progress
+            composer -d framework-tests update --no-progress
             php framework-tests/bin/console lexik:jwt:generate-keypair --skip-if-exists
             php vendor/bin/codecept run Functional -c framework-tests
           fi

composer.json

@@ -31,7 +31,7 @@
         "codeception/module-doctrine": "^3.3",
         "doctrine/orm": "^3.6",
         "friendsofphp/php-cs-fixer": "^3.94",
-        "phpstan/phpstan": "^2.1",
+        "phpstan/phpstan": "^2.2",
         "phpunit/phpunit": "^11.0 | ^12.0",
         "symfony/browser-kit": "^5.4 | ^6.4 | ^7.4 | ^8.0",
         "symfony/cache": "^5.4 | ^6.4 | ^7.4 | ^8.0",

src/Codeception/Module/Symfony.php

@@ -49,8 +49,6 @@
 use Symfony\Component\Notifier\DataCollector\NotificationDataCollector;
 use Symfony\Component\VarDumper\Cloner\Data;
 
-use function array_filter;
-use function array_map;
 use function class_exists;
 use function codecept_root_dir;
 use function count;
@@ -61,6 +59,7 @@
 use function ini_get;
 use function ini_set;
 use function is_object;
+use function is_scalar;
 use function is_subclass_of;
 use function sprintf;
 
@@ -334,8 +333,11 @@ protected function getKernelClass(): string
         if (file_exists($expectedKernelPath)) {
             include_once $expectedKernelPath;
         } else {
-            foreach (glob($path . DIRECTORY_SEPARATOR . '*Kernel.php') ?: [] as $file) {
-                include_once $file;
+            $kernelFiles = glob($path . DIRECTORY_SEPARATOR . '*Kernel.php', GLOB_NOSORT);
+            if ($kernelFiles !== false) {
+                foreach ($kernelFiles as $file) {
+                    include_once $file;
+                }
             }
         }
 
@@ -445,7 +447,13 @@ private function debugSecurityData(SecurityDataCollector $securityCollector): vo
             $roles = $roles->getValue(true);
         }
 
-        $rolesStr = implode(',', array_map('strval', array_filter((array) $roles, 'is_scalar')));
+        $scalarRoles = [];
+        foreach ((array) $roles as $role) {
+            if (is_scalar($role)) {
+                $scalarRoles[] = (string) $role;
+            }
+        }
+        $rolesStr = implode(',', $scalarRoles);
         $this->debugSection('User', sprintf('%s [%s]', $securityCollector->getUser(), $rolesStr));
     }
 

src/Codeception/Module/Symfony/BrowserAssertionsTrait.php

@@ -109,7 +109,8 @@ public function assertResponseCookieValueSame(string $name, string $expectedValu
      */
     public function assertResponseFormatSame(?string $expectedFormat, string $message = ''): void
     {
-        $this->assertThatForResponse(new ResponseFormatSame($this->getClient()->getRequest(), $expectedFormat), $message);
+        $client = $this->getClient();
+        $this->assertThatForResponse(new ResponseFormatSame($client->getRequest(), $expectedFormat), $message);
     }
 
     /**
@@ -233,14 +234,14 @@ public function assertResponseRedirects(?string $expectedLocation = null, ?int $
     {
         $this->assertThatForResponse(new ResponseIsRedirected($verbose), $message);
 
-        if ($expectedLocation) {
+        if ($expectedLocation !== null) {
             $constraint = class_exists(ResponseHeaderLocationSame::class)
                 ? new ResponseHeaderLocationSame($this->getClient()->getRequest(), $expectedLocation)
                 : new ResponseHeaderSame('Location', $expectedLocation);
             $this->assertThatForResponse($constraint, $message);
         }
 
-        if ($expectedCode) {
+        if ($expectedCode !== null) {
             $this->assertThatForResponse(new ResponseStatusCodeSame($expectedCode), $message);
         }
     }
@@ -317,8 +318,9 @@ protected function doRebootClientKernel(): void {}
     public function seePageIsAvailable(?string $url = null): void
     {
         if ($url !== null) {
-            $this->getClient()->request('GET', $url);
-            $this->assertStringContainsString($url, $this->getClient()->getRequest()->getRequestUri());
+            $client = $this->getClient();
+            $client->request('GET', $url);
+            $this->assertStringContainsString($url, $client->getRequest()->getRequestUri());
         }
 
         $this->assertResponseIsSuccessful();
@@ -370,10 +372,11 @@ public function submitSymfonyForm(string $name, array $fields): void
             $params[$name . $key] = $value;
         }
 
-        $node = $this->getClient()->getCrawler()->filter($selector);
+        $client = $this->getClient();
+        $node = $client->getCrawler()->filter($selector);
         $this->assertGreaterThan(0, $node->count(), sprintf('Form "%s" not found.', $selector));
         $form = $node->form();
-        $this->getClient()->submit($form, $params);
+        $client->submit($form, $params);
     }
 
     protected function assertThatForClient(Constraint $constraint, string $message = ''): void

src/Codeception/Module/Symfony/CacheTrait.php

@@ -7,8 +7,10 @@
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpKernel\Profiler\Profile;
 
+use function array_key_exists;
 use function array_unique;
 use function array_values;
+use function is_string;
 
 trait CacheTrait
 {
@@ -40,11 +42,13 @@ protected function getInternalDomains(): array
 
         $domains = [];
         foreach ($this->grabRouterService()->getRouteCollection() as $route) {
-            if ($route->getHost() !== '') {
-                $regex = $route->compile()->getHostRegex();
-                if ($regex !== null && $regex !== '') {
-                    $domains[] = $regex;
-                }
+            if ($route->getHost() === '') {
+                continue;
+            }
+
+            $hostRegex = $route->compile()->getHostRegex();
+            if ($hostRegex !== null && $hostRegex !== '') {
+                $domains[] = $hostRegex;
             }
         }
 
@@ -67,22 +71,23 @@ protected function clearRouterCache(): void
      */
     protected function grabCachedService(string $expectedClass, array $serviceIds): ?object
     {
-        $serviceId = $this->state[$expectedClass] ??= (function () use ($serviceIds, $expectedClass): ?string {
+        if (!array_key_exists($expectedClass, $this->state)) {
+            $this->state[$expectedClass] = null;
             foreach ($serviceIds as $id) {
-                if ($this->getService($id) instanceof $expectedClass) {
-                    return $id;
+                $service = $this->getService($id);
+                if ($service instanceof $expectedClass) {
+                    $this->state[$expectedClass] = $id;
+                    break;
                 }
             }
+        }
 
-            return null;
-        })();
-
+        $serviceId = $this->state[$expectedClass];
         if (!is_string($serviceId)) {
             return null;
         }
 
         $service = $this->getService($serviceId);
-
         return $service instanceof $expectedClass ? $service : null;
     }
 }

src/Codeception/Module/Symfony/ConsoleAssertionsTrait.php

@@ -60,6 +60,7 @@ public function runSymfonyConsoleCommand(
      */
     private function configureOptions(array $parameters): array
     {
+        /** @var array<string, bool|int> $options */
         $options = [];
 
         foreach ($parameters as $key => $value) {

src/Codeception/Module/Symfony/DomCrawlerAssertionsTrait.php

@@ -11,6 +11,8 @@
 use Symfony\Component\DomCrawler\Test\Constraint\CrawlerSelectorTextContains;
 use Symfony\Component\DomCrawler\Test\Constraint\CrawlerSelectorTextSame;
 
+use function sprintf;
+
 trait DomCrawlerAssertionsTrait
 {
     /**
@@ -172,11 +174,14 @@ private function assertCheckboxState(string $fieldName, bool $checked, string $m
 
     private function assertInputValue(string $fieldName, string $expectedValue, bool $same, string $message): void
     {
-        $this->assertThatCrawler(new CrawlerSelectorExists("input[name=\"$fieldName\"]"), $message);
-        $constraint = new CrawlerSelectorAttributeValueSame("input[name=\"$fieldName\"]", 'value', $expectedValue);
+        $selector = "input[name=\"$fieldName\"]";
+        $context = $message ?: sprintf('Value of input "%s"', $fieldName);
+        $this->assertThatCrawler(new CrawlerSelectorExists($selector), $context);
+
+        $constraint = new CrawlerSelectorAttributeValueSame($selector, 'value', $expectedValue);
         if (!$same) {
             $constraint = new LogicalNot($constraint);
         }
-        $this->assertThatCrawler($constraint, $message);
+        $this->assertThatCrawler($constraint, $context);
     }
 }

src/Codeception/Module/Symfony/EventsAssertionsTrait.php

@@ -249,6 +249,14 @@ protected function assertListenerCalled(
             Assert::fail('No event listener was called.');
         }
 
+        $listenersByEvent = [];
+        $allEventListeners = [];
+        foreach ($actualEvents as $actualEvent) {
+            $pretty = $actualEvent['pretty'];
+            $allEventListeners[] = $pretty;
+            $listenersByEvent[$actualEvent['event']][] = $pretty;
+        }
+
         foreach ($expectedListeners as $listener) {
             $listenerName = match (true) {
                 is_array($listener) && isset($listener[0]) => is_string($listener[0]) ? $listener[0] : (is_object($listener[0]) ? $listener[0]::class : 'array'),
@@ -259,26 +267,28 @@ protected function assertListenerCalled(
 
             foreach ($expectedEvents as $event) {
                 $eventStr = (string) $event;
+                $wasCalled = $event === null
+                    ? $this->hasListenerPrefix($allEventListeners, $listenerName)
+                    : $this->hasListenerPrefix($listenersByEvent[$event] ?? [], $listenerName);
+
                 $this->assertSame(
                     $shouldBeCalled,
-                    $this->listenerWasCalled($listenerName, $event, $actualEvents),
+                    $wasCalled,
                     sprintf("The '%s' listener was %scalled%s", $listenerName, $shouldBeCalled ? 'not ' : '', $event ? " for the '{$eventStr}' event" : '')
                 );
             }
         }
     }
 
-    /** @param list<array{event: string, pretty: string}> $actualEvents */
-    private function listenerWasCalled(string $expectedListener, ?string $expectedEvent, array $actualEvents): bool
+    /** @param list<string> $listeners */
+    private function hasListenerPrefix(array $listeners, string $listenerName): bool
     {
-        foreach ($actualEvents as $actualEvent) {
-            if ($expectedEvent !== null && $actualEvent['event'] !== $expectedEvent) {
-                continue;
-            }
-            if (str_starts_with($actualEvent['pretty'], $expectedListener)) {
+        foreach ($listeners as $actualListener) {
+            if (str_starts_with($actualListener, $listenerName)) {
                 return true;
             }
         }
+
         return false;
     }
 
@@ -287,8 +297,8 @@ protected function getDefaultDispatcher(): string
         return 'event_dispatcher';
     }
 
-    protected function grabEventCollector(string $function): EventDataCollector
+    protected function grabEventCollector(string $callingFunction): EventDataCollector
     {
-        return $this->grabCollector(DataCollectorName::EVENTS, $function);
+        return $this->grabCollector(DataCollectorName::EVENTS, $callingFunction);
     }
 }

src/Codeception/Module/Symfony/FormAssertionsTrait.php

@@ -175,14 +175,14 @@ public function seeFormHasErrors(): void
         $this->assertGreaterThan(0, $this->getFormErrorsCount(__FUNCTION__), 'Expecting that the form has errors, but there were none!');
     }
 
-    protected function grabFormCollector(string $function): FormDataCollector
+    protected function grabFormCollector(string $callingFunction): FormDataCollector
     {
-        return $this->grabCollector(DataCollectorName::FORM, $function);
+        return $this->grabCollector(DataCollectorName::FORM, $callingFunction);
     }
 
-    private function getFormErrorsCount(string $function): int
+    private function getFormErrorsCount(string $callingFunction): int
     {
-        $collector = $this->grabFormCollector($function);
+        $collector = $this->grabFormCollector($callingFunction);
         $rawData = $this->getRawCollectorData($collector);
 
         return isset($rawData['nb_errors']) && is_numeric($rawData['nb_errors']) ? (int) $rawData['nb_errors'] : 0;

src/Codeception/Module/Symfony/HttpClientAssertionsTrait.php

@@ -90,16 +90,17 @@ public function assertNotHttpClientRequest(
      */
     private function hasHttpClientRequest(
         string $httpClientId,
-        string $function,
+        string $callingFunction,
         string $expectedUrl,
         string $expectedMethod,
         string|array|null $expectedBody = null,
         array $expectedHeaders = []
     ): bool {
         $expectedHeadersLower = $expectedHeaders === [] ? [] : array_change_key_case($expectedHeaders);
+        $traces = $this->getHttpClientTraces($httpClientId, $callingFunction);
 
-        foreach ($this->getHttpClientTraces($httpClientId, $function) as $trace) {
-            if (!is_array($trace) || ($trace['method'] ?? null) !== $expectedMethod) {
+        foreach ($traces as $trace) {
+            if (($trace['method'] ?? null) !== $expectedMethod) {
                 continue;
             }
 
@@ -124,23 +125,27 @@ private function hasHttpClientRequest(
             }
 
             $actualHeaders = $this->extractValue($options['headers'] ?? []);
-            if (is_array($actualHeaders) && $expectedHeadersLower === array_intersect_key(array_change_key_case($actualHeaders), $expectedHeadersLower)) {
+            if (!is_array($actualHeaders)) {
+                continue;
+            }
+
+            if ($expectedHeadersLower === array_intersect_key(array_change_key_case($actualHeaders), $expectedHeadersLower)) {
                 return true;
             }
         }
 
         return false;
     }
 
-    /** @return array<mixed> */
-    private function getHttpClientTraces(string $httpClientId, string $function): array
+    /** @return list<array<string, mixed>> */
+    private function getHttpClientTraces(string $httpClientId, string $callingFunction): array
     {
-        $clients = $this->grabHttpClientCollector($function)->getClients();
+        $clients = $this->grabHttpClientCollector($callingFunction)->getClients();
         if (!isset($clients[$httpClientId]) || !is_array($clients[$httpClientId])) {
             Assert::fail(sprintf('HttpClient "%s" is not registered.', $httpClientId));
         }
 
-        /** @var array{traces: array<mixed>} $clientData */
+        /** @var array{traces: list<array<string, mixed>>} $clientData */
         $clientData = $clients[$httpClientId];
         return $clientData['traces'];
     }
@@ -155,8 +160,8 @@ private function extractValue(mixed $traceData): mixed
         };
     }
 
-    protected function grabHttpClientCollector(string $function): HttpClientDataCollector
+    protected function grabHttpClientCollector(string $callingFunction): HttpClientDataCollector
     {
-        return $this->grabCollector(DataCollectorName::HTTP_CLIENT, $function);
+        return $this->grabCollector(DataCollectorName::HTTP_CLIENT, $callingFunction);
     }
 }