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

[7.x] Add console test method to expect a confirmation question #31965

Merged
merged 2 commits into from
Mar 14, 2020
Merged

[7.x] Add console test method to expect a confirmation question #31965

merged 2 commits into from
Mar 14, 2020

Conversation

ShawnCZek
Copy link
Contributor

While creating tests for an Artisan command, I came to a quite interesting situation. I found out that expecting a confirmation question is different than answering any other question because answers such as 'Yes' or 'No' do not work here. Actually, a bool value has to be passed.

Scenario

This is an example of a command that could be tested:

<?php

namespace App\Console\Commands;

use Illuminate\Console\Command;

class ExampleCommand extends Command
{
    /**
     * The name and signature of the console command.
     *
     * @var string
     */
    protected $signature = 'foo:bar';

    /**
     * The console command description.
     *
     * @var string
     */
    protected $description = 'An example of a command with a confirmation question';

    /**
     * Execute the console command.
     *
     * @return int
     */
    public function handle(): int
    {
        if (! $this->confirm('Do you want to continue?')) {
            return 1;
        }

        $this->info('Completed the task successfully.');

        return 0;
    }
}

An example of the test following the official documentation:

<?php

namespace Tests\Feature;

use Tests\TestCase;

class ExampleCommandTest extends TestCase
{
    public function testHandle()
    {
        $this->artisan('foo:bar')
            ->expectsQuestion('Do you want to continue?', 'no')
            ->assertExitCode(1);
    }
}

Running this test will result into the following error:

Expected status code 1 but received 0.
Failed asserting that 0 matches expected 1.
Expected :1
Actual   :0

The confirmation question requires true or false as the result value. So the right and confusing way is following:

$this->artisan('foo:bar')
    ->expectsQuestion('Do you want to continue?', false)
    ->assertExitCode(1);

Conclusion

To comply with the available method in Artisan commands for the confirmation question, confirm(), I created a test method and updated the documentation for the expectsQuestion() method.

The example test after merging this PR would look like this:

<?php

namespace Tests\Feature;

use Tests\TestCase;

class ExampleCommandTest extends TestCase
{
    public function testHandle()
    {
        $this->artisan('foo:bar')
            ->expectsConfirmation('Do you want to continue?', 'no')
            ->assertExitCode(1);
    }
}

* Specify a confirmation question that should be asked when the command runs.
*
* @param string $question
* @param string $answer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this not allowing bool, but the above is?

Copy link
Contributor Author

@ShawnCZek ShawnCZek Mar 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method checks whether the answer is yes/no.
The method above also accepts a bool value as it is the way how to test confirmation questions.

Also, in this method I am passing a bool value to the method above and according to the documentation, it was not possible before. But actually it was.

@taylorotwell taylorotwell merged commit 1c5d64d into laravel:7.x Mar 14, 2020
@ShawnCZek ShawnCZek deleted the feature/expects-confirmation-in-console branch March 15, 2020 11:16
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.

3 participants