-
Notifications
You must be signed in to change notification settings - Fork 70
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
Prevent DB warnings in unit tests #339
Conversation
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.
Thanks @mikkamp I don't see anymore the errors in the console and the tests pass. Also now the file seems consistent with the AW one.
❓ In woocommerce/google-listings-and-ads#2183 you mentioned that we should mark extensions as compatible with PHP 8.3, Should we mark this one as Composer as well as the tests run smoothly? Or it's any other step to take?
I've been marking them as compatible in the project thread: peeuvX-19l-p2 |
What I meant about composer is that Composer install runs successfully. As far as I read that's a condition to make it compatible. |
Changes proposed in this Pull Request:
This PR reverts the change in #335 since it still produces warnings when starting tests with an empty database. So here we revert to an earlier hook and add a filter to prevent the notice from showing. We also use
WC_REMOVE_ALL_DATA
so all database tables can be removed and recreated whenWC_Install
is called.The PR also adds PHP version 8.3 to the workflow, so it now runs the default tests on PHP 8.2 and includes the oldest supported version (7.4) and the latest supported version (8.3).
Closes #338
Detailed test instructions:
bin/install-unit-tests.sh <db_name> <db_user> <db_pass>
(selectY
to clear database during install)vendor/bin/phpunit
Changelog entry