Skip to content

Commit

Permalink
Fix QueueWorkTest cases asserting nothing (laravel#32958)
Browse files Browse the repository at this point in the history
Setting up PHPUnit's $this->expectException(),
calls to $worker->daemon() throw that exception
so the test case immediately stops there.
Subsquent assertions are being skipped.

Fixing this shows testJobRunsIfAppIsNotInMaintenanceMode
actually never runs any jobs on the queue.

When running the whole laravael/framework test suite,
Worker::memoryExceeded() can also return true from
previous test cases being run. So both of these
tests may skip running the queue, instead silently
calling exit(12) for no more memory.
  • Loading branch information
derekmd authored May 25, 2020
1 parent b3f2c40 commit 718a736
Showing 1 changed file with 24 additions and 14 deletions.
38 changes: 24 additions & 14 deletions tests/Queue/QueueWorkerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,21 @@ public function testWorkerCanWorkUntilQueueIsEmpty()
$secondJob = new WorkerFakeJob,
]]);

$this->expectException(LoopBreakerException::class);
try {
$worker->daemon('default', 'queue', $workerOptions);

$worker->daemon('default', 'queue', $workerOptions);
$this->fail('Expected LoopBreakerException to be thrown.');
} catch (LoopBreakerException $e) {
$this->assertTrue($firstJob->fired);

$this->assertTrue($firstJob->fired);
$this->assertTrue($secondJob->fired);

$this->assertTrue($secondJob->fired);
$this->assertSame(0, $worker->stoppedWithStatus);

$this->assertSame(0, $worker->stoppedWithStatus);
$this->events->shouldHaveReceived('dispatch')->with(m::type(JobProcessing::class))->twice();

$this->events->shouldHaveReceived('dispatch')->with(m::type(JobProcessing::class))->twice();

$this->events->shouldHaveReceived('dispatch')->with(m::type(JobProcessed::class))->twice();
$this->events->shouldHaveReceived('dispatch')->with(m::type(JobProcessed::class))->twice();
}
}

public function testJobCanBeFiredBasedOnPriority()
Expand Down Expand Up @@ -276,21 +278,23 @@ public function testJobRunsIfAppIsNotInMaintenanceMode()

$maintenanceModeChecker = function () {
if ($this->maintenanceFlags) {
return array_pop($this->maintenanceFlags);
return array_shift($this->maintenanceFlags);
}

throw new LoopBreakerException;
};

$this->expectException(LoopBreakerException::class);

$worker = $this->getWorker('default', ['queue' => [$firstJob, $secondJob]], $maintenanceModeChecker);

$worker->daemon('default', 'queue', $this->workerOptions());
try {
$worker->daemon('default', 'queue', $this->workerOptions());

$this->assertEquals($firstJob->attempts, 1);
$this->fail('Expected LoopBreakerException to be thrown');
} catch (LoopBreakerException $e) {
$this->assertSame(1, $firstJob->attempts);

$this->assertEquals($firstJob->attempts, 0);
$this->assertSame(0, $secondJob->attempts);
}
}

public function testJobDoesNotFireIfDeleted()
Expand Down Expand Up @@ -349,6 +353,7 @@ private function workerOptions(array $overrides = [])
class InsomniacWorker extends Worker
{
public $sleptFor;
public $stopOnMemoryExceeded = false;

public function sleep($seconds)
{
Expand All @@ -366,6 +371,11 @@ public function daemonShouldRun(WorkerOptions $options, $connectionName, $queue)
{
return ! ($this->isDownForMaintenance)();
}

public function memoryExceeded($memoryLimit)
{
return $this->stopOnMemoryExceeded;
}
}

class WorkerFakeManager extends QueueManager
Expand Down

0 comments on commit 718a736

Please sign in to comment.