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

[11.x] Make expectsChoice assertion more intuitive with associative arrays. #52408

Merged
merged 6 commits into from
Aug 7, 2024

Conversation

jessarcher
Copy link
Member

@jessarcher jessarcher commented Aug 7, 2024

Currently, the expectsChoice test method behaves in an unusual way when passing an associative array to the choice method (or the select prompt).

For example, imagine the following scenario:

$this->choice('Choose an option', [
    'one' => 'One',
    'two' => 'Two',
    'three' => 'Three',
]);

// or

select('Choose an option', [
    'one' => 'One',
    'two' => 'Two',
    'three' => 'Three',
]);

To use expectsChoice, you would need to write the test like this:

$this->expectsChoice('Choose an option', 'one', [
    'one',
    'two',
    'three',
    'One',
    'Two',
    'Three',
]);

This is because the available options are retrieved from Symfony using the getAutocompleterValues method. Technically, this could be considered correct because with Symfony, the user must type a value to select an option, and they can either type a key or a value. However, I don't think it's very intuitive, and with the select prompt, it's even less intuitive because the keys are never exposed to the user.

This PR addresses this by detecting the available options using the getChoices method instead, which allows an associative array to be used in the expectsChoice method like so:

$this->expectsChoice('Choose an option', 'one', [
    'one' => 'One',
    'two' => 'Two',
    'three' => 'Three',
]);

To avoid breaking backwards compatibility for anyone relying on the current behaviour, this only uses the getChoice method when an associative array is passed to expectsChoice, meaning that passing a list will continue to function as it currently does.

Fixes laravel/prompts#158

@jessarcher jessarcher marked this pull request as ready for review August 7, 2024 04:10
@taylorotwell taylorotwell merged commit 5a9d85b into 11.x Aug 7, 2024
32 checks passed
@taylorotwell taylorotwell deleted the expects-choice-associative-array branch August 7, 2024 21:02
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.

Prompt::fake() incompatible with PendingCommand created by $this->artisan()?
2 participants