GH#716: test(limits,domain-mapping): add unit tests for Trial_Limits and SSO classes#718
Conversation
📝 WalkthroughWalkthroughThe PR adds two comprehensive test suites for previously untested classes: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
…classes - Add Trial_Limits_Test.php with 10 tests covering singleton behavior, init method, and load_limitations - Add SSO_Test.php with 9 tests covering deprecated SSO compatibility shim - Both classes now have 100% test coverage (previously 0%) - Tests verify singleton pattern, method existence, and deprecation documentation - Closes #716
superdav42
force-pushed
the
feature/gh716-trial-limits-sso-tests
branch
from
c24fed8 to
c9ffca6
Compare
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
tests/WP_Ultimo/Limits/Trial_Limits_Test.php (2)
118-139: Consider using$this->expectNotToPerformAssertions()for "no error" tests.The
$this->assertTrue(true)pattern works but is considered a weak assertion. PHPUnit provides$this->expectNotToPerformAssertions()specifically for tests that verify no exception is thrown.🔧 Alternative approach
public function test_load_limitations_can_be_called() { + $this->expectNotToPerformAssertions(); $instance = $this->get_instance(); // Should not throw any errors $instance->load_limitations(); - - $this->assertTrue(true); } /** * Test init can be called without errors. */ public function test_init_can_be_called() { + $this->expectNotToPerformAssertions(); $instance = $this->get_instance(); // Should not throw any errors $instance->init(); - - $this->assertTrue(true); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Limits/Trial_Limits_Test.php` around lines 118 - 139, Replace the weak no-op assertions in test_load_limitations_can_be_called and test_init_can_be_called: instead of calling $this->assertTrue(true) after invoking $this->get_instance()->load_limitations() and $this->get_instance()->init(), call $this->expectNotToPerformAssertions() at the start of each test so the tests explicitly state they only assert that no exceptions are thrown when calling load_limitations() and init().
1-8: Missingdefined('ABSPATH') || exit;guard.Per coding guidelines, every PHP file should start with
defined('ABSPATH') || exit;. While test files have relaxed requirements, adding this guard maintains consistency across the codebase.🔧 Suggested fix
<?php + +defined('ABSPATH') || exit; namespace WP_Ultimo\Limits;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Limits/Trial_Limits_Test.php` around lines 1 - 8, Add the standard WordPress file guard at the top of the test file by inserting defined('ABSPATH') || exit; as the first executable statement so the file short-circuits when loaded outside WP; update the tests/WP_Ultimo/Limits/Trial_Limits_Test.php file above the namespace declaration (before the namespace and the Trial_Limits_Test class) to maintain consistency with coding guidelines.tests/WP_Ultimo/Domain_Mapping/SSO_Test.php (2)
1-11: Missingdefined('ABSPATH') || exit;guard.Same as the other test file - consider adding the ABSPATH guard for consistency.
🔧 Suggested fix
<?php + +defined('ABSPATH') || exit; namespace WP_Ultimo\Domain_Mapping\SSO;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Domain_Mapping/SSO_Test.php` around lines 1 - 11, Add the WordPress ABSPATH guard to this test file by inserting a defined('ABSPATH') || exit; check immediately after the opening PHP tag and before the namespace declaration so the file exits when loaded outside WP; update the SSO_Test class file (the class named SSO_Test in the WP_Ultimo\Domain_Mapping\SSO namespace) to include that guard for consistency with other tests.
116-138: LGTM on public method verification.The test correctly verifies that the SSO class exposes only the methods from the Singleton trait (
get_instance,init,has_parents), confirming it's a minimal compatibility shim with no additional functionality.One minor improvement: use strict comparison in
in_array()for type safety.🔧 Suggested strict comparison
// Filter out Singleton methods $non_singleton_methods = array_filter($method_names, function ($name) { - return !in_array($name, ['get_instance', 'init', 'has_parents']); + return !in_array($name, ['get_instance', 'init', 'has_parents'], true); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Domain_Mapping/SSO_Test.php` around lines 116 - 138, In test_has_only_singleton_public_methods update the in_array calls to use strict comparison by passing the third argument true; specifically modify the in_array usage inside the array_filter closure that builds $non_singleton_methods (and any other in_array call in this test) so it becomes in_array($name, ['get_instance','init','has_parents'], true) to ensure type-safe matching when filtering public method names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/WP_Ultimo/Domain_Mapping/SSO_Test.php`:
- Around line 1-11: Add the WordPress ABSPATH guard to this test file by
inserting a defined('ABSPATH') || exit; check immediately after the opening PHP
tag and before the namespace declaration so the file exits when loaded outside
WP; update the SSO_Test class file (the class named SSO_Test in the
WP_Ultimo\Domain_Mapping\SSO namespace) to include that guard for consistency
with other tests.
- Around line 116-138: In test_has_only_singleton_public_methods update the
in_array calls to use strict comparison by passing the third argument true;
specifically modify the in_array usage inside the array_filter closure that
builds $non_singleton_methods (and any other in_array call in this test) so it
becomes in_array($name, ['get_instance','init','has_parents'], true) to ensure
type-safe matching when filtering public method names.
In `@tests/WP_Ultimo/Limits/Trial_Limits_Test.php`:
- Around line 118-139: Replace the weak no-op assertions in
test_load_limitations_can_be_called and test_init_can_be_called: instead of
calling $this->assertTrue(true) after invoking
$this->get_instance()->load_limitations() and $this->get_instance()->init(),
call $this->expectNotToPerformAssertions() at the start of each test so the
tests explicitly state they only assert that no exceptions are thrown when
calling load_limitations() and init().
- Around line 1-8: Add the standard WordPress file guard at the top of the test
file by inserting defined('ABSPATH') || exit; as the first executable statement
so the file short-circuits when loaded outside WP; update the
tests/WP_Ultimo/Limits/Trial_Limits_Test.php file above the namespace
declaration (before the namespace and the Trial_Limits_Test class) to maintain
consistency with coding guidelines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bc359ba8-2329-4bde-b5bf-21580431f597
📒 Files selected for processing (2)
tests/WP_Ultimo/Domain_Mapping/SSO_Test.phptests/WP_Ultimo/Limits/Trial_Limits_Test.php
|
Performance Test Results Performance test results for c8205ac are in 🛎️! Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown. URL:
|
What Was DoneAdded comprehensive unit tests for two previously untested classes:
Both classes now have 100% test coverage (previously 0%). Testing EvidenceRuntime verified - All tests pass: vendor/bin/phpunit tests/WP_Ultimo/Limits/Trial_Limits_Test.php
# OK (10 tests, 12 assertions)
vendor/bin/phpunit tests/WP_Ultimo/Domain_Mapping/SSO_Test.php
# OK (9 tests, 14 assertions)Coverage report confirms 100% coverage:
All CI checks passed (PHP 8.2-8.5, Cypress E2E, Code Quality, PHP Lint). Key Decisions
Files Changed
BlockersNone Follow-upNone - both classes are simple singletons with minimal logic. Coverage is complete. Released InMerged to main via PR #718 Closes #716 aidevops.sh v3.5.470 plugin for OpenCode v1.3.0 with claude-sonnet-4-5 |
1 participant
Summary
Adds comprehensive unit tests for two previously untested classes:
inc/limits/class-trial-limits.php) - 10 tests covering singleton behavior, init method, and load_limitationsinc/domain-mapping/class-sso.php) - 9 tests covering the deprecated SSO compatibility shimBoth classes now have 100% test coverage (previously 0%).
Testing Evidence
Runtime verified - All tests pass:
Coverage report confirms 100% coverage for both classes:
Key Decisions
newInstanceWithoutConstructor()to bypass singleton init() which may have dependenciesFiles Changed
tests/WP_Ultimo/Limits/Trial_Limits_Test.php(new, 143 lines)tests/WP_Ultimo/Domain_Mapping/SSO_Test.php(new, 140 lines)Follow-up
None - both classes are simple singletons with minimal logic. Coverage is complete.
Closes #716
aidevops.sh v3.5.468 plugin for OpenCode v1.3.0 with claude-sonnet-4-5
Summary by CodeRabbit