-
-
Notifications
You must be signed in to change notification settings - Fork 86
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
[BUG] fatal error with WPLoader on 4.3.0, LoadSandbox tries to use DB from wp-config, not test config #753
Comments
Thank you @andronocean for the detailed report and research: this seriously helps me. This looks like a regression, since it was working before. I will look into this in the next days and post updates in this issue. |
While I have not found a fix yet, I've found the origin of the issue: in version Your configuration does not use the Are you using WordPress functions in your If you downgrade The issue appeared, for your configuration, in version The issue does not come up when using a SQLite database in the The fix for #744 is correct: |
Aha! That makes perfect sense. My EndToEnd tests on this project treat the site like a black box and do everything via the browser/HTTP interface, and database setup is either in the dump or done via the WPDb module, so I never ran into missing WP. I'd actually been wondering about what WP functions would do. And sure enough, they fail on 4.2.5. I've never clarified a use-case for the EventDispatcherBridge, so yes, I left it out. Seems like I've just been lucky. As for a fix: from my side, the simplest and best option would be to dynamically set which .env file Bedrock is loading. If WPLoader could define a constant (or set an environment variable) that's visible to wp-config (and descendants) during the sandbox load, I can use that to make sure the right environment data gets loaded. As a rough example: // in some sort of WPLoader bootstrap file
define('WP_ENV_FILE', dirname(__FILE__) . '/tests/.env'); // in wp-config.php or Bedrock application.php
if ( ! defined( 'WP_ENV_FILE' ) ) {
define( 'WP_ENV_FILE', realpath( dirname( __FILE__ ) . '/../.env' ) );
} Then Bedrock could load that file with Dotenv as usual, without ever needing to detect if tests are running. (That obviously wouldn't work for vanilla WordPress sites using hardcoded wp-config constants.) |
To add to that thought... in |
This I've already dealt with: redefinition of constants will trigger a warning that can be handled and suppressed. Your idea about the definition of an env var or constant to indicate WordPress is being loaded by WPLoader is a good one. I've been going back and forth about other possible solutions, but they all turn out pretty ugly or complicated; your approach is simple and robust. I will update this issue with further changes. |
Another possible solution to keep in mind. |
@andronocean I've pushed a fix in #755; I'm defining an env var, If you could test it out and let me know, that would be great.
|
I will reply to this myself with one admission and one information:
#755 adds both options: an env var to detect and support for |
@lucatume Thank you for this — I updated to 4.3.3 and modified my configuration accordingly, and it's all working perfectly. The new env var is extremely useful! Another error appeared on the Happily, When I have time I can write up a proper separate issue or make a pull request. I think the exception message thrown could be clarified as it is a very specific failure mode. Would you prefer an issue first or just a PR? |
If you can provide a PR, that would be much appreciated. You explore issues carefully, and a PR from someone that is taking the time to think it out is an excellent contribution. Edit: will I ever learn to use quote? |
Version 3.5
No, new bug in 4.3.0
Environment
OS: macOS 14.6.1
PHP version: 8.2.22
Installed Codeception version: 5.1.2
Installed wp-browser version: 4.3.0
WordPress version: 6.6.1
Local development environment: Roots' Trellis VM environment using Lima as VM manager
WordPress structure and management: Bedrock
Can you perform the test manually?
Yes.
Codeception configuration file
Paste, in a fenced YAML block, the content of your Codeception configuration file; remove any sensitive data!
Suite configuration file
Paste, in a fenced YAML block, the content of the suite configuration file; remove any sensitive data!
For completeness, here’s my tests/.env file too:
Describe the bug
I updated to 4.3.0 and my End-to-End and Functional test suites started crashing immediately with a fatal error (see output below). Both of these suites use WPLoader in loadOnly: true mode (difference between them is one uses WPWebDriver and other WPBrowser module)
It appears that the changes to the
LoadSandbox
class are causing it to load my Bedrock configuration. The sandboxed WP then tries to create a database connection using the credentials in the Bedrock config, instead of connecting to the database I’ve specified in suite config and my tests/.env file. This fails because my Bedrock config is meant to run inside of my local development VM, and therefore it’s set forlocalhost
access to my development database.As shown in the tests/.env file above, I’ve given WPLoader credentials to access a database running in the VM remotely (
WORDPRESS_E2E_DB_HOST=192.168.64.10
). (I have it using its own database and a different MySQL user.) The output below shows that instead the sandboxed WordPress is trying to use the'example_com'@'localhost'
user while running on my host Mac.Output
Here’s the output of
vendor/bin/codecept run EndToEnd --debug
The smoking gun is in the exception trace, where it says
require_once() at /path/to/example/site/web/wp-config.php:9
. In Bedrock, that wp-config file loads application config prior to requiring wp-settings.php:To Reproduce
(This might be onerous to set up; if necessary I could give you a bare-bones repo with a configured Trellis & Bedrock project)
Expected behavior
What has worked perfectly until 4.3.0 is to have the WebDriver/WPBrowser tests access the same VM environment that runs my development instance of a site. This helps me maintain parity. I run the test suites directly on my host for convenience (wonderful for IDE integration).
Setting DB credentials for WPLoader should ensure that those are always used by the tests.
Additional context (and thoughts!)
I’m using the local SQLite db option for Integration tests, so I haven’t experienced any issues there with a loadOnly: false setup. Everything there is confined to the host machine, no VM involvement.
I’ve tested this with both PHPUnit 9.6 and 10.5, so that doesn’t appear to be a factor. I don’t think my setup is too bizarre...
Thoughts: I’m not sure how WPLoader can avoid applying the Bedrock config if it has to include wp-load.php and all that that reaches out to. Whatever it did before worked, however.
I could modify the Bedrock bootstrap config to, for example, check an environment variable to determine which .env file(s) to load (I’m already doing something similar inside the VM to check request headers for Chromedriver requests and load production config instead of development.) But I also don’t like making my code too test-aware.
(Or maybe I should just create a docker container for my tests already 😄)
The text was updated successfully, but these errors were encountered: