[BUG] Empty dbPassword in WPLoader results in undefined DB_PASSWORD constant in test environment

[jerclarke]

Environment
OS: MacOS
PHP version: 8.3.22
Installed Codeception version: 5.3.2
Installed wp-browser version: 4.5.5
WordPress version: 6.8.2
Local development environment: Valet
WordPress structure and management: Default

Can you perform the test manually?
Yes. In normal usage DB_PASSWORD is loaded from wp-config.php and of course works. The issue is related to the elaborate dance WP-Browser does to set the constants based on .env values in Integration tests.

Codeception configuration file

I managed to reproduce this on an almost totally-standard install of the modern default setup and Integration config. The only difference is that it uses MySQL rather than the SQLite plugin:

namespace: Tests
support_namespace: Support
paths:
    tests: tests
    output: tests/_output
    data: tests/Support/Data
    support: tests/Support
    envs: tests/_envs
actor_suffix: Tester
params:
    - tests/.env
extensions:
    enabled:
        - Codeception\Extension\RunFailed
        - lucatume\WPBrowser\Extension\ChromeDriverController
        - lucatume\WPBrowser\Extension\BuiltInServerController
        - lucatume\WPBrowser\Extension\Symlinker
    config:
        lucatume\WPBrowser\Extension\ChromeDriverController:
            port: '%CHROMEDRIVER_PORT%'
        lucatume\WPBrowser\Extension\BuiltInServerController:
            workers: 5
            port: '%BUILTIN_SERVER_PORT%'
            docroot: '%WORDPRESS_ROOT_DIR%'
            # env:
            #     DATABASE_TYPE: sqlite
            #     DB_ENGINE: sqlite
            #     DB_DIR: '%codecept_root_dir%/tests/Support/Data'
            #     DB_FILE: db.sqlite
        lucatume\WPBrowser\Extension\Symlinker:
            wpRootFolder: "tests/_wordpress" 
            plugins:
                - .
    commands:
        - lucatume\WPBrowser\Command\RunOriginal
        - lucatume\WPBrowser\Command\RunAll
        - lucatume\WPBrowser\Command\GenerateWPUnit
        - lucatume\WPBrowser\Command\DbExport
        - lucatume\WPBrowser\Command\DbImport
        - lucatume\WPBrowser\Command\MonkeyCachePath
        - lucatume\WPBrowser\Command\MonkeyCacheClear
        - lucatume\WPBrowser\Command\DevStart
        - lucatume\WPBrowser\Command\DevStop
        - lucatume\WPBrowser\Command\DevInfo
        - lucatume\WPBrowser\Command\DevRestart
        - lucatume\WPBrowser\Command\ChromedriverUpdate

Suite configuration file
Paste, in a fenced YAML block, the content of the suite configuration file; remove any sensitive data!

actor: IntegrationTester
bootstrap: _bootstrap.php
modules:
    enabled:
        - lucatume\WPBrowser\Module\WPLoader
    config:
        lucatume\WPBrowser\Module\WPLoader:
           wpRootFolder: "tests/_wordpress" 
           dbName: "wp_gv_dev_wpunit"
           dbHost: "127.0.0.1"
          # ! Empty password triggers the problem, non-empty password works fine!
          #  dbUser: "testmysqluser"
          #  dbPassword: "testmysqlpassword"
           dbUser: "root"
           dbPassword: ""
           wpDebug: true
           tablePrefix: '%TEST_TABLE_PREFIX%'
           domain: '%WORDPRESS_DOMAIN%'
           adminEmail: 'admin@%WORDPRESS_DOMAIN%'
           title: 'Integration Tests'
           plugins: ['./newwpbrowsertestplugin.php']
           theme: ''

Describe the bug

The ultimate essence of the bug, as I understand it, is that if the dbPassword is empty in the WPLoader config, as it is when using the default Valet setup, the DB_PASSWORD constant never gets set up for use in the tested plugin. IMO, it would be better if WP-Browser ensured that DB_PASSWORD gets set up as expected even if it's empty.

The thing about it is you would never notice the problem UNLESS you have a plugin that itself uses DB_PASSWORD directly. The magic deep inside WP-Browser correctly ensures that as far as core WP is concerned, the db password is never missing, and the db loads correctly even with dbPassword: "". But because our plugin loads a second database using DB_PASSWORD, I finally figured out that it wasn't being defined.

For me this was a multi-day journey trying to solve it after moving to Valet and having everything explode in various ways, including this empty password thing. The error messages are basically non-existent with this problem, requiring a lot of --debug and figuring out error messages that are confusing. It's also really hard to find with XDebug because there are so many layers of loading wp-config.php during the setup phase of Integration tests.

Anyway, I finally isolated it down and found what I think is the source of the problem in wp-tests-config.php, where it does the following:

foreach ([
             'ABSPATH' => $abspath,
            ...
             'DB_PASSWORD' => $wpLoaderConfig['dbPassword'],
             ...
         ] as $const => $value) {
    if ($value && !defined($const)) {
        define($const, $value);
    }
}

At this point in the code, $wpLoaderConfig has the value correctly set as empty, but because the if statement is just checking $value, it returns empty and fails to define the DB_PASSWORD constant.

So like I said, I can totally see how this never came up, because empty password is weird and calling DB_PASSWORD directly in a plugin is even stranger. At the same time, doesn't it make sense to fix it? The developer experience of bumping up against this is truly horrible.

To Reproduce

The following test, added to TestCustomTestCaseTest.php with the default configuration+empty mysql password, seems to reproduce the problem for me:

public function test_db_password_constant(): void
    {
        $this->assertNotEmpty( DB_PASSWORD );
    }

In the terminal running that test gives a big red error:

[Error] Undefined constant "Tests\DB_PASSWORD"

This isn't how it looked in my real life tests though, I wish it had been this clear!

Expected behavior

The test above should just pass, because all these constants are intended to be defined at this point.

Additional context

Of course I finally solved this by just not using an empty password, but I wasted a lot of hours getting to that point because the debugging steps were so surreal.

If nothing else, it might be good to add a warning about empty db passwords to the documentation (which I read very carefully during this process), but really they do work, so it would be better to just fix this support for empty passwords instead.