framework

framework copied to clipboard

This is just a failing test, hoping for some help to figure out a fix

I have been trying to test a command that first prompts with a basic select then immediately after prompts with a multisearch.

Im not quite sure what is going on, but it seems the assertions get polluted which each others options, which causes the assertion to fail.

As a side note, to even get to this point, I had to add the following line in ConfiguresPrompts... https://github.com/laravel/framework/blob/2f33b4aabb6936d6ddd3795e79007a6ea76c2d4d/src/Illuminate/Console/Concerns/ConfiguresPrompts.php#L273

otherwise, array_filter errors here....

https://github.com/laravel/framework/blob/2f33b4aabb6936d6ddd3795e79007a6ea76c2d4d/src/Illuminate/Console/Concerns/ConfiguresPrompts.php#L275

@jessarcher I was hoping you may be able to shed some light on all of this as I think I have traced this as far as I can for now and its all getting a bit over my head :confused:

Here is the PHPUnit output when running this test, why is the What is you name? assertion seeing the option of Dr as that is an option from the next prompt?

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

From what I gathered:

https://github.com/laravel/framework/blob/3672fbe6eb71092733330dfc60011e09270812c2/src/Illuminate/Testing/PendingCommand.php#L398-L404

is definitely the culprit as the $argument question does not always match the $question[0] but is always overridden and should be more in line with:

if ($argument->getQuestion() === $question[0] && isset($this->test->expectedChoices[$question[0]])) {
    $this->test->expectedChoices[$question[0]]['actual'] = $argument->getAutocompleterValues();

    return true;
}

return false;

but then you get mockery exception of

Mockery\Exception\InvalidCountException: Method askQuestion(<Closure===true>) from Mockery_1_Illuminate_Console_OutputStyle should be called
 exactly 1 times but called 2 times.

and I have no idea where that expectation is set up as it seems to come from orchestra.

@donnysim thanks for the input, yes, I also saw that message when testing, but I was unable to figure out why

Don't want to be a pest, but have you got any thoughts on this @jessarcher 🙏 Thanks

Hey @JayBizzle,

The test assertion story for the search and multisearch prompts is a bit complicated at the moment.

We rely on the Symfony fallback when running tests so that the existing console test assertions can be used. For example, the text prompt falls back to using a Symfony Question, which can be tested using the expectsQuestion method.

There is no equivalent Symfony component for the search and multisearch prompts. Instead, the fallback must first ask for the user's search string using a Symfony Question, pass the result to the search/multisearch callback, and finally ask the user to select option(s) using a Symfony ChoiceQuestion.

This means you'd need to use the expectsQuestion method followed by the expectsChoice method to mock a single search/multisearch prompt.

I have updated your test accordingly and added standalone tests for the search and multisearch prompts.

This could likely be improved by adding a new expectsSearch method that automatically handles the above. Ultimately, I'd like to build test helpers into Prompts so that the Symfony fallback isn't required during testing.

The reason you needed Arr::wrap was because your expectsChoice assertion instructed the mock to return a string ('Dr') when it should always return an array (['Dr']).

I've added an expectsSearch method, which works for the search and multisearch prompts (similar to how expectsChoice works for the select and multiselect prompts). The API might be a little confusing due to the number of required parameters and order, but I've tried to keep it consistent with the other methods.

I'd still like to build test assertions directly into laravel/prompts, but hopefully, this will help in the meantime.

Really appreciate the input here @jessarcher

I'll give these changes a test against my code base and report back 👍

Thanks

@JayBizzle did you figure that one out?

Just on a different project at the moment, but hoping to get a chance to check this potential fix next week.

Will update when I know more

Thanks

Hey @jessarcher

Sorry it has taken me so long to check out the changes you made. I have finally just got around to testing this out. Looking good so far. Just one thing im not sure why is happening....

I have the following test..

// snip
->expectsSearch(
    'Search for the tables you would like to import',
    'backups/2020-10-08/11:00/fake_table.sql.gz',
    'fake',
    [
        // 'None',
        'backups/2020-10-08/11:00/fake_table.sql.gz',
    ]
)

When the None option is commented out, i get the following PHPUnit failure...

1) Tests\App\Console\Commands\MysqlImporterTest::test_we_can_successfully_import_specific_backup_files
Question "Search for the tables you would like to import" has different options.
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 'backups/2020-10-08/11:00/Y8ejb.sql.gz'
+    0 => 'None'
+    1 => 'backups/2020-10-08/11:00/Y8ejb.sql.gz'
 )

I am really not sure where that None option is coming from.

The code i am testing looks pretty much similar to what your tests look like...

$files = collect(multisearch(
    label: 'Search for the tables you would like to import',
    options: fn (string $value) => strlen($value) > 0
        ? $files->filter(fn ($file) => str_contains($file, $value))->values()->toArray()
        : [],
    hint: 'Use the arrow keys and spacebar to select the tables you would like to import.'
));

Hey @JayBizzle,

Laravel inserts the "None" option because Symfony's choice component requires the user to select an option, while Laravel Prompts allows the user not to select anything when the required argument isn't passed. The "None" option ensures the user can still choose nothing when the Symfony fallback is activated.

However, when running tests, the "None" option should not get inserted:

https://github.com/laravel/framework/blob/f0de069d2c40e77cae9d669bb712a21f9bdee1cb/src/Illuminate/Console/Concerns/ConfiguresPrompts.php#L255-L263

Is it possible that your APP_ENV environment variable is set to something other than testing when running unit tests?

@jessarcher Thanks for the reply.

You are correct, further up the test I was doing something like this (as these commands we have should only be run in production)...

$this->app->detectEnvironment(fn () => 'production');

Ive refactored my tests to get around this, so this now all works as expected 👍

Thanks again