Fix loading incorrect values from config/app.php

[alexey-shapilov]

Bootstrapping Laravel Kernel (to load environment variables from file) before reading the application configuration from config/app.php

[Naktibalda]

What issue does this change fix?

[alexey-shapilov]

I have config/app.php with code

return [
...
    'url' => env('APP_URL', 'https://example.com'),
....
];

and
.env.testing with

APP_URL=http://localhost

and code

if (is_null($request)) {
    $appConfig = require $this->module->config['project_dir'] . 'config/app.php';
    $request = SymfonyRequest::create($appConfig['url']);
}

return wrong $appConfig because function env can't load correct value from .env.testing file. It is because Laravel Application hasn't loaded .env.testing yet via the vlucas/phpdotenv package and $appConfig['url'] and server property of $request are wrong

[davidstoker]

Is there anything that can be done to get this incorporated? I ran into this same problem in an application that was upgraded from Laravel 6.x and Codeception 3.1.2 to Laravel 7.x and Codeception 4.1.13.

Tests executed normally in Laravel 6 but then the SymfonyRequest::create call started failing with a type error since $appConfig['url'] began coming through as null without proper loading of the environment variables.

I had to pull in these changes locally to get the tests working again in the project.

[TavoNiievez]

Hey @alexey-shapilov and @davidstoker we are talking about unit tests, right?

I have not been able to reproduce this error in Laravel 8.19 over PHP 8.0.0, Codeception 4.1.13 and Laravel-module 1.1.1:

        # config/app.php
        'url' => env('APP_URL', 'https://example.com'),

        # env.testing
        APP_URL=http://localhost

        /** @var Laravel5 $module */
        $module = $this->getModule('Laravel5');
        $appConfig = require $module->config['project_dir'] . 'config/app.php';
        // $appConfig['url'] is 'http://localhost' as expected
        $request = SymfonyRequest::create($appConfig['url']);

[davidstoker]

@TavoNiievez Yes, it occurs on my unit suite though I have multiple suites and see this occurring on other suite types too.

The ordering of the $this->app->make('Illuminate\Contracts\Http\Kernel')->bootstrap(); statement looks to be key here since that actually causes environment variables to be loaded. When it is called after the config values are referenced it throws Argument 1 passed to Symfony\Component\HttpFoundation\Request::create() must be of the type string, null given. Moving that statement before if (is_null($request)) causes them to load and everything proceeds correctly.

The $this->app = $this->kernel = $this->loadApplication(); statement earlier in the initialize function will call loadEnvironmentFrom in loadApplication but that just sets the environment file name and doesn't handle loading.

[TavoNiievez]

@alexey-shapilov and @davidstoker

Ok, a few things have happened in the last few days:

First, this module has a new home in https://github.com/Codeception/module-laravel, and this repository will be deprecated shortly.

If it turns out that it is a problem that is still present in version 2.0, please forward this PR to the new repository.

I can merge it as long as it doesn't affect the current tests.

The version 2.0 release description may be helpful too.

[davidstoker]

Hi @TavoNiievez, thanks for the follow-up here. I tried the 2.0 release and observed the same behavior straight away.

In looking further and comparing your attempt to reproduce, I had this defined in config/app.php:

'url' => env('APP_URL'),

Since .env values haven't loaded by the time the Request is created, this would default to null causing the failure. Laravel's default in 5.2+ has this:

'url' => env('APP_URL', 'http://localhost'),

If I change my config to have a default value, this works with the 2.0 release and should work with this older version too. This does result in the Symfony Request being instantiated with http://localhost in $request = SymfonyRequest::create($appConfig['url']); though.

I observed my tests still working correctly and using the .env value for the URL I had defined since the this->app->make('Illuminate\Contracts\Http\Kernel')->bootstrap(); call executes after the request is created and loads the values.

The same change suggested here works in 2.0 to call bootstrap() earlier but not a clear bug since it can be caused by the config. @TavoNiievez Any additional thoughts? In my case, I'm going to adjust my config and move forward using the 2.0 release.

[TavoNiievez]

@davidstoker thanks for your research, your time is greatly appreciated.

What catches my attention is that according to @alexey-shapilov, he also had the config with a default value...

I have config/app.php with code

return [
...
    'url' => env('APP_URL', 'https://example.com'),
....
];

maybe he should change 'https://example.com' to 'http://localhost'?

So there are two possible ways here:

1. We could turn this into documentation, like adding a try catch in that code block and adding a message indicating to check if you have defined default values, or...
2. We could try the change that is proposed in this PR and if nothing strange happens in the CI jobs merge it in the same way.