Allow static validator to run before prompt's validator.
Summary
Prompts allow to specify a validator for a prompt using validate() closure or static $validateUsing closure.
Prompt::validate() contains a match() with:
$error = match (true) {
is_callable($this->validate) => ($this->validate)($value),
isset(static::$validateUsing) => (static::$validateUsing)($this),
default => throw new RuntimeException('The validation logic is missing.'),
};
This will call the Prompt's validator before the static validator.
Problem
When testing prompts (via PHPUnit or Pest) in consumer projects using interactive inputs, the prompt that has failed the validation will halt the test execution expecting an input.
To work around this, a test executor (PHPUnit in the example below), could override the handling of the incorrect validator to, for example, throw an exception instead of re-asking for an input, and then assert for that exception in the test:
// Fake input.
Prompt::fake(['t', 'e', 's', 't', Key::ENTER]);
// Intercept handling of the validation by throwing an exception.
Prompt::validateUsing(function (Prompt $prompt) {
if (is_callable($prompt->validate)) {
$error = ($prompt->validate)($prompt->value()); // <--- similar to what Prompt::validate() does.
// Intercepting and throwing an exception that will later be picked up by a test and would fail an assertion.
if ($error) {
throw new \RuntimeException(sprintf('Validation for "%s" failed with error "%s".', $prompt->label, $error));
}
}
return NULL;
});
// call the prompt
...
// Assert for the exception.
...
Before this PR, the static $validateUsing will not be assessed if the Prompt's validate() closure is set, making it impossible to intercept the validation handling.
After this PR is merged, the static $validateUsing will take precedence and will allow to intercept the validation.
Changes
- Updated the order of validation callback checks to check for
static::$validateUsing - Updated
validateUsing()method to allow acceptingNULLas a callback to allow resetting the value of$validateUsing. Note that the property value already allows to have aNULL'ified callback, so the changes to this setter follow that property type definition. - Updated tests that were setting the
static::$validateUsingand then erroneously resetting the value usingfn() => nullto clear it: this was setting the value to an empty closure that would return aNULLinstead of unsetting the callback completely, so the intended "resetting" was not working correctly resulting in "bleeding" of thestatic::$validateUsinginto other tests; this was not picked up by tests until a change was made in this PR.
I think this will cause issues with a feature in laravel/framework.
The validateUsing method was introduced in #102 to allow Laravel users to pass a string or array of Laravel validation rules to Prompts in https://github.com/laravel/framework/pull/49497. E.g:
text('What is your name?', validate: 'required|min:2');
Anyone using Prompts with Laravel will have $validateUsing defined by the framework, and currently, that implementation expects to be used only as a fallback. With this change, if someone passes a validation callback directly to a Prompt, that callback will be passed to Laravel's validation fallback, which doesn't handle closures.
In Laravel, we enable the testing of Prompts by disabling interactive mode and defining fallbacks that are used when the Prompt isn't interactive. E.g:
Prompt::interactive(false); // Only called when running tests
TextPrompt::fallbackUsing(function (TextPrompt $prompt) {
// ...
});
This almost entirely bypasses the Prompts package, but it requires that each fallback handles the various signatures and behaviours of every Prompt.
The Prompt::fake() method was only intended for internally testing Prompts handling of individual keypresses and render cycles and in my opinion isn't a great DX for folks to test their apps' usage of Prompts.
I'm open to improving Prompts testability, but I think it should be done with a new API specifically designed to assist in testing code that uses Prompts (rather than testing Prompts itself). I.e. allow asserting that a given Prompt was called (and with the correct arguments) and to mock the return value. E.g.
TextPrompt::expects('What is your name')->andReturns('Jess');
@jessarcher Thank you for reviewing the PR.
I've looked through the functionality you are referring to in laravel and can see that the validatePrompt() expects rules to be passed as an array.
I agree that this change will break that functionality if validate is callable.
But from the point of Prompts, Laravel is a consumer project and should be able to adapt to changes in Prompts.
validatePrompt() in Laravel can be updated to either evaluate the validate callback or to pass it further.
While I agree with the notion of building a better testing API, this PR addresses an existing API introduced in another PR. Specifically, $validateUsing is NULL by default and the change here just follows that up to allow re-setting the value from the setter.
Since this current test API is the only test API we, the consumer project authors, currently have, the proposed change to re-order the match() would allow us to use it in our tests without halting and waiting for input.
Implemented this in my fork https://github.com/AlexSkrypnyk/prompts