From a15e9c29c8d5d5f2d2886ef80b329a218743050a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 22 Oct 2022 18:49:43 +0200 Subject: [PATCH 1/7] Migrated NotifyNewShortUrlToRabbitMqTest to use PHPUnit mocks --- .../NotifyNewShortUrlToRabbitMqTest.php | 101 +++++++++--------- 1 file changed, 49 insertions(+), 52 deletions(-) diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php index 477654bb..40ceb3aa 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php @@ -7,10 +7,8 @@ namespace ShlinkioTest\Shlink\Core\EventDispatcher\RabbitMq; use Doctrine\ORM\EntityManagerInterface; use DomainException; use Exception; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Prophecy\Argument; -use Prophecy\PhpUnit\ProphecyTrait; -use Prophecy\Prophecy\ObjectProphecy; use Psr\Log\LoggerInterface; use RuntimeException; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; @@ -25,48 +23,46 @@ use Throwable; class NotifyNewShortUrlToRabbitMqTest extends TestCase { - use ProphecyTrait; - - private ObjectProphecy $helper; - private ObjectProphecy $updatesGenerator; - private ObjectProphecy $em; - private ObjectProphecy $logger; + private MockObject $helper; + private MockObject $updatesGenerator; + private MockObject $em; + private MockObject $logger; protected function setUp(): void { - $this->helper = $this->prophesize(PublishingHelperInterface::class); - $this->updatesGenerator = $this->prophesize(PublishingUpdatesGeneratorInterface::class); - $this->em = $this->prophesize(EntityManagerInterface::class); - $this->logger = $this->prophesize(LoggerInterface::class); + $this->helper = $this->createMock(PublishingHelperInterface::class); + $this->updatesGenerator = $this->createMock(PublishingUpdatesGeneratorInterface::class); + $this->em = $this->createMock(EntityManagerInterface::class); + $this->logger = $this->createMock(LoggerInterface::class); } /** @test */ public function doesNothingWhenTheFeatureIsNotEnabled(): void { - ($this->listener(false))(new ShortUrlCreated('123')); + $this->helper->expects($this->never())->method('publishUpdate'); + $this->em->expects($this->never())->method('find'); + $this->logger->expects($this->never())->method('warning'); + $this->logger->expects($this->never())->method('debug'); - $this->em->find(Argument::cetera())->shouldNotHaveBeenCalled(); - $this->logger->warning(Argument::cetera())->shouldNotHaveBeenCalled(); - $this->logger->debug(Argument::cetera())->shouldNotHaveBeenCalled(); - $this->helper->publishUpdate(Argument::cetera())->shouldNotHaveBeenCalled(); + ($this->listener(false))(new ShortUrlCreated('123')); } /** @test */ public function notificationsAreNotSentWhenShortUrlCannotBeFound(): void { $shortUrlId = '123'; - $find = $this->em->find(ShortUrl::class, $shortUrlId)->willReturn(null); - $logWarning = $this->logger->warning( - 'Tried to notify {name} for new short URL with id "{shortUrlId}", but it does not exist.', - ['shortUrlId' => $shortUrlId, 'name' => 'RabbitMQ'], + $this->em->expects($this->once())->method('find')->with( + $this->equalTo(ShortUrl::class), + $this->equalTo($shortUrlId), + )->willReturn(null); + $this->logger->expects($this->once())->method('warning')->with( + $this->equalTo('Tried to notify {name} for new short URL with id "{shortUrlId}", but it does not exist.'), + $this->equalTo(['shortUrlId' => $shortUrlId, 'name' => 'RabbitMQ']), ); + $this->logger->expects($this->never())->method('debug'); + $this->helper->expects($this->never())->method('publishUpdate'); ($this->listener())(new ShortUrlCreated($shortUrlId)); - - $find->shouldHaveBeenCalledOnce(); - $logWarning->shouldHaveBeenCalledOnce(); - $this->logger->debug(Argument::cetera())->shouldNotHaveBeenCalled(); - $this->helper->publishUpdate(Argument::cetera())->shouldNotHaveBeenCalled(); } /** @test */ @@ -74,17 +70,17 @@ class NotifyNewShortUrlToRabbitMqTest extends TestCase { $shortUrlId = '123'; $update = Update::forTopicAndPayload(Topic::NEW_SHORT_URL->value, []); - $find = $this->em->find(ShortUrl::class, $shortUrlId)->willReturn(ShortUrl::withLongUrl('')); - $generateUpdate = $this->updatesGenerator->newShortUrlUpdate(Argument::type(ShortUrl::class))->willReturn( - $update, - ); + $this->em->expects($this->once())->method('find')->with( + $this->equalTo(ShortUrl::class), + $this->equalTo($shortUrlId), + )->willReturn(ShortUrl::withLongUrl('')); + $this->updatesGenerator->expects($this->once())->method('newShortUrlUpdate')->with( + $this->isInstanceOf(ShortUrl::class), + )->willReturn($update); + $this->helper->expects($this->once())->method('publishUpdate')->with($this->equalTo($update)); + $this->logger->expects($this->never())->method('debug'); ($this->listener())(new ShortUrlCreated($shortUrlId)); - - $find->shouldHaveBeenCalledOnce(); - $generateUpdate->shouldHaveBeenCalledOnce(); - $this->helper->publishUpdate($update)->shouldHaveBeenCalledOnce(); - $this->logger->debug(Argument::cetera())->shouldNotHaveBeenCalled(); } /** @@ -95,21 +91,22 @@ class NotifyNewShortUrlToRabbitMqTest extends TestCase { $shortUrlId = '123'; $update = Update::forTopicAndPayload(Topic::NEW_SHORT_URL->value, []); - $find = $this->em->find(ShortUrl::class, $shortUrlId)->willReturn(ShortUrl::withLongUrl('')); - $generateUpdate = $this->updatesGenerator->newShortUrlUpdate(Argument::type(ShortUrl::class))->willReturn( - $update, + $this->em->expects($this->once())->method('find')->with( + $this->equalTo(ShortUrl::class), + $this->equalTo($shortUrlId), + )->willReturn(ShortUrl::withLongUrl('')); + $this->updatesGenerator->expects($this->once())->method('newShortUrlUpdate')->with( + $this->isInstanceOf(ShortUrl::class), + )->willReturn($update); + $this->helper->expects($this->once())->method('publishUpdate')->with( + $this->equalTo($update), + )->willThrowException($e); + $this->logger->expects($this->once())->method('debug')->with( + $this->equalTo('Error while trying to notify {name} with new short URL. {e}'), + $this->equalTo(['e' => $e, 'name' => 'RabbitMQ']), ); - $publish = $this->helper->publishUpdate($update)->willThrow($e); ($this->listener())(new ShortUrlCreated($shortUrlId)); - - $this->logger->debug( - 'Error while trying to notify {name} with new short URL. {e}', - ['e' => $e, 'name' => 'RabbitMQ'], - )->shouldHaveBeenCalledOnce(); - $find->shouldHaveBeenCalledOnce(); - $generateUpdate->shouldHaveBeenCalledOnce(); - $publish->shouldHaveBeenCalledOnce(); } public function provideExceptions(): iterable @@ -122,10 +119,10 @@ class NotifyNewShortUrlToRabbitMqTest extends TestCase private function listener(bool $enabled = true): NotifyNewShortUrlToRabbitMq { return new NotifyNewShortUrlToRabbitMq( - $this->helper->reveal(), - $this->updatesGenerator->reveal(), - $this->em->reveal(), - $this->logger->reveal(), + $this->helper, + $this->updatesGenerator, + $this->em, + $this->logger, new RabbitMqOptions($enabled), ); } From 739433ba8b4a34e38e69caaaefe836470ecc9f7e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 22 Oct 2022 19:05:34 +0200 Subject: [PATCH 2/7] Migrated NotifyVisitToRabbitMqTest to use PHPUnit mocks --- .../RabbitMq/NotifyVisitToRabbitMqTest.php | 158 +++++++++--------- 1 file changed, 77 insertions(+), 81 deletions(-) diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php index aef04cdf..94688dd3 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php @@ -8,10 +8,8 @@ use Doctrine\ORM\EntityManagerInterface; use DomainException; use Exception; use PHPUnit\Framework\Assert; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Prophecy\Argument; -use Prophecy\PhpUnit\ProphecyTrait; -use Prophecy\Prophecy\ObjectProphecy; use Psr\Log\LoggerInterface; use RuntimeException; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; @@ -33,48 +31,46 @@ use function Functional\noop; class NotifyVisitToRabbitMqTest extends TestCase { - use ProphecyTrait; - - private ObjectProphecy $helper; - private ObjectProphecy $updatesGenerator; - private ObjectProphecy $em; - private ObjectProphecy $logger; + private MockObject $helper; + private MockObject $updatesGenerator; + private MockObject $em; + private MockObject $logger; protected function setUp(): void { - $this->helper = $this->prophesize(PublishingHelperInterface::class); - $this->updatesGenerator = $this->prophesize(PublishingUpdatesGeneratorInterface::class); - $this->em = $this->prophesize(EntityManagerInterface::class); - $this->logger = $this->prophesize(LoggerInterface::class); + $this->helper = $this->createMock(PublishingHelperInterface::class); + $this->updatesGenerator = $this->createMock(PublishingUpdatesGeneratorInterface::class); + $this->em = $this->createMock(EntityManagerInterface::class); + $this->logger = $this->createMock(LoggerInterface::class); } /** @test */ public function doesNothingWhenTheFeatureIsNotEnabled(): void { - ($this->listener(new RabbitMqOptions(enabled: false)))(new VisitLocated('123')); + $this->helper->expects($this->never())->method('publishUpdate'); + $this->em->expects($this->never())->method('find'); + $this->logger->expects($this->never())->method('warning'); + $this->logger->expects($this->never())->method('debug'); - $this->em->find(Argument::cetera())->shouldNotHaveBeenCalled(); - $this->logger->warning(Argument::cetera())->shouldNotHaveBeenCalled(); - $this->logger->debug(Argument::cetera())->shouldNotHaveBeenCalled(); - $this->helper->publishUpdate(Argument::cetera())->shouldNotHaveBeenCalled(); + ($this->listener(new RabbitMqOptions(enabled: false)))(new VisitLocated('123')); } /** @test */ public function notificationsAreNotSentWhenVisitCannotBeFound(): void { $visitId = '123'; - $findVisit = $this->em->find(Visit::class, $visitId)->willReturn(null); - $logWarning = $this->logger->warning( - 'Tried to notify {name} for visit with id "{visitId}", but it does not exist.', - ['visitId' => $visitId, 'name' => 'RabbitMQ'], + $this->em->expects($this->once())->method('find')->with( + $this->equalTo(Visit::class), + $this->equalTo($visitId), + )->willReturn(null); + $this->logger->expects($this->once())->method('warning')->with( + $this->equalTo('Tried to notify {name} for visit with id "{visitId}", but it does not exist.'), + $this->equalTo(['visitId' => $visitId, 'name' => 'RabbitMQ']), ); + $this->logger->expects($this->never())->method('debug'); + $this->helper->expects($this->never())->method('publishUpdate'); ($this->listener())(new VisitLocated($visitId)); - - $findVisit->shouldHaveBeenCalledOnce(); - $logWarning->shouldHaveBeenCalledOnce(); - $this->logger->debug(Argument::cetera())->shouldNotHaveBeenCalled(); - $this->helper->publishUpdate(Argument::cetera())->shouldNotHaveBeenCalled(); } /** @@ -84,20 +80,21 @@ class NotifyVisitToRabbitMqTest extends TestCase public function expectedChannelsAreNotifiedBasedOnTheVisitType(Visit $visit, array $expectedChannels): void { $visitId = '123'; - $findVisit = $this->em->find(Visit::class, $visitId)->willReturn($visit); + $this->em->expects($this->once())->method('find')->with( + $this->equalTo(Visit::class), + $this->equalTo($visitId), + )->willReturn($visit); each($expectedChannels, function (string $method): void { - $this->updatesGenerator->{$method}(Argument::type(Visit::class))->willReturn( - Update::forTopicAndPayload('', []), - )->shouldBeCalledOnce(); + $this->updatesGenerator->expects($this->once())->method($method)->with( + $this->isInstanceOf(Visit::class), + )->willReturn(Update::forTopicAndPayload('', [])); }); + $this->helper->expects($this->exactly(count($expectedChannels)))->method('publishUpdate')->with( + $this->isInstanceOf(Update::class), + ); + $this->logger->expects($this->never())->method('debug'); ($this->listener())(new VisitLocated($visitId)); - - $findVisit->shouldHaveBeenCalledOnce(); - $this->helper->publishUpdate(Argument::type(Update::class))->shouldHaveBeenCalledTimes( - count($expectedChannels), - ); - $this->logger->debug(Argument::cetera())->shouldNotHaveBeenCalled(); } public function provideVisits(): iterable @@ -124,21 +121,20 @@ class NotifyVisitToRabbitMqTest extends TestCase public function printsDebugMessageInCaseOfError(Throwable $e): void { $visitId = '123'; - $findVisit = $this->em->find(Visit::class, $visitId)->willReturn(Visit::forBasePath(Visitor::emptyInstance())); - $generateUpdate = $this->updatesGenerator->newOrphanVisitUpdate(Argument::type(Visit::class))->willReturn( - Update::forTopicAndPayload('', []), + $this->em->expects($this->once())->method('find')->with( + $this->equalTo(Visit::class), + $this->equalTo($visitId), + )->willReturn(Visit::forBasePath(Visitor::emptyInstance())); + $this->updatesGenerator->expects($this->once())->method('newOrphanVisitUpdate')->with( + $this->isInstanceOf(Visit::class), + )->willReturn(Update::forTopicAndPayload('', [])); + $this->helper->expects($this->once())->method('publishUpdate')->withAnyParameters()->willThrowException($e); + $this->logger->expects($this->once())->method('debug')->with( + $this->equalTo('Error while trying to notify {name} with new visit. {e}'), + $this->equalTo(['e' => $e, 'name' => 'RabbitMQ']), ); - $publish = $this->helper->publishUpdate(Argument::cetera())->willThrow($e); ($this->listener())(new VisitLocated($visitId)); - - $this->logger->debug( - 'Error while trying to notify {name} with new visit. {e}', - ['e' => $e, 'name' => 'RabbitMQ'], - )->shouldHaveBeenCalledOnce(); - $findVisit->shouldHaveBeenCalledOnce(); - $generateUpdate->shouldHaveBeenCalledOnce(); - $publish->shouldHaveBeenCalledOnce(); } public function provideExceptions(): iterable @@ -155,17 +151,18 @@ class NotifyVisitToRabbitMqTest extends TestCase public function expectedPayloadIsPublishedDependingOnConfig( bool $legacy, Visit $visit, - callable $assert, callable $setup, + callable $expect, ): void { $visitId = '123'; - $findVisit = $this->em->find(Visit::class, $visitId)->willReturn($visit); + $this->em->expects($this->once())->method('find')->with( + $this->equalTo(Visit::class), + $this->equalTo($visitId), + )->willReturn($visit); $setup($this->updatesGenerator); + $expect($this->helper, $this->updatesGenerator); ($this->listener(new RabbitMqOptions(true, $legacy)))(new VisitLocated($visitId)); - - $findVisit->shouldHaveBeenCalledOnce(); - $assert($this->helper, $this->updatesGenerator); } public function provideLegacyPayloads(): iterable @@ -173,8 +170,9 @@ class NotifyVisitToRabbitMqTest extends TestCase yield 'legacy non-orphan visit' => [ true, $visit = Visit::forValidShortUrl(ShortUrl::withLongUrl(''), Visitor::emptyInstance()), - function (ObjectProphecy|PublishingHelperInterface $helper) use ($visit): void { - $helper->publishUpdate(Argument::that(function (Update $update) use ($visit): bool { + noop(...), + function (MockObject & PublishingHelperInterface $helper) use ($visit): void { + $helper->method('publishUpdate')->with($this->callback(function (Update $update) use ($visit): bool { $payload = $update->payload; Assert::assertEquals($payload, $visit->jsonSerialize()); Assert::assertArrayNotHasKey('visitedUrl', $payload); @@ -185,13 +183,13 @@ class NotifyVisitToRabbitMqTest extends TestCase return true; })); }, - noop(...), ]; yield 'legacy orphan visit' => [ true, Visit::forBasePath(Visitor::emptyInstance()), - function (ObjectProphecy|PublishingHelperInterface $helper): void { - $helper->publishUpdate(Argument::that(function (Update $update): bool { + noop(...), + function (MockObject & PublishingHelperInterface $helper): void { + $helper->method('publishUpdate')->with($this->callback(function (Update $update): bool { $payload = $update->payload; Assert::assertArrayHasKey('visitedUrl', $payload); Assert::assertArrayHasKey('type', $payload); @@ -199,35 +197,33 @@ class NotifyVisitToRabbitMqTest extends TestCase return true; })); }, - noop(...), ]; yield 'non-legacy non-orphan visit' => [ false, Visit::forValidShortUrl(ShortUrl::withLongUrl(''), Visitor::emptyInstance()), - function (ObjectProphecy|PublishingHelperInterface $helper): void { - $helper->publishUpdate(Argument::type(Update::class))->shouldHaveBeenCalledTimes(2); - }, - function (ObjectProphecy|PublishingUpdatesGeneratorInterface $updatesGenerator): void { + function (MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator): void { $update = Update::forTopicAndPayload('', []); - $updatesGenerator->newOrphanVisitUpdate(Argument::cetera())->shouldNotBeCalled(); - $updatesGenerator->newVisitUpdate(Argument::cetera())->willReturn($update) - ->shouldBeCalledOnce(); - $updatesGenerator->newShortUrlVisitUpdate(Argument::cetera())->willReturn($update) - ->shouldBeCalledOnce(); + $updatesGenerator->expects($this->never())->method('newOrphanVisitUpdate'); + $updatesGenerator->expects($this->once())->method('newVisitUpdate')->withAnyParameters()->willReturn( + $update, + ); + $updatesGenerator->expects($this->once())->method('newShortUrlVisitUpdate')->willReturn($update); + }, + function (MockObject & PublishingHelperInterface $helper): void { + $helper->expects($this->exactly(2))->method('publishUpdate')->with($this->isInstanceOf(Update::class)); }, ]; yield 'non-legacy orphan visit' => [ false, Visit::forBasePath(Visitor::emptyInstance()), - function (ObjectProphecy|PublishingHelperInterface $helper): void { - $helper->publishUpdate(Argument::type(Update::class))->shouldHaveBeenCalledOnce(); - }, - function (ObjectProphecy|PublishingUpdatesGeneratorInterface $updatesGenerator): void { + function (MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator): void { $update = Update::forTopicAndPayload('', []); - $updatesGenerator->newOrphanVisitUpdate(Argument::cetera())->willReturn($update) - ->shouldBeCalledOnce(); - $updatesGenerator->newVisitUpdate(Argument::cetera())->shouldNotBeCalled(); - $updatesGenerator->newShortUrlVisitUpdate(Argument::cetera())->shouldNotBeCalled(); + $updatesGenerator->expects($this->once())->method('newOrphanVisitUpdate')->willReturn($update); + $updatesGenerator->expects($this->never())->method('newVisitUpdate'); + $updatesGenerator->expects($this->never())->method('newShortUrlVisitUpdate'); + }, + function (MockObject & PublishingHelperInterface $helper): void { + $helper->expects($this->once())->method('publishUpdate')->with($this->isInstanceOf(Update::class)); }, ]; } @@ -235,10 +231,10 @@ class NotifyVisitToRabbitMqTest extends TestCase private function listener(?RabbitMqOptions $options = null): NotifyVisitToRabbitMq { return new NotifyVisitToRabbitMq( - $this->helper->reveal(), - $this->updatesGenerator->reveal(), - $this->em->reveal(), - $this->logger->reveal(), + $this->helper, + $this->updatesGenerator, + $this->em, + $this->logger, new OrphanVisitDataTransformer(), $options ?? new RabbitMqOptions(enabled: true, legacyVisitsPublishing: false), ); From d0393799d2d7a1bf530a3b6d543f876240a3ba1d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 22 Oct 2022 19:59:32 +0200 Subject: [PATCH 3/7] Migrated NotifyNewShortUrlToRedisTest to use PHPUnit mocks --- .../NotifyNewShortUrlToRedisTest.php | 65 ++++++++----------- 1 file changed, 28 insertions(+), 37 deletions(-) diff --git a/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php b/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php index ec3338d1..fd0e3904 100644 --- a/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php +++ b/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php @@ -7,10 +7,8 @@ namespace ShlinkioTest\Shlink\Core\EventDispatcher\RedisPubSub; use Doctrine\ORM\EntityManagerInterface; use DomainException; use Exception; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Prophecy\Argument; -use Prophecy\PhpUnit\ProphecyTrait; -use Prophecy\Prophecy\ObjectProphecy; use Psr\Log\LoggerInterface; use RuntimeException; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; @@ -24,30 +22,28 @@ use Throwable; class NotifyNewShortUrlToRedisTest extends TestCase { - use ProphecyTrait; - - private ObjectProphecy $helper; - private ObjectProphecy $updatesGenerator; - private ObjectProphecy $em; - private ObjectProphecy $logger; + private MockObject $helper; + private MockObject $updatesGenerator; + private MockObject $em; + private MockObject $logger; protected function setUp(): void { - $this->helper = $this->prophesize(PublishingHelperInterface::class); - $this->updatesGenerator = $this->prophesize(PublishingUpdatesGeneratorInterface::class); - $this->em = $this->prophesize(EntityManagerInterface::class); - $this->logger = $this->prophesize(LoggerInterface::class); + $this->helper = $this->createMock(PublishingHelperInterface::class); + $this->updatesGenerator = $this->createMock(PublishingUpdatesGeneratorInterface::class); + $this->em = $this->createMock(EntityManagerInterface::class); + $this->logger = $this->createMock(LoggerInterface::class); } /** @test */ public function doesNothingWhenTheFeatureIsNotEnabled(): void { - $this->createListener(false)(new ShortUrlCreated('123')); + $this->helper->expects($this->never())->method('publishUpdate'); + $this->em->expects($this->never())->method('find'); + $this->logger->expects($this->never())->method('warning'); + $this->logger->expects($this->never())->method('debug'); - $this->em->find(Argument::cetera())->shouldNotHaveBeenCalled(); - $this->logger->warning(Argument::cetera())->shouldNotHaveBeenCalled(); - $this->logger->debug(Argument::cetera())->shouldNotHaveBeenCalled(); - $this->helper->publishUpdate(Argument::cetera())->shouldNotHaveBeenCalled(); + $this->createListener(false)(new ShortUrlCreated('123')); } /** @@ -58,21 +54,22 @@ class NotifyNewShortUrlToRedisTest extends TestCase { $shortUrlId = '123'; $update = Update::forTopicAndPayload(Topic::NEW_SHORT_URL->value, []); - $find = $this->em->find(ShortUrl::class, $shortUrlId)->willReturn(ShortUrl::withLongUrl('')); - $generateUpdate = $this->updatesGenerator->newShortUrlUpdate(Argument::type(ShortUrl::class))->willReturn( - $update, + $this->em->expects($this->once())->method('find')->with( + $this->equalTo(ShortUrl::class), + $this->equalTo($shortUrlId), + )->willReturn(ShortUrl::withLongUrl('')); + $this->updatesGenerator->expects($this->once())->method('newShortUrlUpdate')->with( + $this->isInstanceOf(ShortUrl::class), + )->willReturn($update); + $this->helper->expects($this->once())->method('publishUpdate')->with( + $this->equalTo($update), + )->willThrowException($e); + $this->logger->expects($this->once())->method('debug')->with( + $this->equalTo('Error while trying to notify {name} with new short URL. {e}'), + $this->equalTo(['e' => $e, 'name' => 'Redis pub/sub']), ); - $publish = $this->helper->publishUpdate($update)->willThrow($e); $this->createListener()(new ShortUrlCreated($shortUrlId)); - - $this->logger->debug( - 'Error while trying to notify {name} with new short URL. {e}', - ['e' => $e, 'name' => 'Redis pub/sub'], - )->shouldHaveBeenCalledOnce(); - $find->shouldHaveBeenCalledOnce(); - $generateUpdate->shouldHaveBeenCalledOnce(); - $publish->shouldHaveBeenCalledOnce(); } public function provideExceptions(): iterable @@ -84,12 +81,6 @@ class NotifyNewShortUrlToRedisTest extends TestCase private function createListener(bool $enabled = true): NotifyNewShortUrlToRedis { - return new NotifyNewShortUrlToRedis( - $this->helper->reveal(), - $this->updatesGenerator->reveal(), - $this->em->reveal(), - $this->logger->reveal(), - $enabled, - ); + return new NotifyNewShortUrlToRedis($this->helper, $this->updatesGenerator, $this->em, $this->logger, $enabled); } } From 1706a869d9fc4a66a8549f6e820a105760cb0121 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 22 Oct 2022 20:04:12 +0200 Subject: [PATCH 4/7] Migrated NotifyVisitToRedisTest to use PHPUnit mocks --- .../RedisPubSub/NotifyVisitToRedisTest.php | 63 ++++++++----------- 1 file changed, 26 insertions(+), 37 deletions(-) diff --git a/module/Core/test/EventDispatcher/RedisPubSub/NotifyVisitToRedisTest.php b/module/Core/test/EventDispatcher/RedisPubSub/NotifyVisitToRedisTest.php index 5c1e797b..c3188e5d 100644 --- a/module/Core/test/EventDispatcher/RedisPubSub/NotifyVisitToRedisTest.php +++ b/module/Core/test/EventDispatcher/RedisPubSub/NotifyVisitToRedisTest.php @@ -7,10 +7,8 @@ namespace ShlinkioTest\Shlink\Core\EventDispatcher\RedisPubSub; use Doctrine\ORM\EntityManagerInterface; use DomainException; use Exception; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Prophecy\Argument; -use Prophecy\PhpUnit\ProphecyTrait; -use Prophecy\Prophecy\ObjectProphecy; use Psr\Log\LoggerInterface; use RuntimeException; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; @@ -24,30 +22,28 @@ use Throwable; class NotifyVisitToRedisTest extends TestCase { - use ProphecyTrait; - - private ObjectProphecy $helper; - private ObjectProphecy $updatesGenerator; - private ObjectProphecy $em; - private ObjectProphecy $logger; + private MockObject $helper; + private MockObject $updatesGenerator; + private MockObject $em; + private MockObject $logger; protected function setUp(): void { - $this->helper = $this->prophesize(PublishingHelperInterface::class); - $this->updatesGenerator = $this->prophesize(PublishingUpdatesGeneratorInterface::class); - $this->em = $this->prophesize(EntityManagerInterface::class); - $this->logger = $this->prophesize(LoggerInterface::class); + $this->helper = $this->createMock(PublishingHelperInterface::class); + $this->updatesGenerator = $this->createMock(PublishingUpdatesGeneratorInterface::class); + $this->em = $this->createMock(EntityManagerInterface::class); + $this->logger = $this->createMock(LoggerInterface::class); } /** @test */ public function doesNothingWhenTheFeatureIsNotEnabled(): void { - $this->createListener(false)(new VisitLocated('123')); + $this->helper->expects($this->never())->method('publishUpdate'); + $this->em->expects($this->never())->method('find'); + $this->logger->expects($this->never())->method('warning'); + $this->logger->expects($this->never())->method('debug'); - $this->em->find(Argument::cetera())->shouldNotHaveBeenCalled(); - $this->logger->warning(Argument::cetera())->shouldNotHaveBeenCalled(); - $this->logger->debug(Argument::cetera())->shouldNotHaveBeenCalled(); - $this->helper->publishUpdate(Argument::cetera())->shouldNotHaveBeenCalled(); + $this->createListener(false)(new VisitLocated('123')); } /** @@ -57,21 +53,20 @@ class NotifyVisitToRedisTest extends TestCase public function printsDebugMessageInCaseOfError(Throwable $e): void { $visitId = '123'; - $findVisit = $this->em->find(Visit::class, $visitId)->willReturn(Visit::forBasePath(Visitor::emptyInstance())); - $generateUpdate = $this->updatesGenerator->newOrphanVisitUpdate(Argument::type(Visit::class))->willReturn( - Update::forTopicAndPayload('', []), + $this->em->expects($this->once())->method('find')->with( + $this->equalTo(Visit::class), + $this->equalTo($visitId), + )->willReturn(Visit::forBasePath(Visitor::emptyInstance())); + $this->updatesGenerator->expects($this->once())->method('newOrphanVisitUpdate')->with( + $this->isInstanceOf(Visit::class), + )->willReturn(Update::forTopicAndPayload('', [])); + $this->helper->expects($this->once())->method('publishUpdate')->withAnyParameters()->willThrowException($e); + $this->logger->expects($this->once())->method('debug')->with( + $this->equalTo('Error while trying to notify {name} with new visit. {e}'), + $this->equalTo(['e' => $e, 'name' => 'Redis pub/sub']), ); - $publish = $this->helper->publishUpdate(Argument::cetera())->willThrow($e); $this->createListener()(new VisitLocated($visitId)); - - $this->logger->debug( - 'Error while trying to notify {name} with new visit. {e}', - ['e' => $e, 'name' => 'Redis pub/sub'], - )->shouldHaveBeenCalledOnce(); - $findVisit->shouldHaveBeenCalledOnce(); - $generateUpdate->shouldHaveBeenCalledOnce(); - $publish->shouldHaveBeenCalledOnce(); } public function provideExceptions(): iterable @@ -83,12 +78,6 @@ class NotifyVisitToRedisTest extends TestCase private function createListener(bool $enabled = true): NotifyVisitToRedis { - return new NotifyVisitToRedis( - $this->helper->reveal(), - $this->updatesGenerator->reveal(), - $this->em->reveal(), - $this->logger->reveal(), - $enabled, - ); + return new NotifyVisitToRedis($this->helper, $this->updatesGenerator, $this->em, $this->logger, $enabled); } } From 10b0ec301b5b6d98a37c33b2a143f3585a9b2518 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 22 Oct 2022 20:05:06 +0200 Subject: [PATCH 5/7] Migrated ValidationExceptionTest to use PHPUnit mocks --- module/Core/test/Exception/ValidationExceptionTest.php | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/module/Core/test/Exception/ValidationExceptionTest.php b/module/Core/test/Exception/ValidationExceptionTest.php index a0980738..b34badf3 100644 --- a/module/Core/test/Exception/ValidationExceptionTest.php +++ b/module/Core/test/Exception/ValidationExceptionTest.php @@ -8,7 +8,6 @@ use Fig\Http\Message\StatusCodeInterface; use Laminas\InputFilter\InputFilterInterface; use LogicException; use PHPUnit\Framework\TestCase; -use Prophecy\PhpUnit\ProphecyTrait; use RuntimeException; use Shlinkio\Shlink\Core\Exception\ValidationException; use Throwable; @@ -18,8 +17,6 @@ use function print_r; class ValidationExceptionTest extends TestCase { - use ProphecyTrait; - /** * @test * @dataProvider provideExceptions @@ -36,10 +33,10 @@ class ValidationExceptionTest extends TestCase 'something' => {$barValue} EOT; - $inputFilter = $this->prophesize(InputFilterInterface::class); - $getMessages = $inputFilter->getMessages()->willReturn($invalidData); + $inputFilter = $this->createMock(InputFilterInterface::class); + $inputFilter->expects($this->once())->method('getMessages')->with()->willReturn($invalidData); - $e = ValidationException::fromInputFilter($inputFilter->reveal(), $prev); + $e = ValidationException::fromInputFilter($inputFilter, $prev); self::assertEquals($invalidData, $e->getInvalidElements()); self::assertEquals(['invalidElements' => array_keys($invalidData)], $e->getAdditionalData()); @@ -47,7 +44,6 @@ class ValidationExceptionTest extends TestCase self::assertEquals(StatusCodeInterface::STATUS_BAD_REQUEST, $e->getCode()); self::assertEquals($prev, $e->getPrevious()); self::assertStringContainsString($expectedStringRepresentation, (string) $e); - $getMessages->shouldHaveBeenCalledOnce(); } public function provideExceptions(): iterable From 173420c608b9efb0f22bcf8eb0d431265784d8cc Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 22 Oct 2022 20:39:55 +0200 Subject: [PATCH 6/7] Migrated ImportedLinksProcessorTest to use PHPUnit mocks --- .../Importer/ImportedLinksProcessorTest.php | 171 +++++++++--------- 1 file changed, 81 insertions(+), 90 deletions(-) diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php index 2ce93647..926901a9 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -7,10 +7,8 @@ namespace ShlinkioTest\Shlink\Core\Importer; use Cake\Chronos\Chronos; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Prophecy\Argument; -use Prophecy\PhpUnit\ProphecyTrait; -use Prophecy\Prophecy\ObjectProphecy; use RuntimeException; use Shlinkio\Shlink\Core\Importer\ImportedLinksProcessor; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; @@ -23,6 +21,7 @@ use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Importer\Model\ImportedShlinkVisit; use Shlinkio\Shlink\Importer\Params\ImportParams; use Shlinkio\Shlink\Importer\Sources\ImportSource; +use stdClass; use Symfony\Component\Console\Style\StyleInterface; use function count; @@ -32,32 +31,30 @@ use function str_contains; class ImportedLinksProcessorTest extends TestCase { - use ProphecyTrait; - private ImportedLinksProcessor $processor; - private ObjectProphecy $em; - private ObjectProphecy $shortCodeHelper; - private ObjectProphecy $repo; - private ObjectProphecy $io; + private MockObject $em; + private MockObject $shortCodeHelper; + private MockObject $repo; + private MockObject $io; protected function setUp(): void { - $this->em = $this->prophesize(EntityManagerInterface::class); - $this->repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $this->em->getRepository(ShortUrl::class)->willReturn($this->repo->reveal()); + $this->em = $this->createMock(EntityManagerInterface::class); + $this->repo = $this->createMock(ShortUrlRepositoryInterface::class); + $this->em->method('getRepository')->with($this->equalTo(ShortUrl::class))->willReturn($this->repo); - $this->shortCodeHelper = $this->prophesize(ShortCodeUniquenessHelperInterface::class); - $batchHelper = $this->prophesize(DoctrineBatchHelperInterface::class); - $batchHelper->wrapIterable(Argument::cetera())->willReturnArgument(0); + $this->shortCodeHelper = $this->createMock(ShortCodeUniquenessHelperInterface::class); + $batchHelper = $this->createMock(DoctrineBatchHelperInterface::class); + $batchHelper->method('wrapIterable')->willReturnArgument(0); $this->processor = new ImportedLinksProcessor( - $this->em->reveal(), + $this->em, new SimpleShortUrlRelationResolver(), - $this->shortCodeHelper->reveal(), - $batchHelper->reveal(), + $this->shortCodeHelper, + $batchHelper, ); - $this->io = $this->prophesize(StyleInterface::class); + $this->io = $this->createMock(StyleInterface::class); } /** @test */ @@ -70,16 +67,16 @@ class ImportedLinksProcessorTest extends TestCase ]; $expectedCalls = count($urls); - $importedUrlExists = $this->repo->findOneByImportedUrl(Argument::cetera())->willReturn(null); - $ensureUniqueness = $this->shortCodeHelper->ensureShortCodeUniqueness(Argument::cetera())->willReturn(true); - $persist = $this->em->persist(Argument::type(ShortUrl::class)); + $this->repo->expects($this->exactly($expectedCalls))->method('findOneByImportedUrl')->willReturn(null); + $this->shortCodeHelper->expects($this->exactly($expectedCalls)) + ->method('ensureShortCodeUniqueness') + ->willReturn(true); + $this->em->expects($this->exactly($expectedCalls))->method('persist')->with( + $this->isInstanceOf(ShortUrl::class), + ); + $this->io->expects($this->exactly($expectedCalls))->method('text')->with($this->isType('string')); - $this->processor->process($this->io->reveal(), $urls, $this->buildParams()); - - $importedUrlExists->shouldHaveBeenCalledTimes($expectedCalls); - $ensureUniqueness->shouldHaveBeenCalledTimes($expectedCalls); - $persist->shouldHaveBeenCalledTimes($expectedCalls); - $this->io->text(Argument::type('string'))->shouldHaveBeenCalledTimes($expectedCalls); + $this->processor->process($this->io, $urls, $this->buildParams()); } /** @test */ @@ -91,26 +88,21 @@ class ImportedLinksProcessorTest extends TestCase new ImportedShlinkUrl(ImportSource::BITLY, 'baz', [], Chronos::now(), null, 'baz', null), ]; - $importedUrlExists = $this->repo->findOneByImportedUrl(Argument::cetera())->willReturn(null); - $ensureUniqueness = $this->shortCodeHelper->ensureShortCodeUniqueness(Argument::cetera())->willReturn(true); - $persist = $this->em->persist(Argument::type(ShortUrl::class))->will(function (array $args): void { - /** @var ShortUrl $shortUrl */ - [$shortUrl] = $args; - + $this->repo->expects($this->exactly(3))->method('findOneByImportedUrl')->willReturn(null); + $this->shortCodeHelper->expects($this->exactly(3))->method('ensureShortCodeUniqueness')->willReturn(true); + $this->em->expects($this->exactly(3))->method('persist')->with( + $this->isInstanceOf(ShortUrl::class), + )->willReturnCallback(function (ShortUrl $shortUrl): void { if ($shortUrl->getShortCode() === 'baz') { throw new RuntimeException('Whatever error'); } }); + $textCalls = $this->setUpIoText('Skipped. Reason: Whatever error', 'Imported'); - $this->processor->process($this->io->reveal(), $urls, $this->buildParams()); + $this->processor->process($this->io, $urls, $this->buildParams()); - $importedUrlExists->shouldHaveBeenCalledTimes(3); - $ensureUniqueness->shouldHaveBeenCalledTimes(3); - $persist->shouldHaveBeenCalledTimes(3); - $this->io->text(Argument::containingString('Imported'))->shouldHaveBeenCalledTimes(2); - $this->io->text( - Argument::containingString('Skipped. Reason: Whatever error'), - )->shouldHaveBeenCalledOnce(); + self::assertEquals(2, $textCalls->importedCount); + self::assertEquals(1, $textCalls->skippedCount); } /** @test */ @@ -124,24 +116,18 @@ class ImportedLinksProcessorTest extends TestCase new ImportedShlinkUrl(ImportSource::BITLY, 'baz3', [], Chronos::now(), null, 'baz3', null), ]; - $importedUrlExists = $this->repo->findOneByImportedUrl(Argument::cetera())->will( - function (array $args): ?ShortUrl { - /** @var ImportedShlinkUrl $url */ - [$url] = $args; - - return contains(['foo', 'baz2', 'baz3'], $url->longUrl) ? ShortUrl::fromImport($url, true) : null; - }, + $this->repo->expects($this->exactly(count($urls)))->method('findOneByImportedUrl')->willReturnCallback( + fn (ImportedShlinkUrl $url): ?ShortUrl + => contains(['foo', 'baz2', 'baz3'], $url->longUrl) ? ShortUrl::fromImport($url, true) : null, ); - $ensureUniqueness = $this->shortCodeHelper->ensureShortCodeUniqueness(Argument::cetera())->willReturn(true); - $persist = $this->em->persist(Argument::type(ShortUrl::class)); + $this->shortCodeHelper->expects($this->exactly(2))->method('ensureShortCodeUniqueness')->willReturn(true); + $this->em->expects($this->exactly(2))->method('persist')->with($this->isInstanceOf(ShortUrl::class)); + $textCalls = $this->setUpIoText(); - $this->processor->process($this->io->reveal(), $urls, $this->buildParams()); + $this->processor->process($this->io, $urls, $this->buildParams()); - $importedUrlExists->shouldHaveBeenCalledTimes(count($urls)); - $ensureUniqueness->shouldHaveBeenCalledTimes(2); - $persist->shouldHaveBeenCalledTimes(2); - $this->io->text(Argument::containingString('Skipped'))->shouldHaveBeenCalledTimes(3); - $this->io->text(Argument::containingString('Imported'))->shouldHaveBeenCalledTimes(2); + self::assertEquals(2, $textCalls->importedCount); + self::assertEquals(3, $textCalls->skippedCount); } /** @test */ @@ -155,32 +141,20 @@ class ImportedLinksProcessorTest extends TestCase new ImportedShlinkUrl(ImportSource::BITLY, 'baz3', [], Chronos::now(), null, 'baz3', 'bar'), ]; - $importedUrlExists = $this->repo->findOneByImportedUrl(Argument::cetera())->willReturn(null); - $failingEnsureUniqueness = $this->shortCodeHelper->ensureShortCodeUniqueness( - Argument::any(), - true, - )->willReturn(false); - $successEnsureUniqueness = $this->shortCodeHelper->ensureShortCodeUniqueness( - Argument::any(), - false, - )->willReturn(true); - $choice = $this->io->choice(Argument::cetera())->will(function (array $args) { - /** @var ImportedShlinkUrl $url */ - [$question] = $args; - + $this->repo->expects($this->exactly(count($urls)))->method('findOneByImportedUrl')->willReturn(null); + $this->shortCodeHelper->expects($this->exactly(7))->method('ensureShortCodeUniqueness')->willReturnCallback( + fn ($_, bool $hasCustomSlug) => ! $hasCustomSlug, + ); + $this->em->expects($this->exactly(2))->method('persist')->with($this->isInstanceOf(ShortUrl::class)); + $this->io->expects($this->exactly(5))->method('choice')->willReturnCallback(function (string $question) { return some(['foo', 'baz2', 'baz3'], fn (string $item) => str_contains($question, $item)) ? 'Skip' : ''; }); - $persist = $this->em->persist(Argument::type(ShortUrl::class)); + $textCalls = $this->setUpIoText('Error'); - $this->processor->process($this->io->reveal(), $urls, $this->buildParams()); + $this->processor->process($this->io, $urls, $this->buildParams()); - $importedUrlExists->shouldHaveBeenCalledTimes(count($urls)); - $failingEnsureUniqueness->shouldHaveBeenCalledTimes(5); - $successEnsureUniqueness->shouldHaveBeenCalledTimes(2); - $choice->shouldHaveBeenCalledTimes(5); - $persist->shouldHaveBeenCalledTimes(2); - $this->io->text(Argument::containingString('Error'))->shouldHaveBeenCalledTimes(3); - $this->io->text(Argument::containingString('Imported'))->shouldHaveBeenCalledTimes(2); + self::assertEquals(2, $textCalls->importedCount); + self::assertEquals(3, $textCalls->skippedCount); } /** @@ -193,18 +167,16 @@ class ImportedLinksProcessorTest extends TestCase int $amountOfPersistedVisits, ?ShortUrl $foundShortUrl, ): void { - $findExisting = $this->repo->findOneByImportedUrl(Argument::cetera())->willReturn($foundShortUrl); - $ensureUniqueness = $this->shortCodeHelper->ensureShortCodeUniqueness(Argument::cetera())->willReturn(true); - $persistUrl = $this->em->persist(Argument::type(ShortUrl::class)); - $persistVisits = $this->em->persist(Argument::type(Visit::class)); + $this->repo->expects($this->once())->method('findOneByImportedUrl')->willReturn($foundShortUrl); + $this->shortCodeHelper->expects($this->exactly($foundShortUrl === null ? 1 : 0)) + ->method('ensureShortCodeUniqueness') + ->willReturn(true); + $this->em->expects($this->exactly($amountOfPersistedVisits + ($foundShortUrl === null ? 1 : 0)))->method( + 'persist', + )->with($this->callback(fn (object $arg) => $arg instanceof ShortUrl || $arg instanceof Visit)); + $this->io->expects($this->once())->method('text')->with($this->stringContains($expectedOutput)); - $this->processor->process($this->io->reveal(), [$importedUrl], $this->buildParams()); - - $findExisting->shouldHaveBeenCalledOnce(); - $ensureUniqueness->shouldHaveBeenCalledTimes($foundShortUrl === null ? 1 : 0); - $persistUrl->shouldHaveBeenCalledTimes($foundShortUrl === null ? 1 : 0); - $persistVisits->shouldHaveBeenCalledTimes($amountOfPersistedVisits); - $this->io->text(Argument::containingString($expectedOutput))->shouldHaveBeenCalledOnce(); + $this->processor->process($this->io, [$importedUrl], $this->buildParams()); } public function provideUrlsWithVisits(): iterable @@ -251,4 +223,23 @@ class ImportedLinksProcessorTest extends TestCase { return ImportSource::BITLY->toParamsWithCallableMap(['import_short_codes' => static fn () => true]); } + + public function setUpIoText(string $skippedText = 'Skipped', string $importedText = 'Imported'): stdClass + { + $counts = new stdClass(); + $counts->importedCount = 0; + $counts->skippedCount = 0; + + $this->io->method('text')->willReturnCallback( + function (string $output) use ($counts, $skippedText, $importedText) { + if (str_contains($output, $skippedText)) { + $counts->skippedCount++; + } elseif (str_contains($output, $importedText)) { + $counts->importedCount++; + } + }, + ); + + return $counts; + } } From a78c59c11a78e47ffb3b70a6a86e50a7a406298c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 22 Oct 2022 20:41:17 +0200 Subject: [PATCH 7/7] Fixed coding styles --- module/Core/test/Importer/ImportedLinksProcessorTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php index 926901a9..183c54c1 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -231,7 +231,7 @@ class ImportedLinksProcessorTest extends TestCase $counts->skippedCount = 0; $this->io->method('text')->willReturnCallback( - function (string $output) use ($counts, $skippedText, $importedText) { + function (string $output) use ($counts, $skippedText, $importedText): void { if (str_contains($output, $skippedText)) { $counts->skippedCount++; } elseif (str_contains($output, $importedText)) {