-
Notifications
You must be signed in to change notification settings - Fork 216
Chore/fix php test cases #2929
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
base: develop
Are you sure you want to change the base?
Chore/fix php test cases #2929
Conversation
WalkthroughIntroduces HPOS-aware test bootstrap and installation, renames DokanTestCase queue helpers, replaces pending-task runner across Analytics/Product tests, adjusts several Orders/Commission tests and metadata, removes a DB assertion trait, adds a WooCommerce CouponFactory, and updates various REST/Withdrawal tests with grouping and setup tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Tester
participant Bootstrap as tests/php/bootstrap.php
participant WC as WooCommerce
participant COTC as CustomOrdersTableController
participant DB as Database
Tester->>Bootstrap: Load test bootstrap
Bootstrap->>Bootstrap: Enable HPOS, sync options (update_option)
Bootstrap->>WC: Load plugins (Woo, Dokan)
Bootstrap->>WC: install_wc()
WC->>DB: Create core + analytics tables
alt COTC available
Bootstrap->>COTC: create_tables()
COTC->>DB: Create HPOS tables
end
Bootstrap->>DB: Truncate known tables (if exists)
Bootstrap-->>Tester: Ready (HPOS enabled)
sequenceDiagram
autonumber
actor Test as PHPUnit Test
participant DTC as DokanTestCase
participant Q as WC_Helper_Queue
Test->>DTC: run_all_pending_queue()
DTC->>Q: run_all_pending()
Q-->>DTC: All queued actions processed
Test->>DTC: cancel_all_pending_queue()
DTC->>Q: cancel_all_pending()
Q-->>DTC: Queue cleared
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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 |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/php/src/Withdrawal/MakeDefaultWithdrawalMethodTest.php (1)
23-27: Rename the test function to match its behavior.The doc comment (line 23) states "set a default withdrawal method," but the function name (line 27) is
test_that_we_can_remove_withdraw_schedule(). This mismatch creates confusion about the test's actual purpose.Apply this diff to rename the function:
- public function test_that_we_can_remove_withdraw_schedule() { + public function test_that_we_can_set_default_withdrawal_method() {tests/php/src/Orders/OrderStatusChangeTest.php (1)
164-185: Remove processing→cancelled from sub-order whitelist
In includes/Order/Hooks.php, delete 'wc-cancelled' from thewc-processingentry of$default_whitelistso that sub-orders in processing can no longer transition to cancelled.
🧹 Nitpick comments (2)
tests/php/src/REST/VendorProductCategoriesApiTest.php (1)
23-24: Add a PHPDoc for$categoriesfor clarityDocumenting the property type helps readers and static analysers. Consider:
+ /** @var array<string,int> */ protected $categories;tests/php/src/Factories/CouponFactory.php (1)
49-58: Fix docblock parameter nameThe docblock still references
$coupon_code, but the method accepts$coupon_id; adjust the annotation to avoid confusing future readers.- * @param string $coupon_code + * @param int $coupon_id
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
tests/php/bootstrap.php(3 hunks)tests/php/src/Analytics/Reports/OrderQueryFilterTest.php(9 hunks)tests/php/src/Analytics/Reports/OrderStatsQueryFilterTest.php(2 hunks)tests/php/src/Analytics/Reports/OrderStatsScheduleListenerTest.php(2 hunks)tests/php/src/Analytics/Reports/Orders/QueryFilterTest.php(8 hunks)tests/php/src/Analytics/Reports/Orders/Stats/QueryFilterTest.php(2 hunks)tests/php/src/Analytics/Reports/Orders/Stats/ScheduleListenerTest.php(2 hunks)tests/php/src/Analytics/Reports/ProductQueryFilterTest.php(3 hunks)tests/php/src/Analytics/Reports/ProductStatsQueryFilterTest.php(4 hunks)tests/php/src/Analytics/Reports/Products/QueryFilterTest.php(2 hunks)tests/php/src/Analytics/Reports/Products/Stats/QueryFilterTest.php(3 hunks)tests/php/src/Commission/BackwordCompatibility.php(1 hunks)tests/php/src/Commission/CommissionModelTest.php(1 hunks)tests/php/src/Commission/CommissionTest.php(2 hunks)tests/php/src/Commission/OrderCommission.php(1 hunks)tests/php/src/Commission/StrategyTest.php(1 hunks)tests/php/src/CustomAssertion/DBAssertionTrait.php(0 hunks)tests/php/src/DBAssertionTrait.php(0 hunks)tests/php/src/DokanFactoryTest.php(1 hunks)tests/php/src/DokanTestCase.php(2 hunks)tests/php/src/Factories/CouponFactory.php(1 hunks)tests/php/src/Orders/OrderEventListenerTest.php(1 hunks)tests/php/src/Orders/OrderLogTest.php(1 hunks)tests/php/src/Orders/OrderRestoreFromTrashTest.php(1 hunks)tests/php/src/Orders/OrderStatusChangeTest.php(3 hunks)tests/php/src/Orders/VendorBalanceUpdateHandlerTest.php(2 hunks)tests/php/src/REST/AdminOnboardingControllerTest.php(1 hunks)tests/php/src/REST/CustomersControllerTest.php(2 hunks)tests/php/src/REST/DokanDataContinentsControllerTest.php(1 hunks)tests/php/src/REST/DokanDataCountriesControllerTest.php(1 hunks)tests/php/src/REST/VendorProductCategoriesApiTest.php(2 hunks)tests/php/src/ReactUrlGenerationTest.php(1 hunks)tests/php/src/Vendor/UserToVendorConversionTest.php(1 hunks)tests/php/src/VendorNavMenuCheckerTest.php(1 hunks)tests/php/src/Withdrawal/MakeDefaultWithdrawalMethodTest.php(6 hunks)tests/php/src/Withdrawal/WithdrawChargeApiTest.php(2 hunks)tests/php/src/Withdrawal/WithdrawChargeTest.php(1 hunks)
💤 Files with no reviewable changes (2)
- tests/php/src/DBAssertionTrait.php
- tests/php/src/CustomAssertion/DBAssertionTrait.php
🧰 Additional context used
🧬 Code graph analysis (14)
tests/php/src/Analytics/Reports/Products/Stats/QueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
run_all_pending_queue(201-203)
tests/php/src/Analytics/Reports/Orders/Stats/ScheduleListenerTest.php (1)
tests/php/src/DokanTestCase.php (1)
run_all_pending_queue(201-203)
tests/php/src/Analytics/Reports/Orders/Stats/QueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
run_all_pending_queue(201-203)
tests/php/src/Analytics/Reports/OrderStatsQueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
run_all_pending_queue(201-203)
tests/php/src/Analytics/Reports/Orders/QueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
run_all_pending_queue(201-203)
tests/php/src/Orders/VendorBalanceUpdateHandlerTest.php (2)
includes/Order/VendorBalanceUpdateHandler.php (1)
VendorBalanceUpdateHandler(17-230)tests/php/src/DokanTestCase.php (1)
DokanTestCase(23-428)
tests/php/src/Orders/OrderLogTest.php (1)
tests/php/src/DokanTestCase.php (1)
DokanTestCase(23-428)
tests/php/bootstrap.php (1)
includes/Install/Installer.php (1)
create_tables(320-330)
tests/php/src/Analytics/Reports/ProductStatsQueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
run_all_pending_queue(201-203)
tests/php/src/Analytics/Reports/Products/QueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
run_all_pending_queue(201-203)
tests/php/src/Analytics/Reports/ProductQueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
run_all_pending_queue(201-203)
tests/php/src/Orders/OrderEventListenerTest.php (1)
includes/Order/OrderEventListener.php (1)
OrderEventListener(8-144)
tests/php/src/Analytics/Reports/OrderStatsScheduleListenerTest.php (1)
tests/php/src/DokanTestCase.php (1)
run_all_pending_queue(201-203)
tests/php/src/Analytics/Reports/OrderQueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
run_all_pending_queue(201-203)
⏰ 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). (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: api tests (1, 1)
- GitHub Check: e2e tests (2, 3)
🔇 Additional comments (39)
tests/php/src/Analytics/Reports/OrderStatsScheduleListenerTest.php (1)
39-39: LGTM! Consistent queue helper usage.The refactoring to use
run_all_pending_queue()improves test code consistency across the suite while preserving the same functionality.Also applies to: 53-53
tests/php/src/Withdrawal/WithdrawChargeTest.php (1)
8-11: LGTM! Test organization improvement.The addition of PHPUnit group annotations enables selective test execution and aligns with test organization best practices across the test suite.
tests/php/src/Withdrawal/WithdrawChargeApiTest.php (2)
7-11: LGTM! Test organization improvement.The addition of PHPUnit group annotations enables selective test execution and improves test organization.
163-163: LGTM! Data consistency fix.Changing the percentage value from
0to''(empty string) correctly aligns the test expectation with the database setup defined insave_withdraw_data_to_database()(line 42), where the bank method's percentage is stored as an empty string.tests/php/src/Vendor/UserToVendorConversionTest.php (2)
7-7: LGTM! Class rename improves clarity.The rename from
UserToVendorTesttoUserToVendorConversionTestmakes the test class name more descriptive and explicit about what is being tested.
10-12: LGTM! Docblock enhancement improves test documentation.The expanded docblock now clearly describes what the test verifies: that shop data (store name, phone number, etc.) is properly persisted when a user is converted to a vendor. This makes the test's purpose immediately clear to anyone reading the code.
tests/php/src/Withdrawal/MakeDefaultWithdrawalMethodTest.php (5)
9-12: LGTM: Test grouping added.The
@groupannotations enable selective test execution and improve test organization.
77-83: LGTM: Robust JSON extraction.The improved parsing logic correctly handles AJAX responses that may contain HTML error messages before the JSON payload by extracting from the last occurrence of
{.
89-89: LGTM: Enhanced error reporting.Including the raw response in the error message significantly aids debugging when JSON decoding fails.
100-103: LGTM: Essential verification of persistent state.Directly verifying the
dokan_withdraw_default_methoduser meta confirms that the AJAX handler correctly persisted the change, not just returned a success response.
34-56: Approve test setup and formatting The code properly configures user capabilities, payment profiles, and withdrawal options, and PHPCS reports no indentation violations.tests/php/src/Orders/OrderStatusChangeTest.php (1)
8-8: LGTM! Test grouping annotation added.The
@group ordersannotation enables selective test execution and better test organization.tests/php/src/VendorNavMenuCheckerTest.php (1)
6-8: LGTM!The
@group core-featureannotation correctly categorizes this test class for better test organization and selective execution.tests/php/src/Commission/CommissionModelTest.php (1)
13-16: LGTM!The group annotations properly categorize this test class within the commission test suite.
tests/php/src/Commission/StrategyTest.php (1)
15-15: LGTM!The updated group annotation
commission-strategybetter aligns with the test class name and content (testing various commission strategies).tests/php/src/Commission/BackwordCompatibility.php (1)
13-16: LGTM!The group annotations appropriately categorize this backward compatibility test within the commission test suite.
tests/php/src/Commission/OrderCommission.php (1)
9-12: LGTM!The group annotations properly categorize this order commission test class.
tests/php/src/Orders/OrderRestoreFromTrashTest.php (1)
7-10: LGTM!The group annotations correctly categorize this test within the orders test suite.
tests/php/src/Commission/CommissionTest.php (2)
23-26: LGTM!The group annotation categorizes this test class appropriately.
1017-1017: Explain the commented-outget_sourceassertion
The// $this->assertEquals($expected['strategy_source'], $commission->get_source());line remains in the test without explanation. Either remove it if strategy-source assertions are no longer valid, add a TODO (with an issue link) if it’s deferred work, or restore/fix the test to match the intended behavior.tests/php/src/Analytics/Reports/Products/QueryFilterTest.php (2)
58-58: LGTM!The method call correctly uses
run_all_pending_queue(), which aligns with the renamed helper method inDokanTestCase(line 200-202 in tests/php/src/DokanTestCase.php).
92-92: LGTM!The method call correctly uses
run_all_pending_queue(), consistent with the DokanTestCase API changes.tests/php/src/Analytics/Reports/ProductQueryFilterTest.php (1)
58-58: Queue helper rename looks correct
run_all_pending_queue()matches the updated helper inDokanTestCase, so the tests keep their intended timing.Also applies to: 91-91, 137-137
tests/php/src/REST/AdminOnboardingControllerTest.php (1)
11-14: Grouping metadata looks goodThe added
@grouptags align this suite with the new REST grouping convention.tests/php/src/Analytics/Reports/Orders/Stats/QueryFilterTest.php (1)
63-63: Queue helper rename confirmed
run_all_pending_queue()aligns with the renamed helper in the base test case, so queue draining still happens as intended.Also applies to: 100-100
tests/php/src/REST/DokanDataContinentsControllerTest.php (1)
7-10: REST grouping annotation LGTMAdding the
@grouptags keeps this test aligned with the rest of the REST suite categorization.tests/php/src/REST/DokanDataCountriesControllerTest.php (1)
7-10: LGTM!The group annotations improve test categorization and enable selective test execution via
phpunit --group rest-apiorphpunit --group rest-api-data-countries.tests/php/src/ReactUrlGenerationTest.php (1)
8-9: LGTM!The group annotations improve test organization and enable filtering by feature category.
tests/php/src/Analytics/Reports/OrderStatsQueryFilterTest.php (2)
66-66: LGTM!The method rename to
run_all_pending_queue()aligns with the test suite's standardization of queue-based pending task processing.
104-104: LGTM!Consistent with the queue helper API update across the test suite.
tests/php/src/Orders/OrderEventListenerTest.php (2)
3-3: LGTM!The namespace update to
WeDevs\Dokan\Test\Ordersaligns with the directory structure and standardizes test organization.
9-9: LGTM!The group annotation enables selective test execution for order-related tests.
tests/php/src/Analytics/Reports/Orders/Stats/ScheduleListenerTest.php (2)
42-42: LGTM!Consistent with the queue helper API standardization across the test suite.
56-56: LGTM!Aligns with the test suite's queue-based pending task processing.
tests/php/src/Analytics/Reports/OrderQueryFilterTest.php (2)
11-11: LGTM!The additional group annotation improves test categorization and filtering capabilities.
61-61: LGTM!All method renames to
run_all_pending_queue()are consistent with the test suite's queue-based pending task processing standardization.Also applies to: 99-99, 156-156, 194-194, 225-225, 257-257, 285-285, 307-307
tests/php/src/Analytics/Reports/Products/Stats/QueryFilterTest.php (1)
59-59: LGTM!The method renames align with the test suite's standardized queue-based pending task processing.
Also applies to: 93-93, 136-136
tests/php/src/Analytics/Reports/Orders/QueryFilterTest.php (1)
60-60: LGTM!All method renames to
run_all_pending_queue()are consistent with the test suite's queue-based pending task processing standardization. The changes maintain the same behavior while improving API consistency.Also applies to: 100-100, 152-152, 190-190, 221-221, 253-253, 281-281, 303-303
tests/php/src/DokanTestCase.php (1)
201-212: All old queue method calls removedNo instances of
run_all_pending()orcancel_all_pending()remain; tests referencerun_all_pending_queue()(and none callcancel_all_pending_queue(), matching prior usage).
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Title
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit