Closed andronocean closed 2 months ago
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 4.3.0
the WPLoader::_beforeSuite
method was added to fix #744.
The issue was the module would not load WordPress when the EventDispatcherBridge
was not part of the configuration, in the main codeception.yml
file, usually.
Your configuration does not use the lucatume\WPBrowser\Extension\EventDispatcherBridge
extension: this means that, in versions before 4.3.0
, WordPress would not be loaded.
Are you using WordPress functions in your EndToEnd
tests? If you do, they should crash.
If you downgrade wp-browser
to version 4.2.5
and add the lucatume\WPBrowser\Extension\EventDispatcherBridge
extension to your configuration, then you should have the same issue.
The issue appeared, for your configuration, in version 4.3.0
because WPLoader
is actually loading WordPress without relying on the EventDispatcherBridge
extension.
The issue does not come up when using a SQLite database in the WPLoader
module configuration since the DB_HOST
, DB_USER
, DB_PASSWORD
and DB_NAME
constants will not be relevant to the loading of the database file.
The fix for #744 is correct: WPLoader
should load WordPress whether the EventDispatcherBridge
extension is active or not.
The fix for your issue is making sure the loaded WordPress installation will use the database credentials specified in the WPLoader
module configuration, and not the ones provided by the wp-config.php
(or relative files in the case of Bedrock) file, and this is what I'm looking into.
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 loadOnly: false
mode, WPLoader
supports a configFile
parameter. Maybe that could be used here, too?
That obviously wouldn't work for vanilla WordPress sites using hardcoded wp-config constants.
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.
To add to that thought... in
loadOnly: false
mode,WPLoader
supports aconfigFile
parameter. Maybe that could be used here, too?
Another possible solution to keep in mind.
@andronocean I've pushed a fix in #755; I'm defining an env var, WPBROWSER_LOAD_ONLY=1
when loadOnly: true
that should provide a good hook for configurations to change depending on the env.
If you could test it out and let me know, that would be great.
composer require --dev lucatume/wp-browser:dev-v4-issue-753
To add to that thought... in
loadOnly: false
mode,WPLoader
supports aconfigFile
parameter. Maybe that could be used here, too?Another possible solution to keep in mind.
I will reply to this myself with one admission and one information:
configFile
configuration parameter already works and has for some time nowconfigFile
in loadOnly: true
mode. I've updated the documentation and am merging that PR.@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 loadOnly:true
tests, and thanks to the env var I could resolve it immediately. I'd seen it once before a while ago, but had no way to handle it then. Again, it was related to integration with the Roots stack (Acorn this time), trying to take over Codeception's run
command with its own console handler and leading to an InstallationException
in wp-browser.
Happily, WPBROWSER_LOAD_ONLY
let me detect WPLoader and do putenv( 'APP_RUNNING_IN_CONSOLE=false' );
, which makes Acorn behave itself.
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?
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 😄)