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

[7.x] Allow doing truth-test assertions with just a closure #32626

Merged
merged 9 commits into from
May 1, 2020
Merged

[7.x] Allow doing truth-test assertions with just a closure #32626

merged 9 commits into from
May 1, 2020

Conversation

SjorsO
Copy link
Contributor

@SjorsO SjorsO commented May 1, 2020

Currently, for all the truth-test assertions, you have to first pass in a class, then a closure. If you want your editor to assist you with auto-completion, you have to type the class twice. Once as the first argument for the assertQueued method, and once as the type hint for the parameter of the closure.

Mail::assertQueued(SubpictureFinishedEmail::class, function (SubpictureFinishedEmail $email) use ($user) {
    return $email->user->id === $user->id;
});

// or with a short closure

Mail::assertQueued(SubpictureFinishedEmail::class, fn (SubpictureFinishedEmail $email) => $email->user->id === $user->id);

This PR, using a little reflection magic, infers the type from the parameter of the closure. This removes the duplication, and allows you to write the assertions like this instead:

Mail::assertQueued(function (SubpictureFinishedEmail $email) use ($user) {
    return $email->user->id === $user->id;
});

// or with a short closure

Mail::assertQueued(fn (SubpictureFinishedEmail $email) => $email->user->id === $user->id);

This isn't a breaking change, all modified methods will currently throw an exception if you give them a closure as the first argument.

@taylorotwell taylorotwell merged commit 56b11d4 into laravel:7.x May 1, 2020
@taylorotwell
Copy link
Member

Nice PR. Thanks ❤️

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.

2 participants