Codeception: "#" comments in .env params file not parsed correctly

[ivokund]

\Codeception\Configuration::prepareParams uses parse_ini_file() to parse both .ini and .env files.
It makes sense to parse .ini files with parse_ini_file, but should it not use something like https://github.com/vlucas/phpdotenv to parse .env files?

My .env contains comments starting with # that are not parsed correctly by parse_ini_file.

To reproduce,

1. Add this to codeception.yml

params:
    - ../.env

1. Add a comment containing " or ( to your .env file, for example:

# MySQL access configuration (required)

1. Run codecept run and get

PHP Warning:  syntax error, unexpected '(' in <dir>/.env on line 1
 in <dir>/vendor/codeception/codeception/src/Codeception/Configuration.php on line 689

Codeception 2.2.5

[DavertMik]

What can I say about it.... Oooops! Yes, the formats are different but parse_ini worked fine for .env too.
Is there a specification to .env format? Is there a way to parse it better without installing additional packages?

[sergeyklay]

• https://github.com/vlucas/phpdotenv
• https://github.com/josegonzalez/php-dotenv

[DavertMik]

@sergeyklay

Is there a way to parse it better without installing additional packages

[DavertMik]

I think it is actually a bug/feature/change in PHP itself. See the changelog for PHP 7.0

[DavertMik]

For our puposes we can use just http://php.net/manual/en/function.parse-ini-string.php and strip comments.
@ivokund do you wish to send PR with such fix?

[ivokund]

I guess it would be a bit like pretending to support .env files, because some features (like nested variables) would still not be supported. I see three other alternatives here (besides using an external library or incorporating code from that library into Codeception).

First, perhaps if would make more sense to allow a PHP callback as a value for params, in which custom logic (such as loading env variables using vlucas/phpdotenv) could be added.

Something like

params:
    - \app\tests\EnvLoader::loadEnvVariables

In my case, loadEnvVariables would look something like.

$dotEnv = new Dotenv\Dotenv(APP_ROOT);
$dotEnv->load();
return $_ENV;

Another way would be to allow custom shell commands to run before starting tests. For example I could write this to use the source (or .) command to load .env variables from the project root.

source ../.env

Theoretically it would be something like adding a line to _bootstrap.php only executed earlier (I tried adding the dotenv logic to bootstrap, but it was too late, config was already put together before that).

And finally, there's always the choice to require vlucas/phpdotenv only when the user tries to load params from the environment. This way it would be a "suggested" package and not requried by 99% of users that don't need it.

That's all I've got at the moment :) Ideas?

[DavertMik]

First, perhaps if would make more sense to allow a PHP callback as a value for params, in which custom logic (such as loading env variables using vlucas/phpdotenv) could be added

:+1: I really like it. That makes much more sense than my dirty implementation :upside_down_face:

And finally, there's always the choice to require vlucas/phpdotenv only when the user tries to load params from the environment. This way it would be a "suggested" package and not requried by 99% of users that don't need it.

Unfortnately we don't have the way of using _optional_ packages for core functionality. Modules can suggest packages but core not. However, we can start by introducing it there and throw an exception If package is not loaded... But to be honest, I didn't see that complex .env files. Yes. .env is much different than .ini but for simple cases it is just that. So probably by default we can still try to parse it as .ini and if we fail we can throw an exception like: implement your own parser (like \app\tests\EnvLoader::loadEnvVariables)

Does this make sense?

[ivokund]

If it was my call, then I personally would not include faulty functionality (that may or may not work due to comment style) and would leave just the .ini file parsing and implement the custom loader for people who need more customization.

Trying to load .env with PHP's parse_ini stuff is a bit like trying to run PHP7 code on a PHP5 machine - it might work if you're lucky, but can cause headaches when you're trying to use the library.

But of course that's just my opinion and that's your project and decision :)

[omnilight]

Maybe we will allow for inclusion of arbitrary php file before other codeception code will run, but after composer autoloader inclusion? This will give ability execute any helpers, including phpdotenv loader and so on and we will be able to use already existed env variables reading?

[DavertMik]

Maybe we will allow for inclusion of arbitrary php file before other codeception code will run, but after composer autoloader inclusion?

I think it is a good idea to make it happen in _bootstrap.php. Unfortunately it is a breaking change and won't be made until 3.0.