[5.1] All commands in app\Console\Commands\Kernel are instantiated when running unit tests

[markdwhite]

The particular problem is on a project with about 25 commands with a fair bit of DI in constructors, but I guess it applies to all projects to a degree.

With code-coverage turned on, there seems to be excessive amounts of 'what other classes are being loaded' being done by code-coverage which is killing the performance of phpunit. I usually turn code-coverage off because of the overhead, but when I need it, it's painful to see time taken going up by about 50x.

Is there a need for all commands to be instantiated when $app->environment('testing') as if this didn't happen it would alleviate the problem? Or is it just something to live with? Or am I missing an simple way to mitigate this?

[GrahamCampbell]

Feel free to discuss on the forums.

[markdwhite]

@GrahamCampbell

Well, I was hoping for a little more than that...

Could you just confirm that all commands are instantiated each time a unit test is run. At least then I know the issue is correct as I've perceived it.

Thanks.

[GrahamCampbell]

Could you just confirm that all commands are instantiated each time a unit test is run.

Yes, that is correct. That will happen.

[markdwhite]

@GrahamCampbell

Much appreciated. I'll take this to the forums.

[markdwhite]

As a follow up to anyone else wondering about this:

Thanks to sisve and jemaclus on #laravel (freenode) for helping me find the answer.

I'm using TestCase::seed() to seed individual tables as part of the tests, which runs artisan, which loads all the commands, etc...

Artisan is also used by the DatabaseMigrations trait. And likely elsewhere. So it seems that wherever functionality is needed that is provided by artisan, $app->artisan is called up, which depends on Illuminate\Contracts\Console\Kernel being instantiated, which instantiates all registered Commands, etc...

And this involves a lot of classes and this chokes Xdebug.

So the choices now seem to be to mitigate the problem as much as possible (avoid DI in constructors for one), or write replacement traits and override functions that decouple artisan (and it's loading process that is inefficient for Xdebug) from the actual functionality that's required.

[markdwhite]

Or a simpler solution, when running unit tests, remove all commands. Maybe a bit blunt, but adding this to app/Console/Kernel.php does the job:

/**
 * Remove all commands prior to bootstrapping when running unit tests
 */
public function bootstrap() {
    if (env('APP_ENV') == 'testing') {
        $this->commands = [];
    }
    parent::bootstrap();
}

[markdwhite]

@GrahamCampbell

I would like to re-open this for comments.

There is an issue here. It does only apply when Xdebug is in use, and the project has user Commands defined, though this is probably standard at some point in many projects. Though time savings are not huge thay are helpful. And I understand Laravel is still about giving developers something that helps them.

I'd propose a new method Illuminate/Foundation/Console/Kernel::bootstrapBare()

/**
 * Remove all commands prior to bootstrapping (eg: when unit testing)
 *
 * @return void
 */
public function bootstrapBare() {
    $this->commands = [];
    $this->bootstrap();
}

This would be used in laravel/laravel project, test/TestCase.php as a replacement (or left as an alternative for the developer)

$app->make(Illuminate\Contracts\Console\Kernel::class)->bootstrap();

Replaced by

$app->make(Illuminate\Contracts\Console\Kernel::class)->bootstrapBare();

If this is something you'd consider, I'll create a pull request with test coverage.

[jminkler]

Can we just not fix the actual problem, that everything gets instantiated because the signatures are inside the classes themselves? Move them to Kernel in assoc array with the list of commands? Or can a single command have more than 1 sig?

It's not real clear that all the constructors are run when any command is run, maybe at least a note in the docs?