expectsOutput breaks next test on fail

[paulhenri-l]

• Laravel Version: 5.8.29
• PHP Version: 7.3.7
• Database Driver & Version: No database used

Description:

When using expectsOutput in two consecutive tests if the first one fails the second one will also fail.

Steps To Reproduce:

Edited: Better steps to reproduce given a few comments later

What it may be?

After looking a bit in the source code I think I may have an idea of what is causing this bug.

Under the hood the expectsOutput function reports errors in its beforeApplicationDestroyed callback using the $this->fail() method.

The fail method actually throws an exception preventing subsequent beforeApplicationDestroyed callback from being called thus leaving the test suite in an unexpected state.

I've first found out about this bug at work where I am using the DatabaseTransaction trait. If I get a failed expectsOutput assertion my whole test suite breaks because the transaction is not rolled back and subsequent tests cannot communicate with the db because of the transaction's lock.

How could it be fixed?

We could add a new afterApplicationDestroyed callback system and use it in the InteractsWithConsole trait.

These callbacks would get called at the end of the tearDown function.

If fixing this bug is not too much of a hurry for you and you are ok with the addition of the afterApplicationDestroyed callback system - I will do it :)

[themsaid]

I honestly don't understand what your problem is, can you please share simple steps to reproduce the issue? The fail method is used with testing notifications too, is that a problem to you as well or just artisan commands?

[paulhenri-l]

No problem, it actually took me a while to expose and understand this bug.

Steps to reproduce

The simplest way to reproduce it is to add this test into a brand new laravel application

laravel new expose-bug
cd expose-bug
php artisan make:test ExposeBugCommand

Paste this inside of the tests/Feature/ExposeBugCommand.php file.

<?php

namespace Tests\Feature;

use Illuminate\Support\Facades\Artisan;
use Tests\TestCase;

class ExposeBugCommandTest extends TestCase
{
    protected function setUp(): void
    {
        parent::setUp();

        Artisan::command('hello', function () {
            $this->info('hello world');
        });
    }

    public function test_hello_says_hola_mundo()
    {
        $this->artisan('hello')
            ->expectsOutput('hola mundo');
    }

    public function test_hello_says_hello_world()
    {
        $this->artisan('hello')
            ->expectsOutput('hello world');
    }
}

phpunit

Explanations

By simply looking at this test you can see that the first test method should fail and the second one pass.

But if you run phpunit oddly both tests fails (1 failure and 1 exception)

Now if you run phpunit --filter test_hello_says_hello_world you'll see that running only the working test works.

What I have discovered is that InteractsWithConsole when used with expectsOutput is calling this->fail() inside of its beforeApplicationDestroyedCallback as this->fails() throws it is breaking the execution flow of the tearDown method and preventing it from properly tearing down the test suite.

Inside of the tearDown method there is a call to Mockery::close() but because of the this->fail() exception this line is never reached.

On subsequent tests Mockery::close() gets called and throws an error related to the previous test.

Notifications

If there are other test traits using this->fail() in their callbacks then they are also preventing the test suite from properly tearing down and should be fixed. I can do that.

PRs

We've been discussing the fix with @GrahamCampbell and @crynobone inside of these pr:

• [5.8] Fix expects output bug #29248
• [5.9] Fix expects output bug #29252
• Fix expectsOutput bug orchestral/testbench-core#25

Hope it helps 😃 if it's still not clear do not hesitate I'll do my best to give extra details.

[paulhenri-l]

The issue is now getting fixed in #29267

[paulhenri-l]

Resolved in #29267 closing.