Skip to content

Commit

Permalink
[11.x] Fix expectsChoice assertion with optional multiselect prom…
Browse files Browse the repository at this point in the history
…pts. (#51078)

* Replace `PromptOption` with customized `ChoiceQuestion`

* Fix expectsChoice assertion for optional `multiselect`

* Fix prompt fallback test

* Update Choice.php

---------

Co-authored-by: Taylor Otwell <[email protected]>
  • Loading branch information
jessarcher and taylorotwell authored Apr 16, 2024
1 parent 6a8b505 commit f57756a
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 100 deletions.
33 changes: 19 additions & 14 deletions src/Illuminate/Console/Concerns/ConfiguresPrompts.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace Illuminate\Console\Concerns;

use Illuminate\Console\PromptOption;
use Illuminate\Console\PromptValidationException;
use Laravel\Prompts\ConfirmPrompt;
use Laravel\Prompts\MultiSearchPrompt;
Expand Down Expand Up @@ -231,11 +230,13 @@ protected function restorePrompts()
*/
private function selectFallback($label, $options, $default = null)
{
if ($default !== null) {
$default = array_search($default, array_is_list($options) ? $options : array_keys($options));
$answer = $this->components->choice($label, $options, $default);

if (! array_is_list($options) && $answer === (string) (int) $answer) {
return (int) $answer;
}

return PromptOption::unwrap($this->components->choice($label, PromptOption::wrap($options), $default));
return $answer;
}

/**
Expand All @@ -249,24 +250,28 @@ private function selectFallback($label, $options, $default = null)
*/
private function multiselectFallback($label, $options, $default = [], $required = false)
{
$options = PromptOption::wrap($options);
$default = $default !== [] ? implode(',', $default) : null;

if ($required === false) {
$options = [new PromptOption(null, 'None'), ...$options];
if ($required === false && ! $this->laravel->runningUnitTests()) {
$options = array_is_list($options)
? ['None', ...$options]
: ['' => 'None'] + $options;

if ($default === []) {
$default = [null];
if ($default === null) {
$default = 'None';
}
}

$default = $default !== []
? implode(',', array_keys(array_filter($options, fn ($option) => in_array($option->value, $default))))
: null;
$answers = $this->components->choice($label, $options, $default, null, true);

$answers = PromptOption::unwrap($this->components->choice($label, $options, $default, multiple: true));
if (! array_is_list($options)) {
$answers = array_map(fn ($value) => $value === (string) (int) $value ? (int) $value : $value, $answers);

This comment has been minimized.

Copy link
@joelclermont

joelclermont Jun 2, 2024

PendingCommand::expectsChoice defines $answer as allowing array|string, but this line will throw a TypeError if you pass it a string now since array_map requires the second param to be an array.

}

if ($required === false) {
return array_values(array_filter($answers, fn ($value) => $value !== null));
return array_is_list($options)
? array_values(array_filter($answers, fn ($value) => $value !== 'None'))
: array_filter($answers, fn ($value) => $value !== '');
}

return $answers;
Expand Down
60 changes: 0 additions & 60 deletions src/Illuminate/Console/PromptOption.php

This file was deleted.

21 changes: 20 additions & 1 deletion src/Illuminate/Console/View/Components/Choice.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,29 @@ public function render($question, $choices, $default = null, $attempts = null, $
{
return $this->usingQuestionHelper(
fn () => $this->output->askQuestion(
(new ChoiceQuestion($question, $choices, $default))
$this->getChoiceQuestion($question, $choices, $default)
->setMaxAttempts($attempts)
->setMultiselect($multiple)
),
);
}

/**
* Get a ChoiceQuestion instance that handles array keys like Prompts.
*
* @param string $question
* @param array $choices
* @param mixed $default
* @return \Symfony\Component\Console\Question\ChoiceQuestion
*/
protected function getChoiceQuestion($question, $choices, $default)
{
return new class($question, $choices, $default) extends ChoiceQuestion
{
protected function isAssoc(array $array): bool
{
return ! array_is_list($array);
}
};
}
}
55 changes: 30 additions & 25 deletions tests/Console/ConfiguresPromptsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Illuminate\Console\Command;
use Illuminate\Console\OutputStyle;
use Illuminate\Console\View\Components\Factory;
use Laravel\Prompts\Prompt;
use Mockery as m;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;
Expand All @@ -23,8 +24,10 @@ protected function tearDown(): void
}

#[DataProvider('selectDataProvider')]
public function testSelectFallback($prompt, $expectedDefault, $selection, $expectedReturn)
public function testSelectFallback($prompt, $expectedOptions, $expectedDefault, $return, $expectedReturn)
{
Prompt::fallbackWhen(true);

$command = new class($prompt) extends Command
{
public $answer;
Expand All @@ -42,8 +45,8 @@ public function handle()

$this->runCommand($command, fn ($components) => $components
->expects('choice')
->withArgs(fn ($question, $options, $default) => $default === $expectedDefault)
->andReturnUsing(fn ($question, $options, $default) => $options[$selection])
->with('Test', $expectedOptions, $expectedDefault)
->andReturn($return)
);

$this->assertSame($expectedReturn, $command->answer);
Expand All @@ -52,18 +55,20 @@ public function handle()
public static function selectDataProvider()
{
return [
'list with no default' => [fn () => select('foo', ['a', 'b', 'c']), null, 1, 'b'],
'numeric keys with no default' => [fn () => select('foo', [1 => 'a', 2 => 'b', 3 => 'c']), null, 1, 2],
'assoc with no default' => [fn () => select('foo', ['a' => 'A', 'b' => 'B', 'c' => 'C']), null, 1, 'b'],
'list with default' => [fn () => select('foo', ['a', 'b', 'c'], 'b'), 1, 1, 'b'],
'numeric keys with default' => [fn () => select('foo', [1 => 'a', 2 => 'b', 3 => 'c'], 2), 1, 1, 2],
'assoc with default' => [fn () => select('foo', ['a' => 'A', 'b' => 'B', 'c' => 'C'], 'b'), 1, 1, 'b'],
'list with no default' => [fn () => select('Test', ['a', 'b', 'c']), ['a', 'b', 'c'], null, 'b', 'b'],
'numeric keys with no default' => [fn () => select('Test', [1 => 'a', 2 => 'b', 3 => 'c']), [1 => 'a', 2 => 'b', 3 => 'c'], null, '2', 2],
'assoc with no default' => [fn () => select('Test', ['a' => 'A', 'b' => 'B', 'c' => 'C']), ['a' => 'A', 'b' => 'B', 'c' => 'C'], null, 'b', 'b'],
'list with default' => [fn () => select('Test', ['a', 'b', 'c'], 'b'), ['a', 'b', 'c'], 'b', 'b', 'b'],
'numeric keys with default' => [fn () => select('Test', [1 => 'a', 2 => 'b', 3 => 'c'], 2), [1 => 'a', 2 => 'b', 3 => 'c'], 2, '2', 2],
'assoc with default' => [fn () => select('Test', ['a' => 'A', 'b' => 'B', 'c' => 'C'], 'b'), ['a' => 'A', 'b' => 'B', 'c' => 'C'], 'b', 'b', 'b'],
];
}

#[DataProvider('multiselectDataProvider')]
public function testMultiselectFallback($prompt, $expectedDefault, $selection, $expectedReturn)
public function testMultiselectFallback($prompt, $expectedOptions, $expectedDefault, $return, $expectedReturn)
{
Prompt::fallbackWhen(true);

$command = new class($prompt) extends Command
{
public $answer;
Expand All @@ -81,8 +86,8 @@ public function handle()

$this->runCommand($command, fn ($components) => $components
->expects('choice')
->withArgs(fn ($question, $options, $default, $multiple) => $default === $expectedDefault && $multiple === true)
->andReturnUsing(fn ($question, $options, $default, $multiple) => array_values(array_filter($options, fn ($index) => in_array($index, $selection), ARRAY_FILTER_USE_KEY)))
->with('Test', $expectedOptions, $expectedDefault, null, true)
->andReturn($return)
);

$this->assertSame($expectedReturn, $command->answer);
Expand All @@ -91,18 +96,18 @@ public function handle()
public static function multiselectDataProvider()
{
return [
'list with no default' => [fn () => multiselect('foo', ['a', 'b', 'c']), '0', [2, 3], ['b', 'c']],
'numeric keys with no default' => [fn () => multiselect('foo', [1 => 'a', 2 => 'b', 3 => 'c']), '0', [2, 3], [2, 3]],
'assoc with no default' => [fn () => multiselect('foo', ['a' => 'A', 'b' => 'B', 'c' => 'C']), '0', [2, 3], ['b', 'c']],
'list with default' => [fn () => multiselect('foo', ['a', 'b', 'c'], ['b', 'c']), '2,3', [2, 3], ['b', 'c']],
'numeric keys with default' => [fn () => multiselect('foo', [1 => 'a', 2 => 'b', 3 => 'c'], [2, 3]), '2,3', [2, 3], [2, 3]],
'assoc with default' => [fn () => multiselect('foo', ['a' => 'A', 'b' => 'B', 'c' => 'C'], ['b', 'c']), '2,3', [2, 3], ['b', 'c']],
'required list with no default' => [fn () => multiselect('foo', ['a', 'b', 'c'], required: true), null, [1, 2], ['b', 'c']],
'required numeric keys with no default' => [fn () => multiselect('foo', [1 => 'a', 2 => 'b', 3 => 'c'], required: true), null, [1, 2], [2, 3]],
'required assoc with no default' => [fn () => multiselect('foo', ['a' => 'A', 'b' => 'B', 'c' => 'C'], required: true), null, [1, 2], ['b', 'c']],
'required list with default' => [fn () => multiselect('foo', ['a', 'b', 'c'], ['b', 'c'], required: true), '1,2', [1, 2], ['b', 'c']],
'required numeric keys with default' => [fn () => multiselect('foo', [1 => 'a', 2 => 'b', 3 => 'c'], [2, 3], required: true), '1,2', [1, 2], [2, 3]],
'required assoc with default' => [fn () => multiselect('foo', ['a' => 'A', 'b' => 'B', 'c' => 'C'], ['b', 'c'], required: true), '1,2', [1, 2], ['b', 'c']],
'list with no default' => [fn () => multiselect('Test', ['a', 'b', 'c']), ['None', 'a', 'b', 'c'], 'None', ['None'], []],
'numeric keys with no default' => [fn () => multiselect('Test', [1 => 'a', 2 => 'b', 3 => 'c']), ['' => 'None', 1 => 'a', 2 => 'b', 3 => 'c'], 'None', [''], []],
'assoc with no default' => [fn () => multiselect('Test', ['a' => 'A', 'b' => 'B', 'c' => 'C']), ['' => 'None', 'a' => 'A', 'b' => 'B', 'c' => 'C'], 'None', [''], []],
'list with default' => [fn () => multiselect('Test', ['a', 'b', 'c'], ['b', 'c']), ['None', 'a', 'b', 'c'], 'b,c', ['b', 'c'], ['b', 'c']],
'numeric keys with default' => [fn () => multiselect('Test', [1 => 'a', 2 => 'b', 3 => 'c'], [2, 3]), ['' => 'None', 1 => 'a', 2 => 'b', 3 => 'c'], '2,3', ['2', '3'], [2, 3]],
'assoc with default' => [fn () => multiselect('Test', ['a' => 'A', 'b' => 'B', 'c' => 'C'], ['b', 'c']), ['' => 'None', 'a' => 'A', 'b' => 'B', 'c' => 'C'], 'b,c', ['b', 'c'], ['b', 'c']],
'required list with no default' => [fn () => multiselect('Test', ['a', 'b', 'c'], required: true), ['a', 'b', 'c'], null, ['b', 'c'], ['b', 'c']],
'required numeric keys with no default' => [fn () => multiselect('Test', [1 => 'a', 2 => 'b', 3 => 'c'], required: true), [1 => 'a', 2 => 'b', 3 => 'c'], null, ['2', '3'], [2, 3]],
'required assoc with no default' => [fn () => multiselect('Test', ['a' => 'A', 'b' => 'B', 'c' => 'C'], required: true), ['a' => 'A', 'b' => 'B', 'c' => 'C'], null, ['b', 'c'], ['b', 'c']],
'required list with default' => [fn () => multiselect('Test', ['a', 'b', 'c'], ['b', 'c'], required: true), ['a', 'b', 'c'], 'b,c', ['b', 'c'], ['b', 'c']],
'required numeric keys with default' => [fn () => multiselect('Test', [1 => 'a', 2 => 'b', 3 => 'c'], [2, 3], required: true), [1 => 'a', 2 => 'b', 3 => 'c'], '2,3', ['2', '3'], [2, 3]],
'required assoc with default' => [fn () => multiselect('Test', ['a' => 'A', 'b' => 'B', 'c' => 'C'], ['b', 'c'], required: true), ['a' => 'A', 'b' => 'B', 'c' => 'C'], 'b,c', ['b', 'c'], ['b', 'c']],
];
}

Expand All @@ -112,7 +117,7 @@ protected function runCommand($command, $expectations)

$application->shouldReceive('make')->withArgs(fn ($abstract) => $abstract === OutputStyle::class)->andReturn($outputStyle = m::mock(OutputStyle::class));
$application->shouldReceive('make')->withArgs(fn ($abstract) => $abstract === Factory::class)->andReturn($factory = m::mock(Factory::class));
$application->shouldReceive('runningUnitTests')->andReturn(true);
$application->shouldReceive('runningUnitTests')->andReturn(false);
$application->shouldReceive('call')->with([$command, 'handle'])->andReturnUsing(fn ($callback) => call_user_func($callback));
$outputStyle->shouldReceive('newLinesWritten')->andReturn(1);

Expand Down
34 changes: 34 additions & 0 deletions tests/Integration/Console/PromptsAssertionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,38 @@ public function handle()
->expectsChoice('Which names do you like?', ['John', 'Jane'], ['John', 'Jane', 'Sally', 'Jack'])
->expectsOutput('You like John, Jane.');
}

public function testAssertionForOptionalMultiselectPrompt()
{
$this->app[Kernel::class]->registerCommand(
new class extends Command
{
protected $signature = 'test:multiselect';

public function handle()
{
$names = multiselect(
label: 'Which names do you like?',
options: ['John', 'Jane', 'Sally', 'Jack'],
);

if (empty($names)) {
$this->line('You like nobody.');
} else {
$this->line(sprintf('You like %s.', implode(', ', $names)));
}
}
}
);

$this
->artisan('test:multiselect')
->expectsChoice('Which names do you like?', ['John', 'Jane'], ['John', 'Jane', 'Sally', 'Jack'])
->expectsOutput('You like John, Jane.');

$this
->artisan('test:multiselect')
->expectsChoice('Which names do you like?', ['None'], ['John', 'Jane', 'Sally', 'Jack'])
->expectsOutput('You like nobody.');
}
}

0 comments on commit f57756a

Please sign in to comment.