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

[10.x] Add serializeAndRestore() to QueueFake andBusFake #48131

Merged
merged 11 commits into from Aug 25, 2023

Conversation

cosmastech
Copy link
Contributor

@cosmastech cosmastech commented Aug 21, 2023

Some context


Queues are awesome. Jobs are awesome. Faking within tests is awesome.

But testing that a job is pushed is not always enough. Nor is testing the job's properties match expected values.

The normal process for jobs is that they are pushed to the queue where they are serialized and deserialized when they are run by a worker process. If there is an issue trying to serialize an object for your queue, it will fail... but probably not within your test suite 😟

Take for instance this job:

class MyJob implements ShouldQueue
{
    use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;

    public function __construct(public $value) {}
}

If we write a unit test like:

public function testCanDispatchJob()
{
    Queue::fake();
    MyJob::dispatch(fn() => true);
    Queue::assertDispatched(MyJob::class, fn($job) => $job->value instanceof \Closure); ✅ 
}

We see no problem in our test suite so it gets approved and merged... but then we release it into production and all of these jobs fail to serialize.

image

Instead, now we can write our tests to leverage QueueFake@serializeAndRestoreJobs(). This simulates the queuing process insofar that we will call serialize() and unserialize() on the job.

public function testCanDispatchJobImproved()
{
    Queue::fake()->serializeAndRestore();

    MyJob::dispatch(fn() => true); // ❌ Exception  Serialization of 'Closure' is not allowed. 🥳 
}

Will gladly add a PR(s) for this to be added to EventFake and BusFake.

@cosmastech cosmastech changed the title [10.x] QueueFake@serializeAndRestoreJobs() [10.x] QueueFake@serializeAndRestoreItems() Aug 21, 2023
@taylorotwell
Copy link
Member

Can you add to Bus as well and mark as ready for review?

@taylorotwell taylorotwell marked this pull request as draft August 23, 2023 19:31
@cosmastech cosmastech changed the title [10.x] QueueFake@serializeAndRestoreItems() [10.x] Add serializeAndRestore() to QueueFake andBusFake Aug 24, 2023
@cosmastech cosmastech marked this pull request as ready for review August 24, 2023 00:57
@cosmastech
Copy link
Contributor Author

cosmastech commented Aug 24, 2023

Can you add to Bus as well and mark as ready for review?

@taylorotwell Done.

I know that you moved the logic out of the trait I had originally created. My two cents is that it would be a lot cleaner to leave it as a trait rather than duplicating the code in BusFake && QueueFake. I can make that change if you agree.

Also, not sure if we want to add this for EventFake as well?

@taylorotwell taylorotwell merged commit 02c61c3 into laravel:10.x Aug 25, 2023
18 of 19 checks passed
@cosmastech cosmastech deleted the feature/add-serializing branch August 25, 2023 14:30
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.

None yet

2 participants