GH#713: write unit tests for Base_PayPal_Gateway#717
Conversation
- Create Base_PayPal_Gateway_Test.php with 33 tests covering all public and protected methods - Test URL generation (sandbox vs live mode for payments and subscriptions) - Test subscription ID type detection (REST API I- prefix vs legacy NVP) - Test partner attribution header injection - Test subscription description truncation and HTML entity decoding - Test site actions hook integration with membership gateway matching - Test support flags (recurring, amount_update) - Achieve 100% code coverage (48/48 lines) for Base_PayPal_Gateway class Closes #713
📝 WalkthroughWalkthroughA new comprehensive test file was added for the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
🔨 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: |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/WP_Ultimo/Gateways/Base_PayPal_Gateway_Test.php (3)
649-656: UseassertNotFalse()instead ofassertTrue()forhas_filter()return value.
has_filter()returns the lowest priority (integer, e.g.,10) if callbacks exist, orfalseif none. UsingassertTrue()works because10is truthy, but it's semantically misleading. The assertion should reflect the actual return type.🧹 Suggested fix
public function test_hooks_registers_site_actions_filter(): void { // Remove any existing hooks first. remove_all_filters( 'wu_element_get_site_actions' ); $this->gateway->hooks(); - $this->assertTrue( has_filter( 'wu_element_get_site_actions' ) ); + $this->assertNotFalse( has_filter( 'wu_element_get_site_actions' ) ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Gateways/Base_PayPal_Gateway_Test.php` around lines 649 - 656, The test test_hooks_registers_site_actions_filter uses assertTrue on has_filter, but has_filter returns an int priority or false; update the assertion to assertNotFalse(has_filter('wu_element_get_site_actions')) so the test correctly checks that a filter exists; locate the test method test_hooks_registers_site_actions_filter in Base_PayPal_Gateway_Test and replace the assertTrue call with assertNotFalse while keeping the remove_all_filters and $this->gateway->hooks() calls unchanged.
587-604: Log tests only verify no exceptions—consider adding behavior verification.The tests acknowledge the limitation but provide no actual verification of logging behavior. If this is intentional due to difficulty mocking global functions, consider adding a comment noting this as a coverage gap or using a filter/action to intercept log calls if one exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Gateways/Base_PayPal_Gateway_Test.php` around lines 587 - 604, The tests test_log_calls_wu_log_add_with_default_level and test_log_calls_wu_log_add_with_custom_level only assert no exception and don't verify logging behavior; either add a clear comment noting this is an accepted coverage gap due to the unmockable global wu_log_add, or modify the tests to verify behavior by intercepting the log (e.g., attach a WP filter/action or wrapper that your Base_PayPal_Gateway::public_log uses) and assert it received the expected message/level when calling public_log on the gateway; update the test names/comments to reflect which approach you chose and reference the public_log method as the target to hook or document.
17-19: Unused import:Siteclass is not referenced in this file.The
Sitemodel is imported but never used in the test file. Consider removing it to keep imports clean.🧹 Suggested fix
use WP_Ultimo\Models\Customer; use WP_Ultimo\Models\Membership; -use WP_Ultimo\Models\Site;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Gateways/Base_PayPal_Gateway_Test.php` around lines 17 - 19, The import for the Site model is unused in Base_PayPal_Gateway_Test: remove the line "use WP_Ultimo\Models\Site;" from the top of the test file (while keeping the existing imports for Customer and Membership) and run the tests to confirm there are no references to Site elsewhere in the class.
🤖 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/Gateways/Base_PayPal_Gateway_Test.php`:
- Around line 649-656: The test test_hooks_registers_site_actions_filter uses
assertTrue on has_filter, but has_filter returns an int priority or false;
update the assertion to
assertNotFalse(has_filter('wu_element_get_site_actions')) so the test correctly
checks that a filter exists; locate the test method
test_hooks_registers_site_actions_filter in Base_PayPal_Gateway_Test and replace
the assertTrue call with assertNotFalse while keeping the remove_all_filters and
$this->gateway->hooks() calls unchanged.
- Around line 587-604: The tests test_log_calls_wu_log_add_with_default_level
and test_log_calls_wu_log_add_with_custom_level only assert no exception and
don't verify logging behavior; either add a clear comment noting this is an
accepted coverage gap due to the unmockable global wu_log_add, or modify the
tests to verify behavior by intercepting the log (e.g., attach a WP
filter/action or wrapper that your Base_PayPal_Gateway::public_log uses) and
assert it received the expected message/level when calling public_log on the
gateway; update the test names/comments to reflect which approach you chose and
reference the public_log method as the target to hook or document.
- Around line 17-19: The import for the Site model is unused in
Base_PayPal_Gateway_Test: remove the line "use WP_Ultimo\Models\Site;" from the
top of the test file (while keeping the existing imports for Customer and
Membership) and run the tests to confirm there are no references to Site
elsewhere in the class.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 32c6c431-aa12-4af5-9e98-00f5e5d2d707
📒 Files selected for processing (1)
tests/WP_Ultimo/Gateways/Base_PayPal_Gateway_Test.php
|
Performance Test Results Performance test results for 1151206 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:
|
🔨 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: |
Summary
Implements comprehensive unit tests for the
Base_PayPal_Gatewayclass, achieving 100% code coverage (48/48 lines).Changes
tests/WP_Ultimo/Gateways/Base_PayPal_Gateway_Test.phpwith 33 testsI-prefix vs legacy NVP)ULTIMATE_SP_PPCPBN code)supports_recurring,supports_amount_update)Test Results
Verification
All tests pass in multisite environment:
Closes #713
aidevops.sh v3.5.468 plugin for OpenCode v1.3.0 with claude-sonnet-4-5 spent 4m and 11,869 tokens on this as a headless worker.
Summary by CodeRabbit