lorisleiva / laravel-actions

⚡️ Laravel components that take care of one specific task
https://laravelactions.com
MIT License
2.52k stars 124 forks source link

Add assert with params convenience methods #286

Closed patrickomeara closed 3 months ago

patrickomeara commented 3 months ago

Problem

When I am testing that actions are pushed correctly, I almost always want to check the parameters that it is pushed with.

Currently it is a little cumbersome to check the params, as they are passed in as an array.

SendPodcastPublishedEmail::dispatch($user, $podcast);

// currently...
SendPodcastPublishedEmail::assertPushed(fn ($a, $params) => $params[0]->is($user) && $params[1]->is($podcast));

Additionally, the thing I want to check is in the second argument so I have an unused variable.

Solution

If we spread the params into the callback we can use type safety to check we have the correct type and shortcut the param check.

SendPodcastPublishedEmail::dispatch($user, $podcast);

// assert by closure
SendPodcastPublishedEmail::assertPushedWithParams(fn (User $u, Podcast $p) => $user->is($u) && $podcast->is($p));

Or we can assert by an array

// assert by array
SendPodcastPublishedEmail::assertPushedWithParams([$user, $podcast]);

Or my favourite way if the action only has a single param.

SendWelcomeEmail::dispatch($user);

// assert by first class callable
SendWelcomeEmail::assertPushedWithParams($user->is(...));

This PR also adds the ability to assert the queue using assertPushedWithParamsOn(), and assert that an action wasn't pushed with params using assertNotPushedWithParams()

I later found a test helper assertJobPushedWith() that could be replaced.

Note: Naming the methods assertPushedWith(), assertNotPushedWith() also works well, but sounds a little strange when checking the queue as well assertPushedWithOn() I'm easy either way.

lorisleiva commented 3 months ago

Hiya! 👋

Thanks for the great PR!

I'm really liking this and I love the fact that this kills a test helper. It's usually a good sign that this is needed for the API. However, I'm having a hard time digesting the name assertPushedWithParamsOn haha.

Also, I'm not sure if this is now the convention in Laravel, but in testing frameworks, I'm more used to a single With suffix instead of WithParams. I think the following API could be a bit more intuitive.

SendPodcastPublishedEmail::assertPushedWith([$user, $podcast]);
SendPodcastPublishedEmail::assertPushedOn("someQueue");

What do you think?

patrickomeara commented 3 months ago

I think you're right about the naming, I'll update that and clean it up. Let me have a think about the queue, because having it in two different assertions could assert two different jobs have been pushed.

patrickomeara commented 3 months ago

I've cleaned up the naming, and it feels much better.

The queue can be asserted using:

SendPodcastPublishedEmail::assertPushedWith([$user, $podcast], 'someQueue');
lorisleiva commented 3 months ago

Perfect, thank you for this! 🍺