Skip to content

Conversation

@terabytesoftw
Copy link
Member

@terabytesoftw terabytesoftw commented Nov 12, 2025

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

Summary by CodeRabbit

  • Tests

    • Refactored memory-related tests and added a new boundary test validating behavior at the 90% memory threshold; test data simplified for clarity.
  • Chores

    • Updated changelog entries (text-only edits).

@terabytesoftw terabytesoftw added the bug Something isn't working label Nov 12, 2025
@coderabbitai
Copy link

coderabbitai bot commented Nov 12, 2025

Walkthrough

Updated changelog text and refined stateless application memory tests: adjusted a data provider and test signature, removed a boolean expectation, and added a boundary test that asserts clean() returns true when memory usage equals the 90% threshold.

Changes

Cohort / File(s) Summary
Changelog Updates
CHANGELOG.md
Replaced Bug #194 entry with Dep #194; added new Bug #195 entry (text-only).
Stateless memory tests
tests/http/stateless/ApplicationMemoryTest.php, tests/provider/StatelessApplicationProvider.php
Simplified memoryThreshold() provider to return (memoryLimit, message) pairs; removed expected-boolean cases and updated phpdoc. Updated testCleanBehaviorWithDifferentMemoryLimits signature to drop the boolean parameter and adjusted assertions; added testCleanReturnsTrueWhenMemoryUsageIsExactlyPercentThreshold() to assert clean() is true at 90% boundary.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant T as Test
  participant A as StatelessApplication
  participant M as MemoryInspector
  note right of T `#DDEBF7`: setup & execute request
  T->>A: execute request
  A->>M: getCurrentMemoryUsage()
  M-->>A: currentUsage
  note right of A `#F7F6D9`: compute limit = currentUsage / 0.9
  T->>A: setMemoryLimit(limit)
  T->>A: call clean()
  A->>M: measure usage inside clean()
  M-->>A: usage
  alt usage <= threshold
    A-->>T: clean() returns true
  else usage > threshold
    A-->>T: clean() returns false
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–25 minutes

  • Pay attention to:
    • The adjusted phpstan/phpdoc return type in tests/provider/StatelessApplicationProvider.php.
    • The updated test method signature and any downstream data-provider usages.
    • The boundary calculation (currentUsage / 0.9) and whether it reliably produces the intended threshold across environments.

Possibly related PRs

Poem

🐰 A rabbit notes in tidy hops,
Changelog tweaked and test bench props,
Ninety percent — a gentle nudge,
Assertions snug, no logic fudge.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a test for memory usage at 90% threshold in ApplicationMemoryTest.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-mini-172

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 34e7b0d and 7a9509d.

📒 Files selected for processing (2)
  • tests/http/stateless/ApplicationMemoryTest.php (2 hunks)
  • tests/provider/StatelessApplicationProvider.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/http/stateless/ApplicationMemoryTest.php
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 180
File: composer.json:64-65
Timestamp: 2025-09-28T11:51:01.274Z
Learning: In the yii2-extensions/psr-bridge project, terabytesoftw prefers removing the `--only-covered` flag from infection mutation testing commands to run mutation testing on all code rather than just covered code paths.
⏰ 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). (20)
  • GitHub Check: mutation / PHP 8.3-ubuntu-latest
  • GitHub Check: phpunit / PHP 8.4-windows-2022
  • GitHub Check: phpunit / PHP 8.3-windows-2022
  • GitHub Check: phpunit / PHP 8.4-ubuntu-latest
  • GitHub Check: phpunit / PHP 8.1-ubuntu-latest
  • GitHub Check: phpunit / PHP 8.2-windows-2022
  • GitHub Check: phpunit / PHP 8.3-ubuntu-latest
  • GitHub Check: phpunit / PHP 8.2-ubuntu-latest
  • GitHub Check: linter / Super Linter
  • GitHub Check: phpunit / PHP 8.3-windows-2022
  • GitHub Check: phpunit / PHP 8.3-ubuntu-latest
  • GitHub Check: phpunit / PHP 8.2-windows-2022
  • GitHub Check: phpunit / PHP 8.2-ubuntu-latest
  • GitHub Check: phpunit / PHP 8.1-ubuntu-latest
  • GitHub Check: phpunit / PHP 8.4-ubuntu-latest
  • GitHub Check: phpunit / PHP 8.1-windows-2022
  • GitHub Check: mutation / PHP 8.3-ubuntu-latest
  • GitHub Check: phpstan / PHP 8.4-ubuntu-latest
  • GitHub Check: composer-require-checker / PHP 8.4-ubuntu-latest
  • GitHub Check: linter / Super Linter
🔇 Additional comments (2)
tests/provider/StatelessApplicationProvider.php (2)

446-450: LGTM! Documentation and type annotations correctly updated.

The docblock and PHPStan type annotation have been properly updated to reflect the simplified data provider structure. The removal of the expected boolean cleanup result makes the provider more focused and easier to maintain.


454-467: Good refactoring! Simplified data provider improves test clarity.

The data structure simplification is well-executed:

  • Removed the boolean expected result, making the provider more straightforward
  • All three test cases consistently verify scenarios where memory usage is below the 90% threshold
  • Clear, descriptive assertion messages for each case

The design improvement of moving the boundary condition test (90% threshold) to a dedicated test method provides better visibility and makes the test suite more maintainable.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Nov 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (b2d19b1) to head (7a9509d).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##                main      #195   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity       362       362           
===========================================
  Files             13        13           
  Lines            867       867           
===========================================
  Hits             867       867           

☔ 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 (1)
tests/http/stateless/ApplicationMemoryTest.php (1)

78-97: Good boundary test addition for clarity.

The test correctly verifies that clean() returns true when memory usage is exactly at the 90% threshold. The math is sound: setting limit = currentUsage / 0.9 ensures that bound = limit * 0.9 = currentUsage, creating the exact boundary condition.

Note that similar logic exists in testCleanBehaviorWithDifferentMemoryLimits (lines 65-73), but having this as a dedicated test improves clarity and makes the boundary condition more explicit and easier to understand.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b2d19b1 and 34e7b0d.

📒 Files selected for processing (2)
  • CHANGELOG.md (1 hunks)
  • tests/http/stateless/ApplicationMemoryTest.php (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 180
File: composer.json:64-65
Timestamp: 2025-09-28T11:51:01.274Z
Learning: In the yii2-extensions/psr-bridge project, terabytesoftw prefers removing the `--only-covered` flag from infection mutation testing commands to run mutation testing on all code rather than just covered code paths.
🧬 Code graph analysis (1)
tests/http/stateless/ApplicationMemoryTest.php (3)
tests/support/TestCase.php (1)
  • statelessApplication (160-220)
src/http/StatelessApplication.php (2)
  • setMemoryLimit (326-335)
  • clean (141-152)
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). (13)
  • GitHub Check: phpunit / PHP 8.1-ubuntu-latest
  • GitHub Check: phpunit / PHP 8.2-windows-2022
  • GitHub Check: phpunit / PHP 8.4-windows-2022
  • GitHub Check: phpunit / PHP 8.4-ubuntu-latest
  • GitHub Check: phpunit / PHP 8.3-ubuntu-latest
  • GitHub Check: phpunit / PHP 8.2-ubuntu-latest
  • GitHub Check: phpunit-dev / PHP 8.6-ubuntu-latest
  • GitHub Check: phpunit-dev / PHP 8.5-ubuntu-latest
  • GitHub Check: phpstan / PHP 8.4-ubuntu-latest
  • GitHub Check: mutation / PHP 8.3-ubuntu-latest
  • GitHub Check: linter / Super Linter
  • GitHub Check: mutation / PHP 8.3-ubuntu-latest
  • GitHub Check: linter / Super Linter
🔇 Additional comments (1)
CHANGELOG.md (1)

9-10: LGTM! Changelog entries are accurate.

The reclassification of #194 as a dependency update and the new Bug #195 entry correctly reflect the changes in this PR.

@terabytesoftw terabytesoftw merged commit 5413a7d into main Nov 12, 2025
50 checks passed
@terabytesoftw terabytesoftw deleted the fix-mini-172 branch November 12, 2025 11:02
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