framework icon indicating copy to clipboard operation
framework copied to clipboard

BatchRepositoryFake doesn't function as a repository

Open Patabugen opened this issue 2 years ago • 1 comments

  • Laravel Version: 9.21.6
  • PHP Version: 8.0.20
  • Database Driver & Version: n/a (no database involved)

Description:

When using Bus::fake() to test a task which calls $batch->fresh() - fresh() returns null because the BatchRepositoryFake doesn't function as a repository.

I would expect it to return the batch object.

Fix

From looking at the source it seems like it's just not been written, rather than being a bug.

https://github.com/laravel/framework/blob/b9203fca96960ef9cd8860cb4ec99d1279353a8d/src/Illuminate/Support/Testing/Fakes/BatchRepositoryFake.php

I'd suggest it works like an Array repository, and I'd be happy to submit a PR if that sounds like the right thing for it to do?

Patabugen avatar Aug 10 '22 14:08 Patabugen

Locally I made an ArrayBatchRepository - but unless I'm missing something (often likely!) the BusFake() doesn't use the container to create a repository, so I wasn't able to use mocking to inject it.

My proposal would be make the Fake repository work as below (I've not done anything about the increment & decrement methods yet).

<?php

namespace Tests\Fixtures\Bus;

use Carbon\CarbonImmutable;
use Closure;
use Illuminate\Bus\Batch;
use Illuminate\Bus\BatchRepository;
use Illuminate\Bus\PendingBatch;
use Illuminate\Bus\UpdatedBatchJobCounts;
use Illuminate\Support\Facades\Facade;
use Illuminate\Support\Str;
use Illuminate\Support\Testing\Fakes\QueueFake;

class ArrayBatchRepository implements BatchRepository
{

    private array $batches = [];

    /**
     * Retrieve a list of batches.
     *
     * @param  int  $limit
     * @param  mixed  $before
     * @return \Illuminate\Bus\Batch[]
     */
    public function get($limit, $before)
    {
        return $this->batches;
    }

    /**
     * Retrieve information about an existing batch.
     *
     * @param  string  $batchId
     * @return \Illuminate\Bus\Batch|null
     */
    public function find(string $batchId)
    {
        return $this->batches[$batchId] ?? null;
    }

    /**
     * Store a new pending batch.
     *
     * @param  \Illuminate\Bus\PendingBatch  $batch
     * @return \Illuminate\Bus\Batch
     */
    public function store(PendingBatch $batch)
    {
        $id = (string) Str::orderedUuid();

        $this->batches[$id] = new Batch(
            new QueueFake(Facade::getFacadeApplication()),
            $this,
            $id,
            $batch->name,
            count($batch->jobs),
            count($batch->jobs),
            0,
            [],
            $batch->options,
            CarbonImmutable::now(),
            null,
            null
        );

        return $this->find($id);
    }

    /**
     * Increment the total number of jobs within the batch.
     *
     * @param  string  $batchId
     * @param  int  $amount
     * @return void
     */
    public function incrementTotalJobs(string $batchId, int $amount)
    {
    }

    /**
     * Decrement the total number of pending jobs for the batch.
     *
     * @param  string  $batchId
     * @param  string  $jobId
     * @return \Illuminate\Bus\UpdatedBatchJobCounts
     */
    public function decrementPendingJobs(string $batchId, string $jobId)
    {
        return new UpdatedBatchJobCounts;
    }

    /**
     * Increment the total number of failed jobs for the batch.
     *
     * @param  string  $batchId
     * @param  string  $jobId
     * @return \Illuminate\Bus\UpdatedBatchJobCounts
     */
    public function incrementFailedJobs(string $batchId, string $jobId)
    {
        return new UpdatedBatchJobCounts;
    }

    /**
     * Mark the batch that has the given ID as finished.
     *
     * @param  string  $batchId
     * @return void
     */
    public function markAsFinished(string $batchId)
    {
        optional($this->find($batchId))->finishedAt = CarbonImmutable::now();
    }

    /**
     * Cancel the batch that has the given ID.
     *
     * @param  string  $batchId
     * @return void
     */
    public function cancel(string $batchId)
    {
        optional($this->find($batchId))->cancel();
    }

    /**
     * Delete the batch that has the given ID.
     *
     * @param  string  $batchId
     * @return void
     */
    public function delete(string $batchId)
    {
        unset($this->batches[$batchId]);
    }

    /**
     * Execute the given Closure within a storage specific transaction.
     *
     * @param  \Closure  $callback
     * @return mixed
     */
    public function transaction(Closure $callback)
    {
        return $callback();
    }
}

Patabugen avatar Aug 10 '22 18:08 Patabugen

This seems like a feature request. You're welcome to attempt a PR if you like 👍

driesvints avatar Aug 11 '22 07:08 driesvints