Require bootstrap.php if exists, to load all necessary .env files

[TavoNiievez]

I'm replacing #4 with this.

In my last comment I expressed that I had problems with compatibility logic in lower versions of Symfony, but since the minimum compatible version is now 5.4 we can move on.
I require the tests/bootstrap.php file if it exists, otherwise I try to use dotEnv manually.
If all that fails I throw an exception telling the user what to do.

In the tests I performed this works regardless of whether or not you have .env and .env.test set in codeception.yml.
This will also allow to remove these values from the recipe since they are now loaded automatically.

Closes #99.

[c33s]

i am a big fan of BC
the check for the file don't really hurt, i would keep it in.

i just had a look at your code and thought about it. i am not sure if it is optimal. maybe it tries to think too much for "me". you are loading the tests/bootstrap.php file. isn't the file already loaded by codeception by default? what happens if i define bootstrap: _bootstrap.php in codeception.yml won't i get a strange behavior if the symfony module tries to be smart and loads a hardcoded test/bootstrap.php file which may still exist but the developer switched to tests/_bootstrap.php via config and wonders which magic is happening.

if i want to load the config/bootstrap.php (for dotenv handling) but also want to use test/bootstrap.php (for loading BypassFinals::enable();) this would not be possible with this code.

i would want codeception to load the tests/bootstrap.php file from the codeception.yml (or whatever name was defined there) and the symfony module should only load the dotenv stuff via the old (config/bootstrap.php) or the newer method.

also i am not sure if overriding the environment is the perfect solution but for that i am not sure.

just some thoughts. what do you think? do i missunderstand something or are my thoughts valid?

[TavoNiievez]

Got it.
Your message suggests that it would be great if the bootstrap file matched the one in the codeception.yml file. I agree. However, that's not a use case that this PR is solving.

Many comments in the issue thread prefer the Symfony module to handle environments like Symfony does, via tests/bootstrap.php or DotEnv.

If Codeception already loaded it, the require_once won't do anything. Enhancements to customize via configuration would be nice, but it's not essential to solving the issue.

btw, I wasn't aware that Codeception generates a tests/bootstrap.php, which seems to mirror Symfony's:
https://github.com/symfony/recipes/blob/main/symfony/phpunit-bridge/6.3/tests/bootstrap.php

[c33s]

i think the symfony module should only

• load the dotenv system
• config/bootstrap.php if it exists
• don't interact with the tests/bootstrap.php (shouldn't that be automatically loaded before through codeception? currently i load dotenv there and the symfony module has then access to all vars from the .env files) as far as i know this file is specific for codeception, there i can bootstrap my personal test environment (like loading bypass finals, set my environment, ...)
• should not hardcode the environment, sometimes i maybe have two test environments like test and test-quick

[c33s]

i think you mix something up, we have config/bootstrap which is from legacy symfony, then we have the symfony php unit specific tests/bootstrap.php which is php-unit-symfony-bridge specific (has nothing to do with codeception, it only helps to integrate php unit with symfony, exactly what the bridge is made for). and then there is the "official" codeception tests/bootstrap.php this file is defined in codeception.yml and it used for booting up the codeception environment
see https://codeception.com/docs/Customization > BOOTSTRAP section. it is just a naming/file collision between codeception and the phpunit symfony bridge.

[TavoNiievez]

Coincidentally, they share name and code:
https://github.com/symfony/recipes/blob/main/symfony/phpunit-bridge/5.1/tests/bootstrap.php
Codeception should use this version when Symfony 4.4 is not supported:
https://github.com/symfony/recipes/blob/main/symfony/phpunit-bridge/5.3/tests/bootstrap.php

The module must interact with tests/bootstrap.php for automatic loading. The Codeception recipe lacks a bootstrap key and it is not generated on new installs.

As for avoiding 'hardcoding' the environment, it is not hardcoded. You must specify the environment in the module configuration:

    - Symfony:
        app_path: 'src'
        environment: 'test'

This value affects the tests. Update it if necessary; code added in this PR will work correctly.

[c33s]

in gerneral i mean we have to look at codeceptions bootstrap.php in a more global perspective. we also have to assume the file exists without symfony. and even with symfony the files maybe completly different if i want to boot my test env differently. so i would disagree that the two files share the same code (even if they may).

with hardcoded i mean, that you will always overwrite a already set environment. for example if a set test in the config as my environment and then use codeceptions bootstrap.php to change this to test-fast with some custom code your code will always reset the environment to test. that was my definition of hardcoded. i can't switch the symfony environment dynamically any more.

The module must interact with tests/bootstrap.php for automatic loading.

can you explain that please? why must the module interact with it? isn't the file loaded by codeception automatically if i defined bootstrap: bootstrap.php. i think we are fixing the wrong problem. if the recipes tests/boostrap.php files is not loaded we must change the default config generated by codeception to include bootstrap: bootstrap.php

another problem would be that your code will prevent to load codeceptions bootstap.php at all if config/bootstrap.php exists. why would you want that exactly? if i load bypass finals in codeceptions bootstrap.php and then want to boostrap symfony with config/bootstrap.php this would no longer work with your code. your code would require me to delete config/bootstrap.php and then manually add its code to codeceptions bootstrap.php so that it would work again.

but i don't know codceptions internals enough, maybe we even don't need to load tests/bootstrap.php in the module at all (would make the most sense to me) as it was already loaded by codeception before.
in this case symfony module should only look for config/bootstrap.php and load it if its there (then your code would work but we could remove the tests/bootstrap.php handling completly)

so my suggestion would be:

• verify how codeception interacts with its bootstrap.php (loaded before symfony module?) is the bootstrap: bootstrap.php the default config?
• handle the case that tests/bootstrap.php exists but only contains non-symfony boostraping which must be loaded.
• maybe even check if dotenv is already loaded
• only change the environment to the config value if it is not already set / or allow to disable the environment setting via module config.

does this make sense for you?

[TavoNiievez]

verify how codeception interacts with its bootstrap.php (loaded before symfony module?)
is the bootstrap: bootstrap.php the default config?

I've mentioned before: no, it's not in new installs. Not in the recipe either. If it's not loaded, Symfony module won't load; tests won't run. Tested it before the PR. Try it yourself. Changing the recipe won't help everyone who lacks the key.

handle the case that tests/bootstrap.php exists but only contains non-symfony boostraping which must be loaded.

"The user can modify the bootstrap.php file generated by Codeception/Symfony to remove the part where it loads environments and add their own custom code" is not something I'm going to fix in this PR.

maybe even check if dotenv is already loaded

Is not loaded.

only change the environment to the config value if it is not already set / or allow to disable the environment setting via module config.

You cannot disable the environment setting, it's required.
if you are expecting tests/bootstrap.php to do something custom you should have defined it in your codeception.yml, and if so your code will run after this code and will still work.

Please test it yourself before further discussion; speculating won't help.

[c33s]

we talk past one another and maybe you missunderstood me. i invite you to do a voice chat with me to clearify things.

i just argue for being able to have both: a custom bootstrap.php file without all symfony dotenv handling and the magic of the symfony module which would also load the dotenv code. this is not possible with your code.

[TavoNiievez]

Done in the last commit. Anything else?

[c33s]

@TavoNiievez
no slack here. i switched to matrix.
i currently opended a jitsi meet here if you like: https://fairmeeting.net/MaximumTransportsInheritSlightly (will be online there the next 2 hours)

the config parameter is perfect. it would be enough for me but it is not what i meant.

i meant to also be able to load tests/boostrap.php (custom without dotenv) and let the symfony module do its magic to do all the rest (so the default config settings should simply "make it work" tm.

to be completly flexible and allow all developers to customize their needs i would go more in a config-for-everyting direction.

    private function bootstrapEnvironment(): void
    {
        // with that i can manage if i want to override an existing env
        if (empty($_ENV['APP_ENV']) || $this->config['set_app_env']) {
            $_ENV['APP_ENV'] = $this->config['environment'];
        }

        // so i can load config/bootstrap.php or tests/bootstrap.php or nothing or for example tests/symfonyExtraBootstrap.php
        if (!empty($this->config['bootstrap_file'])) {
            $bootstrapFile = $this->kernel->getProjectDir() . $this->config['bootstrap_file'];
            if (file_exists($bootstrapFile)) {
                require_once $bootstrapFile;
            }
        }

        if ($this->config['load_dotenv'] && !method_exists(Dotenv::class, 'bootEnv')) {
            throw new LogicException(
                "Symfony DotEnv is missing. Try running 'composer require symfony/dotenv'\n" .
                "If you can't install DotEnv add your env files to the 'params' key in codeception.yml\n" .
                "or update your symfony/framework-bundle recipe by running:\n" .
                'composer recipes:install symfony/framework-bundle --force'
            );
            (new Dotenv())->bootEnv('.env');
        }
    }

[TavoNiievez]

Ok, I understand what you want to do, but again I reiterate, that would be an (very custom) enhancement.
And it's not a use case that this PR is solving.

This PR is dedicated to those who are having problems loading their env files just as they would expect them to be loaded in Symfony, nothing more, nothing less.

The $_ENV['app_env'] assignment is only going to happen if you manually enable it in the bootstrap module setting in the first place and decide to let Symfony DotEnv do the work instead of a bootstrap.php file.

If that's not what you need just don't enable this feature.

[TavoNiievez]

@c33s thank you for your help in this matter.