-
-
Notifications
You must be signed in to change notification settings - Fork 0
fix: Update php-forge/support
version 0.2
in composer.json
and refactor assertions in test cases.
#89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…refactor assertions in test cases.
WalkthroughUpdates dev dependency php-forge/support to ^0.2, integrates TestSupport trait into the test base class, replaces direct Assert helper usage with TestCase wrappers in tests, and adds a changelog entry documenting Bug #89. No runtime or public API changes. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 #89 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 133 133
===========================================
Files 4 4
Lines 522 522
===========================================
Hits 522 522 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this 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/base/AbstractCacheManagement.php (1)
570-584
: Optional: prefer strict comparisons in assertions returning integersWhere asserting scalar integers after cache invalidation, assertSame provides stronger guarantees than assertEquals.
Here’s a minimal example pattern (apply where appropriate):
- self::assertEquals(0, self::invokeMethod($behavior, 'getDepthValue')) + self::assertSame(0, self::invokeMethod($behavior, 'getDepthValue'))
📜 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 (4)
CHANGELOG.md
(1 hunks)composer.json
(1 hunks)tests/TestCase.php
(2 hunks)tests/base/AbstractCacheManagement.php
(14 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-07-07T12:38:55.463Z
Learnt from: terabytesoftw
PR: yii2-extensions/nested-sets-behavior#66
File: migrations/m250707_103609_tree.php:0-0
Timestamp: 2025-07-07T12:38:55.463Z
Learning: In the yii2-extensions/nested-sets-behavior project, terabytesoftw prefers application-level validation through the NestedSetsBehavior class rather than database-level constraints for nested set integrity. The migrations are meant to be flexible examples since developers may create their own tables.
Applied to files:
tests/TestCase.php
tests/base/AbstractCacheManagement.php
📚 Learning: 2025-07-08T11:14:30.395Z
Learnt from: terabytesoftw
PR: yii2-extensions/nested-sets-behavior#71
File: tests/mssql/QueryBehaviorTest.php:36-55
Timestamp: 2025-07-08T11:14:30.395Z
Learning: In the yii2-extensions/nested-sets-behavior project, terabytesoftw prefers explicit hard-coded test credentials in unit test files rather than environment variables, as getenv() returns false in their test environment. For unit tests, explicitly defining connection data is considered acceptable and more reliable than environment variable configuration.
Applied to files:
tests/TestCase.php
tests/base/AbstractCacheManagement.php
📚 Learning: 2025-07-08T11:14:34.647Z
Learnt from: terabytesoftw
PR: yii2-extensions/nested-sets-behavior#71
File: tests/mssql/ValidationAndStructureTest.php:45-49
Timestamp: 2025-07-08T11:14:34.647Z
Learning: In the yii2-extensions/nested-sets-behavior project, terabytesoftw prefers explicit connection data (including passwords) in unit test files rather than environment variables, since getEnv() can return false and explicit values ensure more reliable test execution.
Applied to files:
tests/TestCase.php
tests/base/AbstractCacheManagement.php
📚 Learning: 2025-07-08T11:14:31.363Z
Learnt from: terabytesoftw
PR: yii2-extensions/nested-sets-behavior#71
File: tests/mssql/ValidationAndStructureTest.php:50-53
Timestamp: 2025-07-08T11:14:31.363Z
Learning: In the yii2-extensions/nested-sets-behavior project, terabytesoftw uses hardcoded database connection credentials in unit tests because environment variables are not reliable in their test environment (getEnv() returns false). They consider hardcoded connection data acceptable for unit tests.
Applied to files:
tests/TestCase.php
tests/base/AbstractCacheManagement.php
📚 Learning: 2025-07-08T11:14:24.042Z
Learnt from: terabytesoftw
PR: yii2-extensions/nested-sets-behavior#71
File: tests/mssql/NodePrependTest.php:35-54
Timestamp: 2025-07-08T11:14:24.042Z
Learning: In the yii2-extensions/nested-sets-behavior project, terabytesoftw prefers hard-coded database credentials in unit test files rather than environment variables, as `getEnv()` returns false in their testing setup and explicit connection data is more reliable for unit tests.
Applied to files:
tests/TestCase.php
tests/base/AbstractCacheManagement.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). (18)
- GitHub Check: SQL Server tests. / PHP 8.4-mssql-2022-latest
- GitHub Check: phpunit-compatibility / PHP 8.2-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.4-windows-latest
- GitHub Check: phpunit / PHP 8.4-windows-latest
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-windows-latest
- GitHub Check: phpunit / PHP 8.2-windows-latest
- GitHub Check: PostgreSQL tests. / PHP 8.4-pgsql-17
- GitHub Check: MySQL tests. / PHP 8.4-mysql-8.0
- GitHub Check: MySQL tests. / PHP 8.4-mysql-8.4
- GitHub Check: MySQL tests. / PHP 8.4-mysql-latest
- GitHub Check: PostgreSQL tests. / PHP 8.4-pgsql-15
- GitHub Check: PostgreSQL tests. / PHP 8.4-pgsql-16
- GitHub Check: Oracle tests. / PHP 8.4-oci-23
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-windows-latest
- GitHub Check: Oracle tests. / PHP 8.4-oci-23
- GitHub Check: SQL Server tests. / PHP 8.4-mssql-2022-latest
🔇 Additional comments (6)
composer.json (2)
14-14
: Double-check version constraint '^22' for yiisoft/yii2Is the intent to allow Yii 2.2+ (which Composer would write as '^2.2') or literally major 22 (i.e., '^22')? If it’s a typo, consider correcting to '^2.2'.
If confirmed, I can prepare a follow-up diff to adjust the constraint accordingly.
21-21
: Approve dev dependency bump to php-forge/support ^0.2No composer.lock present (typical for libraries), so there’s nothing further to update.
Dev-only change—no runtime impact expected.CHANGELOG.md (1)
8-8
: Changelog entry accurately reflects the PR scopeNotes the dev dependency bump and test assertion refactor; consistent with the code changes.
tests/TestCase.php (1)
7-7
: Adopting TestSupport trait to expose testing helpers is appropriateImporting and using PHPForge\Support\TestSupport at the base TestCase level cleanly centralizes helpers for all tests, aligning with the dependency bump.
Confirm that TestSupport’s helper methods (invokeMethod, inaccessibleProperty) are defined as static in v0.2. If they’re instance methods, switch call sites to $this->… (see example below) to avoid “non-static method called statically” notices in PHP 8.1+.
- self::invokeMethod($behavior, 'getDepthValue') + $this->invokeMethod($behavior, 'getDepthValue')Also applies to: 73-73
tests/base/AbstractCacheManagement.php (2)
101-101
: Refactor to use self::invokeMethod aligns with TestSupportReplacing external Assert helpers with the TestSupport-based wrappers reduces coupling and keeps helpers within the test base class. This reads well across the various tests.
Please confirm TestSupport declares invokeMethod as static. If not, switch to $this->invokeMethod across these sites to avoid static calls to non-static methods.
Also applies to: 106-106, 111-111, 286-286, 291-291, 296-296, 324-324, 329-329, 334-334, 377-377, 383-383, 387-387, 397-397, 403-403, 407-407, 436-436, 444-444, 482-482, 490-490, 527-527, 535-535, 572-572, 578-578, 592-595
453-453
: Refactor to use self::inaccessibleProperty is consistentThe replacement is consistent and reduces reliance on the previous Assert helper. Property names (depthValue, leftValue, rightValue, node, operation) match expected internals and assertions remain clear.
Same note as above: ensure inaccessibleProperty is a static helper in TestSupport; otherwise prefer $this->inaccessibleProperty for PHP 8.x compatibility.
Also applies to: 499-499, 544-544, 597-597, 601-601, 605-605, 616-616, 620-620, 624-624, 628-628, 632-632
Summary by CodeRabbit