test(http): Move event trigger test from ApplicationTest to ApplicationCoreTest for better organization.
#142
Conversation
…ationCoreTest` for better organization.
WalkthroughAdds and reorganizes stateless application event tests: introduces parameterized and event-order tests in Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test
participant R as PSR-7 Request
participant A as StatelessApplication
participant E as EventDispatcher
participant H as RequestHandler
participant S as Response
T->>R: createRequest(...)
T->>A: handle(R)
A->>E: trigger(BEFORE_REQUEST)
E-->>A: listeners run
A->>H: dispatch to handler
H-->>A: returns Response
A->>E: trigger(AFTER_REQUEST)
E-->>A: listeners run
A-->>T: return Response
Note right of A #e2f0d9: Tests assert order: BEFORE -> handler -> AFTER
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #142 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 318 318
===========================================
Files 12 12
Lines 808 808
===========================================
Hits 808 808 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/http/stateless/ApplicationCoreTest.php (1)
363-385: Event-trigger test relocation is appropriate and verifies both BEFORE/AFTER events.The test is clear and minimal. Optional: strengthen it by asserting a single invocation and that the event sender is the application instance. This guards against accidental double-dispatch and ensures the correct source without increasing fragility. Also, nice that this test avoids superglobal mutations; consistent with the base TestCase cleanup policy noted in prior learnings.
Apply this diff to tighten the assertions:
- $eventTriggered = false; + $invocations = 0; + $sender = null; $app = $this->statelessApplication(); $app->on( $eventName, - static function () use (&$eventTriggered): void { - $eventTriggered = true; + static function ($event) use (&$invocations, &$sender): void { + $invocations++; + $sender = $event->sender ?? null; }, ); $app->handle(FactoryHelper::createServerRequestCreator()->createFromGlobals()); - self::assertTrue($eventTriggered, "Should trigger '{$eventName}' event during handle()"); + self::assertSame(1, $invocations, "Should trigger '{$eventName}' exactly once during handle()."); + self::assertSame($app, $sender, "Event sender should be the application instance.");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
tests/http/stateless/ApplicationCoreTest.php(3 hunks)tests/http/stateless/ApplicationTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-08-24T11:52:50.524Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.524Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
tests/http/stateless/ApplicationCoreTest.phptests/http/stateless/ApplicationTest.php
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods, so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/stateless/ApplicationCoreTest.phptests/http/stateless/ApplicationTest.php
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods (lines 28 and 32), so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/stateless/ApplicationCoreTest.phptests/http/stateless/ApplicationTest.php
📚 Learning: 2025-07-20T16:33:57.495Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.495Z
Learning: The TestCase class in yii2-extensions/psr-bridge automatically handles $_SERVER superglobal cleanup by saving its original state before each test and restoring it afterward in setUp() and tearDown() methods. Manual $_SERVER cleanup in individual test methods is unnecessary when extending this TestCase.
Applied to files:
tests/http/stateless/ApplicationCoreTest.phptests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class (tests/TestCase.php) automatically handles temporary file cleanup. The `createTmpFile()` method tracks files in `$this->tmpFiles[]`, and the `tearDown()` method calls `closeTmpFile()` to clean them up. Tests extending TestCase don't need manual file cleanup when using these methods.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-08T15:24:06.085Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/TestCase.php:23-27
Timestamp: 2025-08-08T15:24:06.085Z
Learning: In yii2-extensions/psr-bridge (tests/TestCase.php), maintainer preference: it’s acceptable to use random-looking strings for test-only constants like COOKIE_VALIDATION_KEY; no need to replace with an obviously non-secret value unless CI/secret scanners become problematic.
Applied to files:
tests/http/stateless/ApplicationCoreTest.phptests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-08T15:28:00.166Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/adapter/ServerRequestAdapterTest.php:2215-2215
Timestamp: 2025-08-08T15:28:00.166Z
Learning: In yii2-extensions/psr-bridge tests, prefer using self::COOKIE_VALIDATION_KEY from tests/TestCase over hardcoded 'cookieValidationKey' strings to avoid secret scanners FP and improve maintainability.
Applied to files:
tests/http/stateless/ApplicationCoreTest.phptests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-03T16:24:09.241Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method, then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
tests/http/stateless/ApplicationCoreTest.phptests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-03T16:24:09.241Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method (line 408: `$this->response = new Response($this->components['response'] ?? []);`), then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-06T22:52:05.608Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#64
File: tests/http/StatelessApplicationTest.php:1939-1967
Timestamp: 2025-08-06T22:52:05.608Z
Learning: In yii2-extensions/psr-bridge tests, when testing specific component methods like Request::resolve(), it's necessary to call $app->handle($request) first to initialize all application components before testing the method in isolation. This ensures proper component lifecycle initialization.
Applied to files:
tests/http/stateless/ApplicationCoreTest.phptests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-08T15:28:00.166Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/adapter/ServerRequestAdapterTest.php:2215-2215
Timestamp: 2025-08-08T15:28:00.166Z
Learning: In yii2-extensions/psr-bridge, tests extend tests/TestCase which defines a protected const COOKIE_VALIDATION_KEY. Test code should use self::COOKIE_VALIDATION_KEY instead of hardcoded cookieValidationKey literals.
Applied to files:
tests/http/stateless/ApplicationTest.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (3)
tests/http/stateless/ApplicationTest.php (1)
7-7: Import cleanup matches the test move (LGTM).Dropping TestWith and keeping Group/RequiresPhpExtension aligns with this file’s current usage after relocating the event test. No remaining references to TestWith here.
tests/http/stateless/ApplicationCoreTest.php (2)
8-8: Added TestWith import is correct.Required for the new parameterized test; matches PHPUnit attribute usage elsewhere in the suite.
17-17: StatelessApplication import is necessary for event constant references.This enables referencing EVENT_BEFORE_REQUEST / EVENT_AFTER_REQUEST directly in attributes. Good call.
… sender instance.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/http/stateless/ApplicationCoreTest.php (1)
363-388: Solid, minimal test for BEFORE/AFTER request events; asserts sender identity and single invocation.Nice move into Core tests; this keeps lifecycle coverage in one place.
Two optional nits to consider:
- Make the route explicit to avoid relying on defaults from globals. This reduces flakiness when server vars vary across environments.
Apply inline:
- $app->handle(FactoryHelper::createServerRequestCreator()->createFromGlobals()); + $_SERVER = [ + 'REQUEST_METHOD' => 'GET', + 'REQUEST_URI' => 'site/index', + ]; + $app->handle(FactoryHelper::createServerRequestCreator()->createFromGlobals());
- Consider an additional test to assert event ordering (BEFORE fires before AFTER) in a single handle() execution. This complements the parameterized single-event assertions without changing their intent.
Example (outside this hunk):
public function testEventOrderDuringHandle(): void { $app = $this->statelessApplication(); $sequence = []; $app->on(StatelessApplication::EVENT_BEFORE_REQUEST, static function () use (&$sequence): void { $sequence[] = 'before'; }); $app->on(StatelessApplication::EVENT_AFTER_REQUEST, static function () use (&$sequence): void { $sequence[] = 'after'; }); $_SERVER = ['REQUEST_METHOD' => 'GET', 'REQUEST_URI' => 'site/index']; $app->handle(FactoryHelper::createServerRequestCreator()->createFromGlobals()); self::assertSame(['before', 'after'], $sequence, 'BEFORE should precede AFTER during handle().'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/http/stateless/ApplicationCoreTest.php(2 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-08-24T11:52:50.524Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.524Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods, so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods (lines 28 and 32), so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-07-20T16:33:57.495Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.495Z
Learning: The TestCase class in yii2-extensions/psr-bridge automatically handles $_SERVER superglobal cleanup by saving its original state before each test and restoring it afterward in setUp() and tearDown() methods. Manual $_SERVER cleanup in individual test methods is unnecessary when extending this TestCase.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-03T16:24:09.241Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method, then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-06T22:52:05.608Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#64
File: tests/http/StatelessApplicationTest.php:1939-1967
Timestamp: 2025-08-06T22:52:05.608Z
Learning: In yii2-extensions/psr-bridge tests, when testing specific component methods like Request::resolve(), it's necessary to call $app->handle($request) first to initialize all application components before testing the method in isolation. This ensures proper component lifecycle initialization.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-08T15:24:06.085Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/TestCase.php:23-27
Timestamp: 2025-08-08T15:24:06.085Z
Learning: In yii2-extensions/psr-bridge (tests/TestCase.php), maintainer preference: it’s acceptable to use random-looking strings for test-only constants like COOKIE_VALIDATION_KEY; no need to replace with an obviously non-secret value unless CI/secret scanners become problematic.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-08T15:28:00.166Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/adapter/ServerRequestAdapterTest.php:2215-2215
Timestamp: 2025-08-08T15:28:00.166Z
Learning: In yii2-extensions/psr-bridge tests, prefer using self::COOKIE_VALIDATION_KEY from tests/TestCase over hardcoded 'cookieValidationKey' strings to avoid secret scanners FP and improve maintainability.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-03T16:24:09.241Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method (line 408: `$this->response = new Response($this->components['response'] ?? []);`), then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
🧬 Code graph analysis (1)
tests/http/stateless/ApplicationCoreTest.php (3)
src/http/StatelessApplication.php (2)
StatelessApplication(49-553)handle(269-295)tests/TestCase.php (1)
statelessApplication(134-197)tests/support/FactoryHelper.php (2)
FactoryHelper(46-268)createServerRequestCreator(154-161)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (3)
tests/http/stateless/ApplicationCoreTest.php (3)
12-12: Importing yii\base\Event is correct and aligns with the new listener closure typehint.
17-17: Using StatelessApplication for event constants inside attributes is a clean choice.This avoids stringly-typed event names and keeps tests coupled to the source-of-truth constants.
8-8: ✅ PHPUnit 10+ requirement confirmed
composer.json declaresphpunit/phpunitat ^10.5 (major 10), so the#[TestWith]attribute is fully supported in CI. No further changes needed.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/http/stateless/ApplicationCoreTest.php (1)
336-367: Implement and invoke a reset method for mocked microtimeThe
MockerFunctionsstub currently only providessetMockedMicrotime(float $time): voidand no way to clear or reset the mock, which means once one test sets it, subsequent tests will inherit that value. To ensure full isolation, you should:• In
tests/support/stub/MockerFunctions.php(around line 218), add a reset method, for example:public static function resetMockedMicrotime(): void { // Allow null to indicate “no mock” self::$mockedMicrotime = null; }And update the property and
setMockedMicrotimesignature to accept?floatif necessary:- private static float $mockedMicrotime; + private static ?float $mockedMicrotime = null; // ... - public static function setMockedMicrotime(float $time): void + public static function setMockedMicrotime(?float $time): void• In
tests/http/stateless/ApplicationCoreTest.php(around lines 336–367), wrap the mock in atry/finallyor invoke the reset intearDown()to always clear it:$mockedTime = 1640995200.123456; try { MockerFunctions::setMockedMicrotime($mockedTime); $app = $this->statelessApplication(); $response = $app->handle(FactoryHelper::createRequest('GET', 'site/index')); } finally { MockerFunctions::resetMockedMicrotime(); }This guarantees that no global state lingers between tests.
🧹 Nitpick comments (5)
tests/http/stateless/ApplicationCoreTest.php (5)
54-55: Prefer a deterministic request over createFromGlobals in testsUsing createFromGlobals can couple this test to the runner’s environment. Since the test purpose is container resolution, a deterministic request is safer and faster.
- $app->handle(FactoryHelper::createServerRequestCreator()->createFromGlobals()); + $app->handle(FactoryHelper::createRequest('GET', 'site/index'));
133-140: Make the multi-request test fully deterministic (avoid globals here too)You set $_SERVER and then still build the request via createFromGlobals(). To reduce flakiness and improve readability, prefer FactoryHelper::createRequest(...) and drop the superglobal manipulation altogether in this test. The base TestCase already handles $_SERVER cleanup per learnings, but we can still keep this test self-contained and deterministic.
- // first request - verify adapter caching behavior - $_SERVER = [ - 'REQUEST_METHOD' => 'GET', - 'REQUEST_URI' => 'site/index', - ]; - - $response1 = $app->handle(FactoryHelper::createServerRequestCreator()->createFromGlobals()); + // first request - verify adapter caching behavior + $response1 = $app->handle(FactoryHelper::createRequest('GET', 'site/index')); @@ - // second request with different route - verify stateless behavior - $_SERVER = [ - 'REQUEST_METHOD' => 'GET', - 'REQUEST_URI' => 'site/statuscode', - ]; - - $response2 = $app->handle(FactoryHelper::createServerRequestCreator()->createFromGlobals()); + // second request with different route - verify stateless behavior + $response2 = $app->handle(FactoryHelper::createRequest('GET', 'site/statuscode')); @@ - $_COOKIE = ['test_cookie' => 'test_value']; - $_SERVER = [ - 'REQUEST_METHOD' => 'GET', - 'REQUEST_URI' => 'site/add-cookies-to-response', - ]; - - $response3 = $app->handle(FactoryHelper::createServerRequestCreator()->createFromGlobals()); + $response3 = $app->handle(FactoryHelper::createRequest('GET', 'site/add-cookies-to-response'));Note: Leaving the globals as-is is acceptable per maintainer preference, so treat this as optional.
Also applies to: 172-179, 225-232
271-274: Reduce brittleness of Set-Cookie assertionsExact header concatenation can break if the attribute order changes or a runtime adds attributes. Assert presence of individual cookie lines instead.
- self::assertSame( - 'test=test; Path=/; HttpOnly; SameSite=Lax test2=test2; Path=/; HttpOnly; SameSite=Lax', - implode(' ', $cookieHeaders), - 'PSR-7 Response Set-Cookie headers should match the expected values, confirming correct adapter behavior.', - ); + self::assertContains( + 'test=test; Path=/; HttpOnly; SameSite=Lax', + $cookieHeaders, + "Missing 'test' cookie header.", + ); + self::assertContains( + 'test2=test2; Path=/; HttpOnly; SameSite=Lax', + $cookieHeaders, + "Missing 'test2' cookie header.", + );
225-226: Clarify or remove unused $_COOKIE setupSetting $_COOKIE = ['test_cookie' => 'test_value'] isn't asserted later and can confuse readers. Either remove it or assert that it doesn’t leak into response headers.
Option A (remove):
- $_COOKIE = ['test_cookie' => 'test_value'];Option B (assert no leakage):
self::assertFalse( str_contains(implode(' ', $cookieHeaders), 'test_cookie=test_value'), 'Incoming cookies should not be echoed back in Set-Cookie headers.' );
403-444: Parametrized event test is great; assert event name and consider error-path coverageThe test ensures each event fires once and the sender is correct. Tighten it slightly by asserting the collected name matches the dataset. Optionally, add a dataset to cover the exception path where AFTER_REQUEST is triggered from handleError().
$app->on( $eventName, static function (Event $event) use (&$invocations, &$sequence, &$sender): void { $invocations++; $sender = $event->sender ?? null; $sequence[] = $event->name; }, ); @@ self::assertSame( $app, $sender, 'Event sender should be the application instance.', ); + self::assertSame( + [$eventName], + $sequence, + 'Collected event name should match the dataset.' + );Optional: add a failing-route/exception dataset to assert AFTER_REQUEST still fires on error flow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/http/stateless/ApplicationCoreTest.php(8 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-08-24T11:52:50.524Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.524Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods, so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods (lines 28 and 32), so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-07-20T16:33:57.495Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.495Z
Learning: The TestCase class in yii2-extensions/psr-bridge automatically handles $_SERVER superglobal cleanup by saving its original state before each test and restoring it afterward in setUp() and tearDown() methods. Manual $_SERVER cleanup in individual test methods is unnecessary when extending this TestCase.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-03T16:24:09.241Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method, then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-06T22:52:05.608Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#64
File: tests/http/StatelessApplicationTest.php:1939-1967
Timestamp: 2025-08-06T22:52:05.608Z
Learning: In yii2-extensions/psr-bridge tests, when testing specific component methods like Request::resolve(), it's necessary to call $app->handle($request) first to initialize all application components before testing the method in isolation. This ensures proper component lifecycle initialization.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-08T15:24:06.085Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/TestCase.php:23-27
Timestamp: 2025-08-08T15:24:06.085Z
Learning: In yii2-extensions/psr-bridge (tests/TestCase.php), maintainer preference: it’s acceptable to use random-looking strings for test-only constants like COOKIE_VALIDATION_KEY; no need to replace with an obviously non-secret value unless CI/secret scanners become problematic.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-08T15:28:00.166Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/adapter/ServerRequestAdapterTest.php:2215-2215
Timestamp: 2025-08-08T15:28:00.166Z
Learning: In yii2-extensions/psr-bridge tests, prefer using self::COOKIE_VALIDATION_KEY from tests/TestCase over hardcoded 'cookieValidationKey' strings to avoid secret scanners FP and improve maintainability.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-03T16:24:09.241Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method (line 408: `$this->response = new Response($this->components['response'] ?? []);`), then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
🧬 Code graph analysis (1)
tests/http/stateless/ApplicationCoreTest.php (3)
src/http/StatelessApplication.php (2)
StatelessApplication(49-553)handle(269-295)tests/TestCase.php (1)
statelessApplication(134-197)tests/support/FactoryHelper.php (2)
FactoryHelper(46-268)createRequest(68-89)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (3)
tests/http/stateless/ApplicationCoreTest.php (3)
88-123: LGTM: Solid coverage for BEFORE/AFTER orderingThis verifies the event sequence during handle() and asserts a valid response shape. Nice and focused.
376-387: LGTM: Extra response assertions improve signalCapturing and asserting on the response here adds good coverage that the route is wired as expected before alias checks.
8-18: Confirm PHPUnit Attribute Support and CI ConfigurationI’ve verified the following after running the suggested checks:
- composer.json “require-dev” sets
phpunit/phpunit: ^10.5, so attribute-based tests (e.g.#[TestWith]) are supported by PHPUnit 10+.FactoryHelper::createServerRequestCreator()is defined intests/support/FactoryHelper.php(lines 154–156).- Tests across the suite consistently import and use PHPUnit attributes (
Group,TestWith,DataProviderExternal,RequiresPhpExtension, etc.).Next steps:
- Ensure all CI workflows (e.g. GitHub Actions, Travis CI) install and invoke PHPUnit 10.5 or later, so attribute-based tests run correctly in automated builds.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
tests/http/stateless/ApplicationCoreTest.php (3)
168-168: Same URI normalization as above.Use '/site/statuscode' for consistency.
215-215: Same URI normalization as above.Use '/site/add-cookies-to-response' for consistency.
331-331: Same URI normalization as earlier.Use '/site/index' for consistency. Also, nice alignment with the retrieved learnings to avoid superglobal mutations by using FactoryHelper::createRequest().
🧹 Nitpick comments (4)
tests/http/stateless/ApplicationCoreTest.php (4)
134-134: Prefer absolute-path URIs in test requests for clarity.FactoryHelper accepts relative paths, but using a leading slash avoids ambiguity and mirrors typical HTTP request targets.
Apply:
- $response1 = $app->handle(FactoryHelper::createRequest('GET', 'site/index')); + $response1 = $app->handle(FactoryHelper::createRequest('GET', '/site/index'));
254-261: Avoid brittle exact Set-Cookie matching; assert by parts.Exact header equality is fragile across PHP/SAPI/PSR-7 implementations (attribute order, additional flags). Assert presence and essential attributes instead.
Apply:
- self::assertContains( - 'test=test; Path=/; HttpOnly; SameSite=Lax', - $cookieHeaders, - 'PSR-7 Response Set-Cookie headers should match the expected values, confirming correct adapter behavior.', - ); - self::assertContains( - 'test2=test2; Path=/; HttpOnly; SameSite=Lax', - $cookieHeaders, - 'PSR-7 Response Set-Cookie headers should match the expected values, confirming correct adapter behavior.', - ); + $hasTestCookie = array_filter( + $cookieHeaders, + static fn(string $h): bool => + str_contains($h, 'test=test') + && str_contains($h, 'Path=/') + && str_contains($h, 'HttpOnly') + && str_contains($h, 'SameSite=Lax'), + ) !== []; + $hasTest2Cookie = array_filter( + $cookieHeaders, + static fn(string $h): bool => + str_contains($h, 'test2=test2') + && str_contains($h, 'Path=/') + && str_contains($h, 'HttpOnly') + && str_contains($h, 'SameSite=Lax'), + ) !== []; + self::assertTrue( + $hasTestCookie, + "Expected a 'Set-Cookie' header for 'test' with required attributes.", + ); + self::assertTrue( + $hasTest2Cookie, + "Expected a 'Set-Cookie' header for 'test2' with required attributes.", + );
365-376: Good addition of status and Content-Type assertions; consider DRYing with a tiny helper.These exact assertions repeat across multiple tests; a private helper improves readability and maintenance.
Apply within this method:
- $response = $app->handle(FactoryHelper::createRequest('GET', 'site/index')); - - self::assertSame( - 200, - $response->getStatusCode(), - "Expected HTTP '200' for route 'site/index'.", - ); - self::assertSame( - 'application/json; charset=UTF-8', - $response->getHeaderLine('Content-Type'), - "Expected Content-Type 'application/json; charset=UTF-8' for route 'site/index'.", - ); + $response = $app->handle(FactoryHelper::createRequest('GET', '/site/index')); + $this->assertOkJsonIndex($response);Add this helper to the class (outside the shown range):
private function assertOkJsonIndex(\Psr\Http\Message\ResponseInterface $response): void { self::assertSame(200, $response->getStatusCode(), "Expected HTTP '200' for route 'site/index'."); self::assertSame( 'application/json; charset=UTF-8', $response->getHeaderLine('Content-Type'), "Expected Content-Type 'application/json; charset=UTF-8' for route 'site/index'.", ); }
389-433: Parametrized event trigger test looks solid; add an assertion on the recorded event name and optionally verify handler cleanup.Asserting the captured sequence guards against accidental cross-event emissions. Verifying handler cleanup ensures terminate() unbinds listeners.
Apply:
self::assertSame( $app, $sender, 'Event sender should be the application instance.', ); + self::assertSame( + [$eventName], + $sequence, + 'Only the parameterized event should be recorded by the listener.', + ); + + // Optional: verify cleanup removed listeners for this sender/event. + self::assertFalse( + Event::hasHandlers($app, $eventName), + 'After handle(), application-specific handlers should be detached by cleanupEvents().', + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/http/stateless/ApplicationCoreTest.php(9 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-08-24T11:52:50.524Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.524Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods, so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods (lines 28 and 32), so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-07-20T16:33:57.495Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.495Z
Learning: The TestCase class in yii2-extensions/psr-bridge automatically handles $_SERVER superglobal cleanup by saving its original state before each test and restoring it afterward in setUp() and tearDown() methods. Manual $_SERVER cleanup in individual test methods is unnecessary when extending this TestCase.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-03T16:24:09.241Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method, then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-06T22:52:05.608Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#64
File: tests/http/StatelessApplicationTest.php:1939-1967
Timestamp: 2025-08-06T22:52:05.608Z
Learning: In yii2-extensions/psr-bridge tests, when testing specific component methods like Request::resolve(), it's necessary to call $app->handle($request) first to initialize all application components before testing the method in isolation. This ensures proper component lifecycle initialization.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-08T15:24:06.085Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/TestCase.php:23-27
Timestamp: 2025-08-08T15:24:06.085Z
Learning: In yii2-extensions/psr-bridge (tests/TestCase.php), maintainer preference: it’s acceptable to use random-looking strings for test-only constants like COOKIE_VALIDATION_KEY; no need to replace with an obviously non-secret value unless CI/secret scanners become problematic.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-08T15:28:00.166Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/adapter/ServerRequestAdapterTest.php:2215-2215
Timestamp: 2025-08-08T15:28:00.166Z
Learning: In yii2-extensions/psr-bridge tests, prefer using self::COOKIE_VALIDATION_KEY from tests/TestCase over hardcoded 'cookieValidationKey' strings to avoid secret scanners FP and improve maintainability.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
📚 Learning: 2025-08-03T16:24:09.241Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method (line 408: `$this->response = new Response($this->components['response'] ?? []);`), then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
🧬 Code graph analysis (1)
tests/http/stateless/ApplicationCoreTest.php (3)
src/http/StatelessApplication.php (2)
StatelessApplication(49-553)handle(269-295)tests/TestCase.php (1)
statelessApplication(134-197)tests/support/FactoryHelper.php (2)
FactoryHelper(46-268)createRequest(68-89)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (4)
tests/http/stateless/ApplicationCoreTest.php (4)
12-12: Importing Event is correct and necessary for typed listener closures.Good addition; avoids FQCN clutter in callbacks.
17-17: Importing StatelessApplication for event constants is appropriate.This keeps tests readable and avoids magic strings for event names.
88-123: Event order test is clear and deterministic—LGTM.Concise setup, explicit order assertion, and response sanity checks make this robust.
8-8: No action needed: PHPUnit attributes (including TestWith) are supportedThe project’s composer.json requires PHPUnit ^10.5, which fully supports PHP 8 attributes, and a scan of the test suite shows that PHPUnit\Framework\Attributes (including TestWith) is already in widespread use:
- composer.json declares
phpunit/phpunit: "^10.5"- Numerous tests import and use attributes such as
TestWithandGroup(e.g., in tests/http/stateless/ApplicationCoreTest.php and many others)You can safely keep the
TestWithattribute here without falling back to docblock annotations.
Summary by CodeRabbit