test(http): Rename test method for clarity and remove redundant test for unlimited memory limit in ApplicationMemoryTest class.
#130
Conversation
…for unlimited memory limit in `ApplicationMemoryTest` class.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #130 +/- ##
===========================================
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 review |
✅ Actions performedReview triggered.
|
WalkthroughRenamed and consolidated unlimited-memory tests in ApplicationMemoryTest: removed a duplicate test, merged pre- and post-handle assertions into a single test method, and updated assertion messages and method name. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ 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 (
|
![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/ApplicationMemoryTest.php (1)
171-195: Guardini_setwithtry/finallyand isolate the test in its own processPHPUnit 10.5 includes the
#[RunInSeparateProcess]attribute for method-level process isolation (docs.phpunit.de). To ensure your globalmemory_limitchange can never leak (even if an assertion fails), you should:
- Wrap the
ini_set('memory_limit', '-1')call in atry { … } finally { … }so that the original value is always restored.- Annotate the test with
#[RunInSeparateProcess](or/** @runInSeparateProcess */if you prefer the docblock form).- Add the necessary import for the attribute.
Suggested diff:
--- a/tests/http/stateless/ApplicationMemoryTest.php +++ b/tests/http/stateless/ApplicationMemoryTest.php @@ -use PHPUnit\Framework\Attributes\{DataProviderExternal, Group, TestWith}; +use PHPUnit\Framework\Attributes\{DataProviderExternal, Group, TestWith, RunInSeparateProcess}; #[RunInSeparateProcess] public function testMemoryLimitHandlesUnlimitedValues(): void { $originalLimit = ini_get('memory_limit'); - ini_set('memory_limit', '-1'); - - $app = $this->statelessApplication(); - - self::assertSame( - PHP_INT_MAX, - $app->getMemoryLimit(), - "Memory limit should be 'PHP_INT_MAX' when set to '-1' (unlimited) in 'StatelessApplication'.", - ); - - $app->handle(FactoryHelper::createServerRequestCreator()->createFromGlobals()); - $app->clean(); - - self::assertSame( - PHP_INT_MAX, - $app->getMemoryLimit(), - "Memory limit should be 'PHP_INT_MAX' when set to '-1' (unlimited) in 'StatelessApplication'.", - ); - - ini_set('memory_limit', $originalLimit); + try { + ini_set('memory_limit', '-1'); + + $app = $this->statelessApplication(); + + self::assertSame( + PHP_INT_MAX, + $app->getMemoryLimit(), + "Before handle(): Memory limit should be 'PHP_INT_MAX' when set to '-1' (unlimited).", + ); + + $app->handle(FactoryHelper::createServerRequestCreator()->createFromGlobals()); + $app->clean(); + + self::assertSame( + PHP_INT_MAX, + $app->getMemoryLimit(), + "After clean(): Memory limit should remain 'PHP_INT_MAX' when set to '-1'.", + ); + } finally { + ini_set('memory_limit', $originalLimit); + } }If you’re on an older PHPUnit that doesn’t support attributes, replace
#[RunInSeparateProcess]with a docblock:/** * @runInSeparateProcess */No import is needed for the annotation.
📜 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/ApplicationMemoryTest.php(2 hunks)
🔇 Additional comments (1)
tests/http/stateless/ApplicationMemoryTest.php (1)
171-195: Rename + consolidation looks goodThe renamed test reads clearer and the consolidation of the unlimited-memory assertions into one method makes sense.
Summary by CodeRabbit