Skip to content

Conversation

@terabytesoftw
Copy link
Member

@terabytesoftw terabytesoftw commented Aug 22, 2025

Q A
Is bugfix? ✔️
New feature?
Breaks BC?

Summary by CodeRabbit

  • Tests
    • Converted memory-limit checks to a parameterized, data-driven test to remove duplication and clarify intent.
    • Added a shared data provider supplying multiple positive memory-limit cases (small KB, mid/large MB, near 32-bit max; includes an extra 64-bit-only 4GiB case).
    • Removed redundant single-case tests while retaining coverage for override/recalculation behavior.

…telessApplicationProvider` and refactor related tests in `ApplicationMemoryTest` class.
@terabytesoftw terabytesoftw added the bug Something isn't working label Aug 22, 2025
@coderabbitai
Copy link

coderabbitai bot commented Aug 22, 2025

Walkthrough

Refactors memory-limit tests into a single data-driven PHPUnit test using an external provider. Adds StatelessApplicationProvider::memoryLimitPositive() returning multiple positive memory-limit cases. Removes three legacy tests and updates ApplicationMemoryTest to use #[DataProviderExternal(...)] and a parameterized assertion.

Changes

Cohort / File(s) Summary
Stateless HTTP tests
tests/http/stateless/ApplicationMemoryTest.php
Replaced three specific tests with one parameterized test testSetMemoryLimitWithPositiveValueSetsLimitDirectly(int $memoryLimit, string $assertionMessage) annotated with #[DataProviderExternal(StatelessApplicationProvider::class, 'memoryLimitPositive')]; retained testSetMemoryLimitWithPositiveValueOverridesSystemRecalculation; removed the former testSetMemoryLimitWithLargePositiveValueMaintainsValue, testSetMemoryLimitWithPositiveValueDisablesRecalculation, and testSetMemoryLimitWithSmallPositiveValueSetsCorrectly methods; added necessary imports.
Test data providers
tests/provider/StatelessApplicationProvider.php
Added public static function memoryLimitPositive(): array returning named cases (1KB, 128MB, 256MB, near INT_MAX; plus 4GiB on 64-bit) as (int, string) tuples and annotated the return shape for static analysis.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant P as PHPUnit Runner
  participant DP as StatelessApplicationProvider.memoryLimitPositive()
  participant T as ApplicationMemoryTest
  participant A as StatelessApplication (SUT)

  P->>DP: retrieve data sets
  loop for each dataset
    P->>T: invoke testSetMemoryLimitWithPositiveValueSetsLimitDirectly(memoryLimit, message)
    T->>A: setMemoryLimit(memoryLimit)
    A-->>T: current memory limit
    T-->>P: assertEquals(memoryLimit, actual) with message
  end
  Note over DP,T: Data-driven execution via DataProviderExternal
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

I nibble bytes and hop through rows,
One test now feeds the cases it knows.
From tiny crumbs to giant store,
I count the limits, then I score.
Carrot-coded, quick and spry — tests pass by. 🥕


📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between a45ac7a and cf9b525.

📒 Files selected for processing (1)
  • tests/provider/StatelessApplicationProvider.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/provider/StatelessApplicationProvider.php
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-mini-108

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codecov
Copy link

codecov bot commented Aug 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (8744481) to head (cf9b525).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##                main      #127   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity       318       318           
===========================================
  Files             12        12           
  Lines            808       808           
===========================================
  Hits             808       808           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
tests/provider/StatelessApplicationProvider.php (1)

195-220: Consider adding a guarded 64-bit case and aligning units (MB vs MiB).

  • Optional: include a 64-bit–only case (e.g., 4 GiB) to catch regressions where larger limits might be narrowed. Guard it with PHP_INT_SIZE to keep 32-bit runs green.
  • Nit: messages use powers-of-two byte counts while saying “MB”. Consider “MiB” to avoid ambiguity.

Apply this diff inside memoryLimitPositive() to add the 64-bit case without breaking 32-bit:

     public static function memoryLimitPositive(): array
     {
-        return [
+        $data = [
             'small value 1KB' => [
                 1024,
                 1024,
                 "Memory limit should be set to exactly '1024' bytes ('1KB') when positive value is provided.",
             ],
             'medium value 128MB' => [
                 134_217_728,
                 134_217_728,
                 "Memory limit should be set to exactly '134_217_728' bytes ('128MB') when positive value is provided.",
             ],
             'large value 256MB' => [
                 268_435_456,
                 268_435_456,
                 "Memory limit should be set to exactly '268_435_456' bytes ('256MB') when positive value is provided.",
             ],
             'very large value near 32-bit INT_MAX' => [
                 2_147_483_647,
                 2_147_483_647,
                 'Memory limit should handle large positive values correctly without overflow when set to ' .
                 "'2_147_483_647' bytes.",
             ],
-        ];
+        ];
+
+        if (PHP_INT_SIZE >= 8) {
+            $data['ultra large value 4GiB (64-bit only)'] = [
+                4_294_967_296,
+                4_294_967_296,
+                "Memory limit should be set to exactly '4_294_967_296' bytes ('4GiB') when a positive value is provided on 64-bit.",
+            ];
+        }
+
+        return $data;
     }
tests/http/stateless/ApplicationMemoryTest.php (2)

351-367: Strengthen the assertion by checking pre- and post-handle state.

Setting the limit should take effect immediately and remain stable across handle(); asserting both guards against future regressions where handle() might reset the value.

Apply this diff:

     public function testSetMemoryLimitWithPositiveValueSetsLimitDirectly(
         int $memoryLimit,
         int $expectedLimit,
         string $assertionMessage,
     ): void {
         $app = $this->statelessApplication();

         $app->setMemoryLimit($memoryLimit);

-        $app->handle(FactoryHelper::createServerRequestCreator()->createFromGlobals());
+        // Assert before any request is handled.
+        self::assertSame(
+            $expectedLimit,
+            $app->getMemoryLimit(),
+            $assertionMessage
+        );
+
+        // Assert remains unchanged after handling a request.
+        $app->handle(FactoryHelper::createServerRequestCreator()->createFromGlobals());

         self::assertSame(
             $expectedLimit,
             $app->getMemoryLimit(),
             $assertionMessage,
         );
     }

351-367: Optionally simplify the dataset by deriving the expected value in-test.

Since expectedLimit equals memoryLimit in all cases, you can drop the second integer from the provider to reduce redundancy and noise.

If you choose to simplify, update the test signature and assertions:

-    public function testSetMemoryLimitWithPositiveValueSetsLimitDirectly(
-        int $memoryLimit,
-        int $expectedLimit,
-        string $assertionMessage,
-    ): void {
+    public function testSetMemoryLimitWithPositiveValueSetsLimitDirectly(
+        int $memoryLimit,
+        string $assertionMessage,
+    ): void {
         $app = $this->statelessApplication();

         $app->setMemoryLimit($memoryLimit);

-        self::assertSame(
-            $expectedLimit,
+        self::assertSame(
+            $memoryLimit,
             $app->getMemoryLimit(),
             $assertionMessage,
         );

-        $app->handle(FactoryHelper::createServerRequestCreator()->createFromGlobals());
+        $app->handle(FactoryHelper::createServerRequestCreator()->createFromGlobals());

-        self::assertSame(
-            $expectedLimit,
+        self::assertSame(
+            $memoryLimit,
             $app->getMemoryLimit(),
             $assertionMessage,
         );
     }

And adjust the provider accordingly (outside this file):

-    /**
-     * @phpstan-return array<string, array{int, int, string}>
-     */
+    /**
+     * @phpstan-return array<string, array{int, string}>
+     */
     public static function memoryLimitPositive(): array
     {
-        return [
+        return [
             'small value 1KB' => [
-                1024,
-                1024,
+                1024,
                 "Memory limit should be set to exactly '1024' bytes ('1KB') when positive value is provided.",
             ],
             'medium value 128MB' => [
-                134_217_728,
-                134_217_728,
+                134_217_728,
                 "Memory limit should be set to exactly '134_217_728' bytes ('128MB') when positive value is provided.",
             ],
             'large value 256MB' => [
-                268_435_456,
-                268_435_456,
+                268_435_456,
                 "Memory limit should be set to exactly '268_435_456' bytes ('256MB') when positive value is provided.",
             ],
             'very large value near 32-bit INT_MAX' => [
-                2_147_483_647,
-                2_147_483_647,
+                2_147_483_647,
                 'Memory limit should handle large positive values correctly without overflow when set to ' .
                 "'2_147_483_647' bytes.",
             ],
         ];
     }
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 8744481 and 8df5ba5.

📒 Files selected for processing (2)
  • tests/http/stateless/ApplicationMemoryTest.php (2 hunks)
  • tests/provider/StatelessApplicationProvider.php (1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 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/ApplicationMemoryTest.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/ApplicationMemoryTest.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/ApplicationMemoryTest.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/ApplicationMemoryTest.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/ApplicationMemoryTest.php
🧬 Code graph analysis (1)
tests/http/stateless/ApplicationMemoryTest.php (2)
tests/provider/StatelessApplicationProvider.php (1)
  • StatelessApplicationProvider (11-221)
src/http/StatelessApplication.php (3)
  • setMemoryLimit (326-335)
  • handle (269-295)
  • getMemoryLimit (236-245)
⏰ 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). (4)
  • GitHub Check: phpunit / PHP 8.4-windows-latest
  • GitHub Check: phpunit / PHP 8.3-windows-latest
  • GitHub Check: mutation / PHP 8.3-ubuntu-latest
  • GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (3)
tests/provider/StatelessApplicationProvider.php (1)

192-220: Good, focused dataset for positive memory limits.

Covers small/medium/large plus 32-bit boundary nicely; phpstan shape is precise and helpful for IDEs/static analysis.

tests/http/stateless/ApplicationMemoryTest.php (2)

11-11: Import is correct and localized.

The provider FQCN is accurate; keeps tests decoupled from dataset definitions.


7-7: ✅ PHPUnit 10+ Support Verified for DataProviderExternal

– composer.json specifies PHPUnit “^10.5” in require-dev, which satisfies the minimum PHPUnit 10 requirement for the DataProviderExternal attribute.
– PHP version requirement is “>=8.1”, ensuring native attribute syntax is supported.

No further changes needed.

…itional test case for `4GiB` limit on `64-bit` systems.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
tests/provider/StatelessApplicationProvider.php (1)

220-221: Tighten wording in assertion message (nit).

Minor copy edit for clarity and consistency with other messages.

Apply this diff:

-                "provided on '64-bit'.",
+                'provided on 64-bit builds.',
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 8df5ba5 and a45ac7a.

📒 Files selected for processing (2)
  • tests/http/stateless/ApplicationMemoryTest.php (2 hunks)
  • tests/provider/StatelessApplicationProvider.php (1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 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/ApplicationMemoryTest.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/ApplicationMemoryTest.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/ApplicationMemoryTest.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/ApplicationMemoryTest.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/ApplicationMemoryTest.php
🧬 Code graph analysis (1)
tests/http/stateless/ApplicationMemoryTest.php (3)
tests/TestCase.php (1)
  • statelessApplication (134-197)
src/http/StatelessApplication.php (3)
  • setMemoryLimit (326-335)
  • getMemoryLimit (236-245)
  • handle (269-295)
tests/support/FactoryHelper.php (2)
  • FactoryHelper (46-268)
  • createServerRequestCreator (154-161)
🪛 GitHub Check: phpstan / PHP 8.4-ubuntu-latest
tests/provider/StatelessApplicationProvider.php

[failure] 225-225:
Method yii2\extensions\psrbridge\tests\provider\StatelessApplicationProvider::memoryLimitPositive() should return array<string, array{int, int, string}> but returns array{'small value 1KB': array{1024, 'Memory limit should…'}, 'medium value 128MB': array{134217728, 'Memory limit should…'}, 'large value 256MB': array{268435456, 'Memory limit should…'}, 'very large value near 32-bit INT_MAX': array{2147483647, 'Memory limit should…'}, 'ultra large value 4GiB (64-bit only)'?: array{4294967296, 'Memory limit should…'}}.

🪛 GitHub Check: mutation / PHP 8.3-ubuntu-latest
tests/provider/StatelessApplicationProvider.php

[failure] 225-225:
Method yii2\extensions\psrbridge\tests\provider\StatelessApplicationProvider::memoryLimitPositive() should return array<string, array{int, int, string}> but returns array{'small value 1KB': array{1024, 'Memory limit should…'}, 'medium value 128MB': array{134217728, 'Memory limit should…'}, 'large value 256MB': array{268435456, 'Memory limit should…'}, 'very large value near 32-bit INT_MAX': array{2147483647, 'Memory limit should…'}, 'ultra large value 4GiB (64-bit only)'?: array{4294967296, 'Memory limit should…'}}.

⏰ 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: phpunit / PHP 8.3-windows-latest
  • GitHub Check: phpunit / PHP 8.1-windows-latest
🔇 Additional comments (3)
tests/http/stateless/ApplicationMemoryTest.php (3)

351-365: Nice conversion to a single data-driven test.

  • Covers multiple positive limits succinctly and asserts stability across a handled request.
  • Messages are descriptive and will help when a case fails.

11-11: PSR-4 autoload-dev mapping confirmed

The Composer autoload-dev configuration includes the expected PSR-4 entry for the tests namespace:

"autoload-dev": {
  "psr-4": {
    "yii2\\extensions\\psrbridge\\tests\\": "tests"
  }
}

With "yii2\\extensions\\psrbridge\\tests\\": "tests", the StatelessApplicationProvider in tests/provider/StatelessApplicationProvider.php will be autoloaded correctly. No further changes are required.


7-7: PHPUnit version constraint includes DataProviderExternal attribute

The require-dev section in composer.json specifies phpunit/phpunit: ^10.5, which covers PHPUnit 10.5.0 and above—where the #[DataProviderExternal] attribute was introduced(docs.phpunit.de). No further changes are required.

@terabytesoftw
Copy link
Member Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Aug 22, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants