From 2696f9d75f16d50c33d503615781a570c7d027d3 Mon Sep 17 00:00:00 2001 From: WarLikeLaux Date: Fri, 5 Jun 2026 21:15:48 +0600 Subject: [PATCH 1/3] Test uncovered code paths to reach 100% line coverage --- .../Consume/ConsumeMiddlewareFactory.php | 4 ++ .../FailureMiddlewareFactory.php | 4 ++ src/Middleware/MiddlewareFactory.php | 4 ++ src/Middleware/Push/PushMiddlewareFactory.php | 4 ++ tests/Unit/Command/ListenAllCommandTest.php | 22 ++++++++++ tests/Unit/Debug/QueueDecoratorTest.php | 44 +++++++++++++++++++ tests/Unit/Message/EnvelopeTest.php | 11 +++++ .../FailureHandling/FailureEnvelopeTest.php | 10 +++++ .../SendAgainMiddlewareTest.php | 17 +++++++ .../Middleware/Push/MiddlewareFactoryTest.php | 1 + .../Provider/QueueFactoryProviderTest.php | 14 ++++++ tests/Unit/WorkerTest.php | 34 ++++++++++++++ 12 files changed, 169 insertions(+) diff --git a/src/Middleware/Consume/ConsumeMiddlewareFactory.php b/src/Middleware/Consume/ConsumeMiddlewareFactory.php index 66d31798..48349a79 100644 --- a/src/Middleware/Consume/ConsumeMiddlewareFactory.php +++ b/src/Middleware/Consume/ConsumeMiddlewareFactory.php @@ -47,7 +47,11 @@ public function createConsumeMiddleware( $middleware = $this->create($middlewareDefinition); if (!$middleware instanceof ConsumeMiddlewareInterface) { + // self::create() always returns an instance of the required interface or throws, + // so this is unreachable at runtime and kept only for the static analyzer. + // @codeCoverageIgnoreStart throw new InvalidMiddlewareDefinitionException($middlewareDefinition); + // @codeCoverageIgnoreEnd } return $middleware; diff --git a/src/Middleware/FailureHandling/FailureMiddlewareFactory.php b/src/Middleware/FailureHandling/FailureMiddlewareFactory.php index 501131e7..809eb728 100644 --- a/src/Middleware/FailureHandling/FailureMiddlewareFactory.php +++ b/src/Middleware/FailureHandling/FailureMiddlewareFactory.php @@ -51,7 +51,11 @@ public function createFailureMiddleware( $middleware = $this->create($middlewareDefinition); if (!$middleware instanceof FailureMiddlewareInterface) { + // self::create() always returns an instance of the required interface or throws, + // so this is unreachable at runtime and kept only for the static analyzer. + // @codeCoverageIgnoreStart throw new InvalidMiddlewareDefinitionException($middlewareDefinition); + // @codeCoverageIgnoreEnd } return $middleware; diff --git a/src/Middleware/MiddlewareFactory.php b/src/Middleware/MiddlewareFactory.php index 5204a04a..328f8852 100644 --- a/src/Middleware/MiddlewareFactory.php +++ b/src/Middleware/MiddlewareFactory.php @@ -48,7 +48,11 @@ protected function create(callable|array|string $definition): object ?? throw new InvalidMiddlewareDefinitionException($definition); } + // A non-string, non-array value of type callable|array|string is always callable and handled above, + // so this is unreachable at runtime and kept only for completeness. + // @codeCoverageIgnoreStart throw new InvalidMiddlewareDefinitionException($definition); + // @codeCoverageIgnoreEnd } /** diff --git a/src/Middleware/Push/PushMiddlewareFactory.php b/src/Middleware/Push/PushMiddlewareFactory.php index 4326e749..260f9cfd 100644 --- a/src/Middleware/Push/PushMiddlewareFactory.php +++ b/src/Middleware/Push/PushMiddlewareFactory.php @@ -55,7 +55,11 @@ public function createPushMiddleware(mixed $middlewareDefinition): PushMiddlewar $middleware = $this->create($middlewareDefinition); if (!$middleware instanceof PushMiddlewareInterface) { + // self::create() always returns an instance of the required interface or throws, + // so this is unreachable at runtime and kept only for the static analyzer. + // @codeCoverageIgnoreStart throw new InvalidMiddlewareDefinitionException($middlewareDefinition); + // @codeCoverageIgnoreEnd } return $middleware; diff --git a/tests/Unit/Command/ListenAllCommandTest.php b/tests/Unit/Command/ListenAllCommandTest.php index 24175f58..22657fa7 100644 --- a/tests/Unit/Command/ListenAllCommandTest.php +++ b/tests/Unit/Command/ListenAllCommandTest.php @@ -39,4 +39,26 @@ public function testExecute(): void $this->assertEquals(0, $exitCode); } + + public function testNegativePauseIsNormalizedToOne(): void + { + $queue = $this->createMock(QueueInterface::class); + $queue->method('run')->willReturn(1); + + $queueProvider = new PredefinedQueueProvider([ + 'queue1' => $queue, + ]); + + $loop = $this->createMock(LoopInterface::class); + $loop->method('canContinue')->willReturn(true, false); + + $command = new ListenAllCommand( + $queueProvider, + $loop, + ); + $input = new ArrayInput(['--pause' => -1], $command->getNativeDefinition()); + $exitCode = $command->run($input, $this->createMock(OutputInterface::class)); + + $this->assertSame(0, $exitCode); + } } diff --git a/tests/Unit/Debug/QueueDecoratorTest.php b/tests/Unit/Debug/QueueDecoratorTest.php index 9d0169c6..d378bbcc 100644 --- a/tests/Unit/Debug/QueueDecoratorTest.php +++ b/tests/Unit/Debug/QueueDecoratorTest.php @@ -80,4 +80,48 @@ public function testGetName(): void $this->assertEquals('hello', $decorator->getName()); } + + public function testWithMiddlewares(): void + { + $newQueue = $this->createMock(QueueInterface::class); + $newQueue->method('getName')->willReturn('new'); + $queue = $this->createMock(QueueInterface::class); + $queue->expects($this->once()) + ->method('withMiddlewares') + ->with('m1', 'm2') + ->willReturn($newQueue); + $collector = new QueueCollector(); + $decorator = new QueueDecorator( + $queue, + $collector, + ); + + $result = $decorator->withMiddlewares('m1', 'm2'); + + $this->assertInstanceOf(QueueDecorator::class, $result); + $this->assertNotSame($decorator, $result); + $this->assertSame('new', $result->getName()); + } + + public function testWithMiddlewaresAdded(): void + { + $newQueue = $this->createMock(QueueInterface::class); + $newQueue->method('getName')->willReturn('new'); + $queue = $this->createMock(QueueInterface::class); + $queue->expects($this->once()) + ->method('withMiddlewaresAdded') + ->with('m1', 'm2') + ->willReturn($newQueue); + $collector = new QueueCollector(); + $decorator = new QueueDecorator( + $queue, + $collector, + ); + + $result = $decorator->withMiddlewaresAdded('m1', 'm2'); + + $this->assertInstanceOf(QueueDecorator::class, $result); + $this->assertNotSame($decorator, $result); + $this->assertSame('new', $result->getName()); + } } diff --git a/tests/Unit/Message/EnvelopeTest.php b/tests/Unit/Message/EnvelopeTest.php index a27b8bf8..d43dcb1c 100644 --- a/tests/Unit/Message/EnvelopeTest.php +++ b/tests/Unit/Message/EnvelopeTest.php @@ -6,6 +6,7 @@ use LogicException; use PHPUnit\Framework\TestCase; +use Yiisoft\Queue\Message\GenericMessage; use Yiisoft\Queue\Tests\App\DummyEnvelope; final class EnvelopeTest extends TestCase @@ -18,4 +19,14 @@ public function testFromData(): void ); DummyEnvelope::fromData('test', []); } + + public function testWithMetadata(): void + { + $envelope = DummyEnvelope::fromMessage(new GenericMessage('test', 'data')); + + $result = $envelope->withMetadata(['key' => 'value']); + + $this->assertInstanceOf(DummyEnvelope::class, $result); + $this->assertSame(['key' => 'value'], $result->getMetadata()); + } } diff --git a/tests/Unit/Middleware/FailureHandling/FailureEnvelopeTest.php b/tests/Unit/Middleware/FailureHandling/FailureEnvelopeTest.php index 32e2edbb..c39cbf36 100644 --- a/tests/Unit/Middleware/FailureHandling/FailureEnvelopeTest.php +++ b/tests/Unit/Middleware/FailureHandling/FailureEnvelopeTest.php @@ -23,6 +23,16 @@ public function testConstructor(): void $this->assertSame($metadata, $envelope->getMetadata()[FailureEnvelope::META_FAILURE]); } + public function testGetFailureMetadata(): void + { + $message = $this->createMessage(); + $metadata = ['attempt' => 1, 'error' => 'Test error']; + + $envelope = new FailureEnvelope($message, $metadata); + + $this->assertSame($metadata, $envelope->getFailureMetadata()); + } + public function testFromMessageWithExistingMetadata(): void { $existingMetadata = ['attempt' => 1]; diff --git a/tests/Unit/Middleware/FailureHandling/Implementation/SendAgainMiddlewareTest.php b/tests/Unit/Middleware/FailureHandling/Implementation/SendAgainMiddlewareTest.php index 594b6982..ef1b816e 100644 --- a/tests/Unit/Middleware/FailureHandling/Implementation/SendAgainMiddlewareTest.php +++ b/tests/Unit/Middleware/FailureHandling/Implementation/SendAgainMiddlewareTest.php @@ -5,6 +5,7 @@ namespace Yiisoft\Queue\Tests\Unit\Middleware\FailureHandling\Implementation; use Exception; +use InvalidArgumentException; use PHPUnit\Framework\Assert; use PHPUnit\Framework\Attributes\DataProvider; use RuntimeException; @@ -167,6 +168,22 @@ public function testQueueSendingStrategies( self::assertInstanceOf(FailureHandlingRequest::class, $result); } + public static function nonPositiveMaxAttemptsProvider(): array + { + return [ + [0, 'maxAttempts parameter must be a positive integer, 0 given.'], + [-5, 'maxAttempts parameter must be a positive integer, -5 given.'], + ]; + } + + #[DataProvider('nonPositiveMaxAttemptsProvider')] + public function testThrowsOnNonPositiveMaxAttempts(int $maxAttempts, string $expectedMessage): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage($expectedMessage); + new SendAgainMiddleware('', $maxAttempts); + } + private function getStrategy(string $strategyName, QueueInterface $queue): FailureMiddlewareInterface { return match ($strategyName) { diff --git a/tests/Unit/Middleware/Push/MiddlewareFactoryTest.php b/tests/Unit/Middleware/Push/MiddlewareFactoryTest.php index 80f99b73..f0254d84 100644 --- a/tests/Unit/Middleware/Push/MiddlewareFactoryTest.php +++ b/tests/Unit/Middleware/Push/MiddlewareFactoryTest.php @@ -147,6 +147,7 @@ public static function invalidMiddlewareDefinitionProvider(): array 'wrong array with int items' => [[7, 42]], 'array with wrong method name' => [[TestCallableMiddleware::class, 'notExists']], 'array wrong class' => [['class' => TestCallableMiddleware::class]], + 'non-callable scalar' => [42], ]; } diff --git a/tests/Unit/Provider/QueueFactoryProviderTest.php b/tests/Unit/Provider/QueueFactoryProviderTest.php index 3e9aadcf..3453242a 100644 --- a/tests/Unit/Provider/QueueFactoryProviderTest.php +++ b/tests/Unit/Provider/QueueFactoryProviderTest.php @@ -99,6 +99,20 @@ public function testInvalidQueueConfigOnGet(): void $provider->get('queue1'); } + public function testInvalidQueueConfigThrownOnCreate(): void + { + $provider = new QueueFactoryProvider( + [ + 'queue1' => 42, + ], + validate: false, + ); + + $this->expectException(InvalidQueueConfigException::class); + $this->expectExceptionMessage('Invalid definition: 42'); + $provider->get('queue1'); + } + public function testGetHasByStringEnum(): void { $provider = new QueueFactoryProvider( diff --git a/tests/Unit/WorkerTest.php b/tests/Unit/WorkerTest.php index c6c2cb52..e56f9e1c 100644 --- a/tests/Unit/WorkerTest.php +++ b/tests/Unit/WorkerTest.php @@ -14,6 +14,7 @@ use Yiisoft\Test\Support\Log\SimpleLogger; use Yiisoft\Queue\Exception\MessageFailureException; use Yiisoft\Queue\Message\GenericMessage; +use Yiisoft\Queue\Message\IdEnvelope; use Yiisoft\Queue\Message\MessageInterface; use Yiisoft\Queue\Middleware\Consume\ConsumeMiddlewareDispatcher; use Yiisoft\Queue\Middleware\Consume\ConsumeMiddlewareFactoryInterface; @@ -86,6 +87,39 @@ function (MessageInterface $message) { ]; } + public function testProcessLogsMessageId(): void + { + $message = new IdEnvelope(new GenericMessage('simple', ['test-data']), 'message-id-123'); + $logger = new SimpleLogger(); + $container = new SimpleContainer([FakeHandler::class => new FakeHandler()]); + $handlers = ['simple' => FakeHandler::class]; + + /** @var MockObject&QueueInterface $queue */ + $queue = $this->createMock(QueueInterface::class); + $worker = $this->createWorkerByParams($handlers, $container, $logger); + + $worker->process($message, $queue); + FakeHandler::$processedMessages = []; + + $messages = $logger->getMessages(); + $this->assertSame('Processing message #message-id-123.', $messages[0]['message']); + } + + public function testProcessFailsForEmptyMessageType(): void + { + $message = new GenericMessage('', ['test-data']); + $container = new SimpleContainer(); + $handlers = []; + + /** @var MockObject&QueueInterface $queue */ + $queue = $this->createMock(QueueInterface::class); + $worker = $this->createWorkerByParams($handlers, $container); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Queue handler for message type "" does not exist.'); + $worker->process($message, $queue); + } + public function testMessageFailWithDefinitionUndefinedMethodHandler(): void { $this->expectExceptionMessage('Queue handler for message type "simple" does not exist'); From b6e742b5f07af8b197c4dc8b3b689c57375e14fc Mon Sep 17 00:00:00 2001 From: WarLikeLaux Date: Fri, 5 Jun 2026 21:32:05 +0600 Subject: [PATCH 2/3] Rename `ListenAllCommand` pause test to match its assertion --- tests/Unit/Command/ListenAllCommandTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/Command/ListenAllCommandTest.php b/tests/Unit/Command/ListenAllCommandTest.php index 22657fa7..b6f2a89e 100644 --- a/tests/Unit/Command/ListenAllCommandTest.php +++ b/tests/Unit/Command/ListenAllCommandTest.php @@ -40,7 +40,7 @@ public function testExecute(): void $this->assertEquals(0, $exitCode); } - public function testNegativePauseIsNormalizedToOne(): void + public function testRunsWithNegativePause(): void { $queue = $this->createMock(QueueInterface::class); $queue->method('run')->willReturn(1); From 88dfcb3c8d8d28411d9e8fd30329ee08ebaa80f3 Mon Sep 17 00:00:00 2001 From: WarLikeLaux Date: Mon, 8 Jun 2026 08:25:48 +0600 Subject: [PATCH 3/3] Drop unreachable branches in middleware factories --- .../Consume/ConsumeMiddlewareFactory.php | 8 ------ .../FailureMiddlewareFactory.php | 8 ------ src/Middleware/MiddlewareFactory.php | 26 +++++++------------ src/Middleware/Push/PushMiddlewareFactory.php | 8 ------ tests/Unit/QueueTest.php | 13 ++++++++++ 5 files changed, 23 insertions(+), 40 deletions(-) diff --git a/src/Middleware/Consume/ConsumeMiddlewareFactory.php b/src/Middleware/Consume/ConsumeMiddlewareFactory.php index 48349a79..698e7fb1 100644 --- a/src/Middleware/Consume/ConsumeMiddlewareFactory.php +++ b/src/Middleware/Consume/ConsumeMiddlewareFactory.php @@ -46,14 +46,6 @@ public function createConsumeMiddleware( $middleware = $this->create($middlewareDefinition); - if (!$middleware instanceof ConsumeMiddlewareInterface) { - // self::create() always returns an instance of the required interface or throws, - // so this is unreachable at runtime and kept only for the static analyzer. - // @codeCoverageIgnoreStart - throw new InvalidMiddlewareDefinitionException($middlewareDefinition); - // @codeCoverageIgnoreEnd - } - return $middleware; } diff --git a/src/Middleware/FailureHandling/FailureMiddlewareFactory.php b/src/Middleware/FailureHandling/FailureMiddlewareFactory.php index 809eb728..b699d8b4 100644 --- a/src/Middleware/FailureHandling/FailureMiddlewareFactory.php +++ b/src/Middleware/FailureHandling/FailureMiddlewareFactory.php @@ -50,14 +50,6 @@ public function createFailureMiddleware( $middleware = $this->create($middlewareDefinition); - if (!$middleware instanceof FailureMiddlewareInterface) { - // self::create() always returns an instance of the required interface or throws, - // so this is unreachable at runtime and kept only for the static analyzer. - // @codeCoverageIgnoreStart - throw new InvalidMiddlewareDefinitionException($middlewareDefinition); - // @codeCoverageIgnoreEnd - } - return $middleware; } diff --git a/src/Middleware/MiddlewareFactory.php b/src/Middleware/MiddlewareFactory.php index 328f8852..20510734 100644 --- a/src/Middleware/MiddlewareFactory.php +++ b/src/Middleware/MiddlewareFactory.php @@ -31,28 +31,22 @@ public function __construct( */ protected function create(callable|array|string $definition): object { - try { - $callable = $this->callableFactory->create($definition); - - return $this->wrapMiddleware($callable); - } catch (InvalidCallableConfigurationException) { - // Not a callable, try internal checks - } + if (is_string($definition) || is_array($definition)) { + try { + return $this->wrapMiddleware($this->callableFactory->create($definition)); + } catch (InvalidCallableConfigurationException) { + // Not a callable, try internal checks + } - if (is_string($definition)) { - return $this->getFromContainer($definition); - } + if (is_string($definition)) { + return $this->getFromContainer($definition); + } - if (is_array($definition)) { return $this->tryGetFromArrayDefinition($definition) ?? throw new InvalidMiddlewareDefinitionException($definition); } - // A non-string, non-array value of type callable|array|string is always callable and handled above, - // so this is unreachable at runtime and kept only for completeness. - // @codeCoverageIgnoreStart - throw new InvalidMiddlewareDefinitionException($definition); - // @codeCoverageIgnoreEnd + return $this->wrapMiddleware($this->callableFactory->create($definition)); } /** diff --git a/src/Middleware/Push/PushMiddlewareFactory.php b/src/Middleware/Push/PushMiddlewareFactory.php index 260f9cfd..b42269a2 100644 --- a/src/Middleware/Push/PushMiddlewareFactory.php +++ b/src/Middleware/Push/PushMiddlewareFactory.php @@ -54,14 +54,6 @@ public function createPushMiddleware(mixed $middlewareDefinition): PushMiddlewar $middleware = $this->create($middlewareDefinition); - if (!$middleware instanceof PushMiddlewareInterface) { - // self::create() always returns an instance of the required interface or throws, - // so this is unreachable at runtime and kept only for the static analyzer. - // @codeCoverageIgnoreStart - throw new InvalidMiddlewareDefinitionException($middlewareDefinition); - // @codeCoverageIgnoreEnd - } - return $middleware; } diff --git a/tests/Unit/QueueTest.php b/tests/Unit/QueueTest.php index 4cccc86e..aa0279c9 100644 --- a/tests/Unit/QueueTest.php +++ b/tests/Unit/QueueTest.php @@ -4,6 +4,7 @@ namespace Yiisoft\Queue\Tests\Unit; +use Yiisoft\Queue\Adapter\AdapterInterface; use Yiisoft\Queue\Cli\SignalLoop; use Yiisoft\Queue\Message\IdEnvelope; use Yiisoft\Queue\Message\GenericMessage; @@ -32,6 +33,18 @@ public function testPushSuccessful(): void self::assertSame([$message], $adapter->getMessagesList()); } + public function testPushLogsWhenIdNotAssigned(): void + { + $message = new GenericMessage('simple', null); + $adapter = $this->createMock(AdapterInterface::class); + $adapter->method('push')->willReturn($message); + $queue = $this->createQueue($adapter); + + $result = $queue->push($message); + + self::assertNull(IdEnvelope::fromMessage($result)->getId()); + } + public function testPushSynchronouslyProcessesMessage(): void { $queue = $this->createQueue();