-
-
Notifications
You must be signed in to change notification settings - Fork 365
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
Feat: Add a "toPassAny" expectation method #1286
base: 3.x
Are you sure you want to change the base?
Conversation
* | ||
* @throws AssertionFailedError Rethrows first caught exception on failure | ||
*/ | ||
public function toPassAny(\Closure ...$tests): Expectation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to recommend toBeOneOf()
, and to allow non-closure values that will be converted to toBe()
assertions. This allows for:
expect($foo)->toBeOneOf(1, 2, 3);
Here's the code:
public function toBeOneOf(mixed ...$tests): Expectation {
$firstException = null;
if ($tests === []) {
return $this;
}
foreach ($tests as $test) {
if (!($test instanceof \Closure)) {
$test = fn ($expectation) => $expectation->toBe($test);
}
try {
$test(new Expectation($this->value));
return $this;
} catch (AssertionFailedError $e) {
$firstException ??= $e;
}
}
/** @var AssertionFailedError $firstException */
throw $firstException;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me "toBeOneOf" both implies an exact count and hides the fact that any expectation could be provided.
I also don't like falling back to a default expectation because in my opinion it makes the behavior confusing when your array might contain values that are closures. Id prefer an additional ->toBeAny(...)
that does something like return $this->toPassAny(array_map(fn($i) => fn($e) => $e->toBe($i), $items));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toBeAny is good, or toBeAnyOf().
You shouldn't be passing an array, it's variadic. Your example above should spread the array as args (… array_map(…)
) or it won't work with any of the code…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually like the idea of "toBeOneOf" as you say the name implies, an xor type deal. I wrote it and then ditched it but it sounds more useful with that name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah you'd need to spread it. Not the easiest code snippet to write on mobile 🙂
What:
Description:
Add a
toPassAny
expectation to allow for effective->or
branches:For example if I want to make sure an array only contains
(InterfaceA & InterfaceB) | OtherBaseClass
Open questions
I don't believe so, the functionality
toPassAny
provides is fully encapsulated and doesn't have any effect on later expectation calls.If we have an exception that outputs all the failures, we run the risk of outputting very long failures when a lot of branches are provided.
If we have an exception that is generic and doesn't output any of the failures, we make it hard to debug issues folks run into when tests fail.