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

[9.x] Adds partial queue faking #41344

Closed
wants to merge 9 commits into from
Closed

[9.x] Adds partial queue faking #41344

wants to merge 9 commits into from

Conversation

yoeriboven
Copy link
Contributor

@yoeriboven yoeriboven commented Mar 4, 2022

This PR allows you to only fake certain jobs.

Useful if you want to test if one job dispatches another job.

<?php

namespace App\Jobs;

use ...

class ParentJob
{
    public function handle() 
    {
        ChildJob::dispatch();
    }
}
/** @test */
public function it_dispatches_another_job()
{
    Queue::fake(ChildJob::class);

    ParentJob::dispatch();

    Queue::assertPushed(ChildJob::class);
}

When using Queue::fake() all jobs will be faked. Therefore the ParentJob won't run and will never dispatch the ChildJob.

With this addition you can choose which job to fake. Not passing a parameter will fake all jobs like before.

Example: I run a web scraper which scrapes the homepage of a news site in a job. That job will then dispatch a job for every article it found.

@taylorotwell
Copy link
Member

taylorotwell commented Mar 4, 2022

One potential problem I see here is the following situation:

Queue::fake(ChildJob::class);

ParentJob::dispatch()->onConnection('non-default-connection');

The ParentJob would be dispatched, but to the wrong connection. This is because QueueFake::connection just returns $this.

This may be solvable by checking the $job->connection property if the job is an object?

@yoeriboven
Copy link
Contributor Author

yoeriboven commented Mar 7, 2022

I'm not sure I understand what the problem is.

Don't we want to run ParentJob on the queue driver defined in phpunit.xml during testing? (usually sync) Else you'll have to have a queue worker running as well.

@taylorotwell
Copy link
Member

taylorotwell commented Mar 7, 2022

The queue driver in the phpunit.xml is just the default driver. Any queued jobs that are dispatched with a different, explicit connection would dispatch to the connection that is explicitly specified. It is possible applications are actually running a queue driver during some integration tests... just thinking of all the possible issues. 🤷‍♂️

@yoeriboven
Copy link
Contributor Author

I pushed some changes and I suppose this works now.

I'd recommend you to check it yourself since you know a lot more about the queueing system than I do.

@taylorotwell
Copy link
Member

taylorotwell commented Mar 7, 2022

OK - just thinking through this from the very beginning.

I would test your original example like so:

Queue::fake();
(new ParentJob)->handle();
Queue::assertPushed(ChildJob::class);

Is there another example you are trying to test that isn't currently already possible?

@yoeriboven
Copy link
Contributor Author

In that example you'd lose automatic dependency injection in the handle method. I guess you could do it in the constructor but you'll quickly get a lot of parameters in your constructor that way.

The example that made me attempt this PR is when using batches. When you want to test that a job is added to a batch.

class ParentJob
{
    use Batchable, Dispatchable, InteractsWithQueue, Queueable, SerializesModels;

    public function handle() 
    {
        $this->batch()->add(new ChildJob);
    }
{
Queue::fake();

Bus::batch(new ParentJob)->dispatch();

Queue::assertPushed(ChildJob::class);

You can't do (new ParentJob)->handle() because it is not part of a batch. When only faking ChildJob and executing ParentJob you can assert that it was pushed to the queue.

Thinking of it we could also add this:

Queue::fake(ChildJob::class);

$batch = Bus::batch(new ParentJob)->dispatch();

Queue::assertAddedToBatch($batch, ChildJob::class);

@taylorotwell
Copy link
Member

Added here: #41425

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