Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[6.x] Fix testing commands that do not resolve 'OutputStyle::class' from the container #32687

Merged
merged 1 commit into from
May 7, 2020

Conversation

olsgreen
Copy link
Contributor

@olsgreen olsgreen commented May 5, 2020

Tests that look for output (via expectsOutput) from commands that do not resolve OutputStyle from the application container fail unexpectedly.

An example of this is the default Symfony 'list' command, this command is not a descendant of Illuminate\Console\Command and therefore doesn't resolve its output interface from the container.

You can verify the behaviour by adding the following test to the stock FeatureTest:

public function testList()
{
    $this->artisan('list')
        ->expectsOutput("Available commands:")
        ->assertExitCode(0);
}

This tests the list command, which will fail if run against the current codebase but pass against this fix.

This shouldn't break anything as we're simply passing the mock directly to the kernels call method as opposed to relying on it being resolved.

If accepted, I'm guessing this can also be merged up?

@taylorotwell
Copy link
Member

I'm curious what command you are actually trying to test? If it's not a Laravel command, I'm curious if we should really expect Laravel's helpers to be able to test it, etc.

@olsgreen
Copy link
Contributor Author

olsgreen commented May 6, 2020

I am working with a command derived from a parent that is part of a package originally intended for Symfony.

I can your reasoning, this is an edge case and I can easily work around the issue, it would have been nice for it to work out of the box.

Also, this is part of the reason the output dumping PR I submitted failed for you, you were trying to test 'list' a Symfony command. (the other reason is it's crappy code that shouldn't have been a PR in the first place 🙈, revised PR coming)

@taylorotwell taylorotwell merged commit c2e3c45 into laravel:6.x May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants