Skip to content

Commit 4671a98

Browse files
TavoNiievezclaude
andcommitted
Improve assertion traits: DX, type safety, and performance
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 250f628 commit 4671a98

22 files changed

Lines changed: 149 additions & 90 deletions

.github/workflows/main.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,9 @@ jobs:
5555
5656
if [ "${{ matrix.symfony }}" = "7.4" ]; then
5757
composer require codeception/module-rest --dev
58+
git -C framework-tests checkout -- composer.json
5859
git -C framework-tests apply resetFormatsAfterRequest_issue_test.patch
59-
composer -d framework-tests install --no-progress
60+
composer -d framework-tests update --no-progress
6061
php framework-tests/bin/console lexik:jwt:generate-keypair --skip-if-exists
6162
php vendor/bin/codecept run Functional -c framework-tests
6263
fi

.jules/palette_dx.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
## 2026-06-09 - DX: Avoid 'callable'-like Variable Names
2+
**Learning:** Using variables named `$function` for strings representing method or function names causes cognitive confusion with PHP's `callable` pseudo-type or anonymous functions.
3+
**Action:** Always rename vague variables like `$function` to context-specific names like `$callingFunction` to improve code clarity and discoverability.

composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
"codeception/module-doctrine": "^3.3",
3232
"doctrine/orm": "^3.6",
3333
"friendsofphp/php-cs-fixer": "^3.94",
34-
"phpstan/phpstan": "^2.1",
34+
"phpstan/phpstan": "^2.2",
3535
"phpunit/phpunit": "^11.0 | ^12.0",
3636
"symfony/browser-kit": "^5.4 | ^6.4 | ^7.4 | ^8.0",
3737
"symfony/cache": "^5.4 | ^6.4 | ^7.4 | ^8.0",
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Codeception\Exception;
6+
7+
use InvalidArgumentException;
8+
9+
class InvalidSessionAttributeException extends InvalidArgumentException
10+
{
11+
}

src/Codeception/Module/Symfony.php

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,6 @@
4949
use Symfony\Component\Notifier\DataCollector\NotificationDataCollector;
5050
use Symfony\Component\VarDumper\Cloner\Data;
5151

52-
use function array_filter;
53-
use function array_map;
5452
use function class_exists;
5553
use function codecept_root_dir;
5654
use function count;
@@ -61,6 +59,7 @@
6159
use function ini_get;
6260
use function ini_set;
6361
use function is_object;
62+
use function is_scalar;
6463
use function is_subclass_of;
6564
use function sprintf;
6665

@@ -334,8 +333,11 @@ protected function getKernelClass(): string
334333
if (file_exists($expectedKernelPath)) {
335334
include_once $expectedKernelPath;
336335
} else {
337-
foreach (glob($path . DIRECTORY_SEPARATOR . '*Kernel.php') ?: [] as $file) {
338-
include_once $file;
336+
$kernelFiles = glob($path . DIRECTORY_SEPARATOR . '*Kernel.php', GLOB_NOSORT);
337+
if ($kernelFiles !== false) {
338+
foreach ($kernelFiles as $file) {
339+
include_once $file;
340+
}
339341
}
340342
}
341343

@@ -445,7 +447,13 @@ private function debugSecurityData(SecurityDataCollector $securityCollector): vo
445447
$roles = $roles->getValue(true);
446448
}
447449

448-
$rolesStr = implode(',', array_map('strval', array_filter((array) $roles, 'is_scalar')));
450+
$scalarRoles = [];
451+
foreach ((array) $roles as $role) {
452+
if (is_scalar($role)) {
453+
$scalarRoles[] = (string) $role;
454+
}
455+
}
456+
$rolesStr = implode(',', $scalarRoles);
449457
$this->debugSection('User', sprintf('%s [%s]', $securityCollector->getUser(), $rolesStr));
450458
}
451459

src/Codeception/Module/Symfony/BrowserAssertionsTrait.php

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ public function assertResponseCookieValueSame(string $name, string $expectedValu
109109
*/
110110
public function assertResponseFormatSame(?string $expectedFormat, string $message = ''): void
111111
{
112-
$this->assertThatForResponse(new ResponseFormatSame($this->getClient()->getRequest(), $expectedFormat), $message);
112+
$client = $this->getClient();
113+
$this->assertThatForResponse(new ResponseFormatSame($client->getRequest(), $expectedFormat), $message);
113114
}
114115

115116
/**
@@ -233,14 +234,14 @@ public function assertResponseRedirects(?string $expectedLocation = null, ?int $
233234
{
234235
$this->assertThatForResponse(new ResponseIsRedirected($verbose), $message);
235236

236-
if ($expectedLocation) {
237+
if ($expectedLocation !== null) {
237238
$constraint = class_exists(ResponseHeaderLocationSame::class)
238239
? new ResponseHeaderLocationSame($this->getClient()->getRequest(), $expectedLocation)
239240
: new ResponseHeaderSame('Location', $expectedLocation);
240241
$this->assertThatForResponse($constraint, $message);
241242
}
242243

243-
if ($expectedCode) {
244+
if ($expectedCode !== null) {
244245
$this->assertThatForResponse(new ResponseStatusCodeSame($expectedCode), $message);
245246
}
246247
}
@@ -317,8 +318,9 @@ protected function doRebootClientKernel(): void {}
317318
public function seePageIsAvailable(?string $url = null): void
318319
{
319320
if ($url !== null) {
320-
$this->getClient()->request('GET', $url);
321-
$this->assertStringContainsString($url, $this->getClient()->getRequest()->getRequestUri());
321+
$client = $this->getClient();
322+
$client->request('GET', $url);
323+
$this->assertStringContainsString($url, $client->getRequest()->getRequestUri());
322324
}
323325

324326
$this->assertResponseIsSuccessful();
@@ -370,10 +372,11 @@ public function submitSymfonyForm(string $name, array $fields): void
370372
$params[$name . $key] = $value;
371373
}
372374

373-
$node = $this->getClient()->getCrawler()->filter($selector);
375+
$client = $this->getClient();
376+
$node = $client->getCrawler()->filter($selector);
374377
$this->assertGreaterThan(0, $node->count(), sprintf('Form "%s" not found.', $selector));
375378
$form = $node->form();
376-
$this->getClient()->submit($form, $params);
379+
$client->submit($form, $params);
377380
}
378381

379382
protected function assertThatForClient(Constraint $constraint, string $message = ''): void

src/Codeception/Module/Symfony/CacheTrait.php

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@
77
use Symfony\Component\DependencyInjection\ContainerInterface;
88
use Symfony\Component\HttpKernel\Profiler\Profile;
99

10+
use function array_key_exists;
1011
use function array_unique;
1112
use function array_values;
13+
use function is_string;
1214

1315
trait CacheTrait
1416
{
@@ -40,11 +42,13 @@ protected function getInternalDomains(): array
4042

4143
$domains = [];
4244
foreach ($this->grabRouterService()->getRouteCollection() as $route) {
43-
if ($route->getHost() !== '') {
44-
$regex = $route->compile()->getHostRegex();
45-
if ($regex !== null && $regex !== '') {
46-
$domains[] = $regex;
47-
}
45+
if ($route->getHost() === '') {
46+
continue;
47+
}
48+
49+
$hostRegex = $route->compile()->getHostRegex();
50+
if ($hostRegex !== null && $hostRegex !== '') {
51+
$domains[] = $hostRegex;
4852
}
4953
}
5054

@@ -67,22 +71,23 @@ protected function clearRouterCache(): void
6771
*/
6872
protected function grabCachedService(string $expectedClass, array $serviceIds): ?object
6973
{
70-
$serviceId = $this->state[$expectedClass] ??= (function () use ($serviceIds, $expectedClass): ?string {
74+
if (!array_key_exists($expectedClass, $this->state)) {
75+
$this->state[$expectedClass] = null;
7176
foreach ($serviceIds as $id) {
72-
if ($this->getService($id) instanceof $expectedClass) {
73-
return $id;
77+
$service = $this->getService($id);
78+
if ($service instanceof $expectedClass) {
79+
$this->state[$expectedClass] = $id;
80+
break;
7481
}
7582
}
83+
}
7684

77-
return null;
78-
})();
79-
85+
$serviceId = $this->state[$expectedClass];
8086
if (!is_string($serviceId)) {
8187
return null;
8288
}
8389

8490
$service = $this->getService($serviceId);
85-
8691
return $service instanceof $expectedClass ? $service : null;
8792
}
8893
}

src/Codeception/Module/Symfony/ConsoleAssertionsTrait.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ public function runSymfonyConsoleCommand(
6060
*/
6161
private function configureOptions(array $parameters): array
6262
{
63+
/** @var array<string, bool|int> $options */
6364
$options = [];
6465

6566
foreach ($parameters as $key => $value) {

src/Codeception/Module/Symfony/DomCrawlerAssertionsTrait.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
use Symfony\Component\DomCrawler\Test\Constraint\CrawlerSelectorTextContains;
1212
use Symfony\Component\DomCrawler\Test\Constraint\CrawlerSelectorTextSame;
1313

14+
use function sprintf;
15+
1416
trait DomCrawlerAssertionsTrait
1517
{
1618
/**
@@ -172,11 +174,14 @@ private function assertCheckboxState(string $fieldName, bool $checked, string $m
172174

173175
private function assertInputValue(string $fieldName, string $expectedValue, bool $same, string $message): void
174176
{
175-
$this->assertThatCrawler(new CrawlerSelectorExists("input[name=\"$fieldName\"]"), $message);
176-
$constraint = new CrawlerSelectorAttributeValueSame("input[name=\"$fieldName\"]", 'value', $expectedValue);
177+
$selector = "input[name=\"$fieldName\"]";
178+
$context = $message ?: sprintf('Value of input "%s"', $fieldName);
179+
$this->assertThatCrawler(new CrawlerSelectorExists($selector), $context);
180+
181+
$constraint = new CrawlerSelectorAttributeValueSame($selector, 'value', $expectedValue);
177182
if (!$same) {
178183
$constraint = new LogicalNot($constraint);
179184
}
180-
$this->assertThatCrawler($constraint, $message);
185+
$this->assertThatCrawler($constraint, $context);
181186
}
182187
}

src/Codeception/Module/Symfony/EventsAssertionsTrait.php

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,14 @@ protected function assertListenerCalled(
249249
Assert::fail('No event listener was called.');
250250
}
251251

252+
$listenersByEvent = [];
253+
$allEventListeners = [];
254+
foreach ($actualEvents as $actualEvent) {
255+
$pretty = $actualEvent['pretty'];
256+
$allEventListeners[] = $pretty;
257+
$listenersByEvent[$actualEvent['event']][] = $pretty;
258+
}
259+
252260
foreach ($expectedListeners as $listener) {
253261
$listenerName = match (true) {
254262
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(
259267

260268
foreach ($expectedEvents as $event) {
261269
$eventStr = (string) $event;
270+
$wasCalled = $event === null
271+
? $this->hasListenerPrefix($allEventListeners, $listenerName)
272+
: $this->hasListenerPrefix($listenersByEvent[$event] ?? [], $listenerName);
273+
262274
$this->assertSame(
263275
$shouldBeCalled,
264-
$this->listenerWasCalled($listenerName, $event, $actualEvents),
276+
$wasCalled,
265277
sprintf("The '%s' listener was %scalled%s", $listenerName, $shouldBeCalled ? 'not ' : '', $event ? " for the '{$eventStr}' event" : '')
266278
);
267279
}
268280
}
269281
}
270282

271-
/** @param list<array{event: string, pretty: string}> $actualEvents */
272-
private function listenerWasCalled(string $expectedListener, ?string $expectedEvent, array $actualEvents): bool
283+
/** @param list<string> $listeners */
284+
private function hasListenerPrefix(array $listeners, string $listenerName): bool
273285
{
274-
foreach ($actualEvents as $actualEvent) {
275-
if ($expectedEvent !== null && $actualEvent['event'] !== $expectedEvent) {
276-
continue;
277-
}
278-
if (str_starts_with($actualEvent['pretty'], $expectedListener)) {
286+
foreach ($listeners as $actualListener) {
287+
if (str_starts_with($actualListener, $listenerName)) {
279288
return true;
280289
}
281290
}
291+
282292
return false;
283293
}
284294

@@ -287,8 +297,8 @@ protected function getDefaultDispatcher(): string
287297
return 'event_dispatcher';
288298
}
289299

290-
protected function grabEventCollector(string $function): EventDataCollector
300+
protected function grabEventCollector(string $callingFunction): EventDataCollector
291301
{
292-
return $this->grabCollector(DataCollectorName::EVENTS, $function);
302+
return $this->grabCollector(DataCollectorName::EVENTS, $callingFunction);
293303
}
294304
}

0 commit comments

Comments
 (0)