t549: Write PHPUnit tests for PayPal_OAuth_Handler to reach >=80% coverage#554
Conversation
…dge cases - Add tests for install_webhook_after_oauth indirect coverage via handle_oauth_return - Add tests for delete_webhooks_on_disconnect indirect coverage via ajax_disconnect - Add tests for AJAX methods without nonce (security coverage) - Add tests for empty/missing optional fields in OAuth return - Add tests for malformed proxy responses - Add standalone test file for direct webhook method testing (for future use) - Test coverage improvements target 80%+ for PayPal_OAuth_Handler class
- Add test for verify_merchant_via_proxy test mode parameter - Add test for ajax_initiate_oauth request body structure - Add test for ajax_disconnect deauthorize request parameters - Verify non-blocking deauthorize request behavior - Test correct testMode flag propagation in all proxy requests
- Document all test improvements made - List all methods now covered - Provide test execution instructions - Explain indirect testing approach for protected methods
📝 WalkthroughWalkthroughAdds extensive PHPUnit tests for PayPal_OAuth_Handler: a standalone test runnable outside WordPress, expanded tests in the existing test file covering OAuth/webhook flows, AJAX/nonces, proxy responses, and a coverage-summary document. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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: |
|
Performance Test Results Performance test results for 5a5611f are in 🛎️! URL:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Test.php (1)
1896-1908: Remove unused$resultvariable assignments.The
$resultvariable is assigned but never used. The test's intent is to capture the request body, not verify the return value.♻️ Proposed fix
// Test with test_mode = true (default) - $result = $method->invoke($this->handler, 'MERCHANT123', 'TRACKING456'); + $method->invoke($this->handler, 'MERCHANT123', 'TRACKING456'); $this->assertIsArray($captured_body); $this->assertTrue($captured_body['testMode']); // Test with test_mode = false $test_mode_prop = $reflection->getProperty('test_mode'); $test_mode_prop->setValue($this->handler, false); - $result = $method->invoke($this->handler, 'MERCHANT789', 'TRACKING789'); + $method->invoke($this->handler, 'MERCHANT789', 'TRACKING789');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Test.php` around lines 1896 - 1908, Remove the unused $result assignments by calling $method->invoke(...) directly where currently assigned to $result; specifically replace the two lines assigning $result = $method->invoke($this->handler, 'MERCHANT123', 'TRACKING456') and $result = $method->invoke($this->handler, 'MERCHANT789', 'TRACKING789') with bare invocations $method->invoke($this->handler, 'MERCHANT123', 'TRACKING456') and $method->invoke($this->handler, 'MERCHANT789', 'TRACKING789'), keeping the surrounding assertions that inspect $captured_body and the test_mode change via $reflection->getProperty('test_mode')->setValue($this->handler, false).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Standalone_Test.php`:
- Around line 441-471: The test_dead code arises because
expectException()/expectExceptionMessage() causes execution to stop before the
assertions for $json_error run; in test_ajax_disconnect_without_permissions
replace the expectException()/expectExceptionMessage() lines with a try/catch
around $this->handler->ajax_disconnect() (or call it and catch the \Exception
thrown by the wp_send_json_error mock) so execution continues and you can assert
on the captured $json_error array; keep the mock for wp_send_json_error, the
$json_error variable, and the ajax_disconnect() call, but wrap that call in try
{ ... } catch (\Exception $e) { /* swallow to allow assertions */ } and then
perform the assertIsArray/assertArrayHasKey/assertStringContainsString checks.
- Around line 406-436: The test currently sets wp_send_json_error to throw an
exception and then calls $this->expectException(), which prevents the later
assertions from running; change the test to capture the error payload without
throwing so assertions can run: update the wp_send_json_error mock in
test_ajax_initiate_oauth_without_permissions to set $json_error and return (do
not throw), remove the $this->expectException()/$this->expectExceptionMessage()
calls, call $this->handler->ajax_initiate_oauth(), and then run the existing
assertions on $json_error to verify the 'message' contains "do not have
permission"; references: ajax_initiate_oauth, wp_send_json_error, $json_error,
expectException.
- Around line 392-401: The test test_is_oauth_feature_enabled_with_constant in
class PayPal_OAuth_Handler_Standalone_Test defines the WU_PAYPAL_OAUTH_ENABLED
constant which pollutes the global PHPUnit process; update the test to run in
isolation by adding the PHPUnit annotation `@runInSeparateProcess` (and optionally
`@preserveGlobalState` disabled) to the test method (or the test class) so the
constant definition cannot affect other tests, or alternatively change the test
to avoid defining the constant and assert behavior via a different injection
point.
- Around line 250-259: Replace deprecated withConsecutive calls by adding
invocation counters and using willReturnCallback on $mock_gateway methods: for
set_test_mode, remove withConsecutive and change to
->expects($this->exactly(2))->method('set_test_mode')->willReturnCallback(function($mode)
use (&$setModeCount) { $setModeCount = ($setModeCount ?? 0) + 1; if
($setModeCount === 1) $this->assertTrue($mode); elseif ($setModeCount === 2)
$this->assertFalse($mode); }); and for delete_webhook replace
willReturnOnConsecutiveCalls with
->expects($this->exactly(2))->method('delete_webhook')->willReturnCallback(function()
use (&$deleteCount) { $deleteCount = ($deleteCount ?? 0) + 1; return
$deleteCount === 1 ? new \WP_Error('delete_error', 'Sandbox webhook not found')
: new \WP_Error('delete_error', 'Live webhook not found'); }); ensuring you
declare the counters ($setModeCount, $deleteCount) in the test scope so the
callbacks can branch per invocation.
---
Nitpick comments:
In `@tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Test.php`:
- Around line 1896-1908: Remove the unused $result assignments by calling
$method->invoke(...) directly where currently assigned to $result; specifically
replace the two lines assigning $result = $method->invoke($this->handler,
'MERCHANT123', 'TRACKING456') and $result = $method->invoke($this->handler,
'MERCHANT789', 'TRACKING789') with bare invocations
$method->invoke($this->handler, 'MERCHANT123', 'TRACKING456') and
$method->invoke($this->handler, 'MERCHANT789', 'TRACKING789'), keeping the
surrounding assertions that inspect $captured_body and the test_mode change via
$reflection->getProperty('test_mode')->setValue($this->handler, false).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01d80ff3-317a-48a6-9747-4128c6c7848f
📒 Files selected for processing (3)
tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Standalone_Test.phptests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Test.phptests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Test_Coverage_Summary.md
| $mock_gateway->expects($this->exactly(2)) | ||
| ->method('set_test_mode') | ||
| ->withConsecutive([true], [false]); | ||
|
|
||
| $mock_gateway->expects($this->exactly(2)) | ||
| ->method('delete_webhook') | ||
| ->willReturnOnConsecutiveCalls( | ||
| new \WP_Error('delete_error', 'Sandbox webhook not found'), | ||
| new \WP_Error('delete_error', 'Live webhook not found') | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check PHPUnit version in composer.json/lock
echo "=== Checking composer.json ==="
cat composer.json | grep -A5 '"phpunit/phpunit"' || echo "Not found in composer.json"
echo -e "\n=== Checking composer.lock ==="
cat composer.lock 2>/dev/null | grep -A10 '"name": "phpunit/phpunit"' | head -20 || echo "No composer.lock found"Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 937
🌐 Web query:
withConsecutive() PHPUnit 10 11 deprecated removed
💡 Result:
withConsecutive was deprecated in PHPUnit 9.6 and removed in PHPUnit 10. It remains unavailable in PHPUnit 11. Status by version: - PHPUnit 9 (before 9.6): Available - PHPUnit 9.6: Deprecated (warnings issued) - PHPUnit 10: Removed - PHPUnit 11: Removed (no reintroduction) Migration: Replace with willReturnCallback using invocation order tracking, e.g.: $mock->expects($this->exactly(2)) ->method('method') ->willReturnCallback(function (...$parameters) use ($invocation) { $count = $invocation->numberOfInvocations; // PHPUnit 10+ if ($count === 1) { $this->assertSame($expected1, $parameters); return $return1; } if ($count === 2) { $this->assertSame($expected2, $parameters); return $return2; } }); In PHPUnit 13 (released Feb 2026), direct replacements withParameterSetsInOrder and withParameterSetsInAnyOrder are introduced.
Citations:
- 1: Remove withConsecutive() sebastianbergmann/phpunit#4565
- 2: Deprecate withConsecutive() sebastianbergmann/phpunit#5063
- 3: https://github.com/sebastianbergmann/phpunit/blob/10.0.0/ChangeLog-10.0.md
- 4: https://docs.phpunit.de/en/11.0/test-doubles.html
- 5: https://stackoverflow.com/questions/75389000/replace-phpunit-method-withconsecutive-abandoned-in-phpunit-10
- 6: https://tomasvotruba.com/blog/how-to-upgrade-deprecated-phpunit-with-consecutive
- 7: https://phpunit.de/announcements/phpunit-13.html
- 8: https://phpunit.expert/articles/better-than-withconsecutive.html
withConsecutive() is deprecated in PHPUnit 9.6 and removed in PHPUnit 10+.
The project currently uses PHPUnit 9.6.34, where this method already emits deprecation warnings. It will fail completely if upgraded to PHPUnit 10. Refactor to use willReturnCallback() with invocation tracking for compatibility with current and future PHPUnit versions.
♻️ Alternative using callback
- $mock_gateway->expects($this->exactly(2))
- ->method('set_test_mode')
- ->withConsecutive([true], [false]);
+ $mock_gateway->expects($this->exactly(2))
+ ->method('set_test_mode')
+ ->willReturnCallback(function($mode) use (&$set_test_mode_calls) {
+ $set_test_mode_calls[] = $mode;
+ });
+ // After test execution, verify: $this->assertEquals([true, false], $set_test_mode_calls);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Standalone_Test.php` around
lines 250 - 259, Replace deprecated withConsecutive calls by adding invocation
counters and using willReturnCallback on $mock_gateway methods: for
set_test_mode, remove withConsecutive and change to
->expects($this->exactly(2))->method('set_test_mode')->willReturnCallback(function($mode)
use (&$setModeCount) { $setModeCount = ($setModeCount ?? 0) + 1; if
($setModeCount === 1) $this->assertTrue($mode); elseif ($setModeCount === 2)
$this->assertFalse($mode); }); and for delete_webhook replace
willReturnOnConsecutiveCalls with
->expects($this->exactly(2))->method('delete_webhook')->willReturnCallback(function()
use (&$deleteCount) { $deleteCount = ($deleteCount ?? 0) + 1; return
$deleteCount === 1 ? new \WP_Error('delete_error', 'Sandbox webhook not found')
: new \WP_Error('delete_error', 'Live webhook not found'); }); ensuring you
declare the counters ($setModeCount, $deleteCount) in the test scope so the
callbacks can branch per invocation.
- Replace deprecated withConsecutive with willReturnCallback pattern in test_delete_webhooks_on_disconnect_with_errors and _success tests - Add @runInSeparateProcess + @preserveGlobalState disabled to test_is_oauth_feature_enabled_with_constant to prevent constant pollution - Move assertions into wp_send_json_error mock closure to fix dead code after expectException in test_ajax_initiate_oauth_without_permissions and test_ajax_disconnect_without_permissions - Remove unused $result assignments in verify_merchant_via_proxy test
🔨 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.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Standalone_Test.php`:
- Line 33: The test class's static property $mock_functions is declared
protected which causes fatal errors when namespace-level mock shim functions
(accessing \PayPal_OAuth_Handler_Standalone_Test::$mock_functions) try to use
it; change the declaration of $mock_functions in the
PayPal_OAuth_Handler_Standalone_Test class from protected static to public
static so external functions can access and modify it, keeping the same default
value (e.g., public static $mock_functions = []); update any related
docblocks/comments if present.
- Around line 522-540: The fallback WP_Error class is being declared inside the
WP_Ultimo\Gateways namespace so new \WP_Error(...) (global) fails in standalone
tests; after the class declaration for WP_Error add a global alias (using
class_alias) so the class is available as \WP_Error, or alternatively declare
the class in the global namespace; update the code near the WP_Error definition
and the class_exists check to ensure the global symbol \WP_Error is registered
so calls like new \WP_Error(...) succeed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5aa0c256-d801-4409-95d9-bded93d3da08
📒 Files selected for processing (2)
tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Standalone_Test.phptests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Test.php
| * | ||
| * @var array | ||
| */ | ||
| protected static $mock_functions = []; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "protected static \$mock_functions|PayPal_OAuth_Handler_Standalone_Test::\\$mock_functions" tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Standalone_Test.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 323
🏁 Script executed:
cat -n tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Standalone_Test.php | sed -n '1,50p'Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1306
🏁 Script executed:
cat -n tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Standalone_Test.php | sed -n '30,40p'Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 301
🏁 Script executed:
cat -n tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Standalone_Test.php | sed -n '480,530p'Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 2135
$mock_functions visibility will fatally break all mock shims.
Line 33 declares protected static $mock_functions, but lines 487–516 access it from namespace-level functions defined outside the class. PHP visibility rules prevent protected properties from being accessed outside the class scope, regardless of namespace. Any call to these mock functions will throw Fatal error: Cannot access protected property.
🐛 Proposed fix
- protected static $mock_functions = [];
+ public static $mock_functions = [];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| protected static $mock_functions = []; | |
| public static $mock_functions = []; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Standalone_Test.php` at line
33, The test class's static property $mock_functions is declared protected which
causes fatal errors when namespace-level mock shim functions (accessing
\PayPal_OAuth_Handler_Standalone_Test::$mock_functions) try to use it; change
the declaration of $mock_functions in the PayPal_OAuth_Handler_Standalone_Test
class from protected static to public static so external functions can access
and modify it, keeping the same default value (e.g., public static
$mock_functions = []); update any related docblocks/comments if present.
| if (!class_exists('\WP_Error')) { | ||
| class WP_Error { | ||
| private $code; | ||
| private $message; | ||
|
|
||
| public function __construct($code = '', $message = '', $data = '') { | ||
| $this->code = $code; | ||
| $this->message = $message; | ||
| } | ||
|
|
||
| public function get_error_code() { | ||
| return $this->code; | ||
| } | ||
|
|
||
| public function get_error_message() { | ||
| return $this->message; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "class_exists\\('\\\\WP_Error'\\)|class WP_Error|new \\\\WP_Error|class_alias" tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Standalone_Test.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 369
🏁 Script executed:
head -30 tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Standalone_Test.php | grep -E "namespace|use "Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 142
🏁 Script executed:
sed -n '515,545p' tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Standalone_Test.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 850
WP_Error fallback is declared in the wrong namespace, causing new \WP_Error(...) calls to fail in standalone mode.
The test file declares namespace WP_Ultimo\Gateways; at the top. When the fallback class is declared with class WP_Error {, it gets placed in the WP_Ultimo\Gateways namespace, not the global namespace. The code checks if (!class_exists('\WP_Error')) but then instantiates with new \WP_Error(...) (lines 96, 263, 265), which looks for the class in the global namespace. In standalone mode, this causes runtime failures.
Add a class alias after the class declaration to make it accessible globally:
Fix
if (!class_exists('\WP_Error')) {
class WP_Error {
private $code;
private $message;
public function __construct($code = '', $message = '', $data = '') {
$this->code = $code;
$this->message = $message;
}
public function get_error_code() {
return $this->code;
}
public function get_error_message() {
return $this->message;
}
}
+ class_alias(__NAMESPACE__ . '\WP_Error', 'WP_Error');
}🧰 Tools
🪛 PHPMD (2.15.0)
[warning] 527-527: Avoid unused parameters such as '$data'. (undefined)
(UnusedFormalParameter)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Standalone_Test.php` around
lines 522 - 540, The fallback WP_Error class is being declared inside the
WP_Ultimo\Gateways namespace so new \WP_Error(...) (global) fails in standalone
tests; after the class declaration for WP_Error add a global alias (using
class_alias) so the class is available as \WP_Error, or alternatively declare
the class in the global namespace; update the code near the WP_Error definition
and the class_exists check to ensure the global symbol \WP_Error is registered
so calls like new \WP_Error(...) succeed.
1 participant
Summary
Changes Made
Test Coverage Improvements
Webhook Methods (Indirect Coverage)
install_webhook_after_oauthtested viahandle_oauth_returndelete_webhooks_on_disconnecttested viaajax_disconnectSecurity Tests
Edge Cases
Additional Files
Testing Instructions
composer install./bin/install-wp-tests.sh wordpress_test root '' localhost latestvendor/bin/phpunit tests/WP_Ultimo/Gateways/PayPal_OAuth_Handler_Test.php --coverage-text --coverage-filter=inc/gateways/class-paypal-oauth-handler.phpNotes
Closes #549
Summary by CodeRabbit
Tests
Documentation