From 843754b7e7cdfe6df7cebe6f7f4f24768dec58f1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 21 Oct 2022 18:32:34 +0200 Subject: [PATCH 01/10] Migrated PixelActionTest to use PHPUnit mocks --- module/Core/test/Action/PixelActionTest.php | 27 +++++++++------------ 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/module/Core/test/Action/PixelActionTest.php b/module/Core/test/Action/PixelActionTest.php index 3d301945..7a150970 100644 --- a/module/Core/test/Action/PixelActionTest.php +++ b/module/Core/test/Action/PixelActionTest.php @@ -5,10 +5,8 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Action; use Laminas\Diactoros\ServerRequest; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Prophecy\Argument; -use Prophecy\PhpUnit\ProphecyTrait; -use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Common\Response\PixelResponse; use Shlinkio\Shlink\Core\Action\PixelAction; @@ -19,32 +17,29 @@ use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; class PixelActionTest extends TestCase { - use ProphecyTrait; - private PixelAction $action; - private ObjectProphecy $urlResolver; - private ObjectProphecy $requestTracker; + private MockObject $urlResolver; + private MockObject $requestTracker; protected function setUp(): void { - $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); - $this->requestTracker = $this->prophesize(RequestTrackerInterface::class); + $this->urlResolver = $this->createMock(ShortUrlResolverInterface::class); + $this->requestTracker = $this->createMock(RequestTrackerInterface::class); - $this->action = new PixelAction($this->urlResolver->reveal(), $this->requestTracker->reveal()); + $this->action = new PixelAction($this->urlResolver, $this->requestTracker); } /** @test */ public function imageIsReturned(): void { $shortCode = 'abc123'; - $this->urlResolver->resolveEnabledShortUrl( - ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, ''), - )->willReturn(ShortUrl::withLongUrl('http://domain.com/foo/bar')) - ->shouldBeCalledOnce(); - $this->requestTracker->trackIfApplicable(Argument::cetera())->shouldBeCalledOnce(); + $this->urlResolver->expects($this->once())->method('resolveEnabledShortUrl')->with( + $this->equalTo(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, '')), + )->willReturn(ShortUrl::withLongUrl('http://domain.com/foo/bar')); + $this->requestTracker->expects($this->once())->method('trackIfApplicable')->withAnyParameters(); $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); - $response = $this->action->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal()); + $response = $this->action->process($request, $this->createMock(RequestHandlerInterface::class)); self::assertInstanceOf(PixelResponse::class, $response); self::assertEquals(200, $response->getStatusCode()); From cd4b632d753b58f0103c00e4e51b0c418ffa7c63 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 21 Oct 2022 18:39:22 +0200 Subject: [PATCH 02/10] Migrated QrActionTest to use PHPUnit mocks --- module/Core/test/Action/QrCodeActionTest.php | 71 +++++++++----------- 1 file changed, 31 insertions(+), 40 deletions(-) diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index 90635a3c..8bec70e4 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -7,10 +7,8 @@ namespace ShlinkioTest\Shlink\Core\Action; use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequest; use Laminas\Diactoros\ServerRequestFactory; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Prophecy\Argument; -use Prophecy\PhpUnit\ProphecyTrait; -use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\NullLogger; @@ -29,50 +27,43 @@ use function imagecreatefromstring; class QrCodeActionTest extends TestCase { - use ProphecyTrait; - private const WHITE = 0xFFFFFF; private const BLACK = 0x0; - private ObjectProphecy $urlResolver; + private MockObject $urlResolver; protected function setUp(): void { - $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); + $this->urlResolver = $this->createMock(ShortUrlResolverInterface::class); } /** @test */ public function aNotFoundShortCodeWillDelegateIntoNextMiddleware(): void { $shortCode = 'abc123'; - $this->urlResolver->resolveEnabledShortUrl(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, '')) - ->willThrow(ShortUrlNotFoundException::class) - ->shouldBeCalledOnce(); - $delegate = $this->prophesize(RequestHandlerInterface::class); - $process = $delegate->handle(Argument::any())->willReturn(new Response()); + $this->urlResolver->expects($this->once())->method('resolveEnabledShortUrl')->with( + $this->equalTo(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, '')) + )->willThrowException(ShortUrlNotFoundException::fromNotFound(ShortUrlIdentifier::fromShortCodeAndDomain(''))); + $delegate = $this->createMock(RequestHandlerInterface::class); + $delegate->expects($this->once())->method('handle')->withAnyParameters()->willReturn(new Response()); - $this->action()->process((new ServerRequest())->withAttribute('shortCode', $shortCode), $delegate->reveal()); - - $process->shouldHaveBeenCalledOnce(); + $this->action()->process((new ServerRequest())->withAttribute('shortCode', $shortCode), $delegate); } /** @test */ public function aCorrectRequestReturnsTheQrCodeResponse(): void { $shortCode = 'abc123'; - $this->urlResolver->resolveEnabledShortUrl(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, '')) - ->willReturn(ShortUrl::createEmpty()) - ->shouldBeCalledOnce(); - $delegate = $this->prophesize(RequestHandlerInterface::class); + $this->urlResolver->expects($this->once())->method('resolveEnabledShortUrl')->with( + $this->equalTo(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, '')), + )->willReturn(ShortUrl::createEmpty()); + $delegate = $this->createMock(RequestHandlerInterface::class); + $delegate->expects($this->never())->method('handle'); - $resp = $this->action()->process( - (new ServerRequest())->withAttribute('shortCode', $shortCode), - $delegate->reveal(), - ); + $resp = $this->action()->process((new ServerRequest())->withAttribute('shortCode', $shortCode), $delegate); self::assertInstanceOf(QrCodeResponse::class, $resp); self::assertEquals(200, $resp->getStatusCode()); - $delegate->handle(Argument::any())->shouldHaveBeenCalledTimes(0); } /** @@ -85,13 +76,13 @@ class QrCodeActionTest extends TestCase string $expectedContentType, ): void { $code = 'abc123'; - $this->urlResolver->resolveEnabledShortUrl(ShortUrlIdentifier::fromShortCodeAndDomain($code, ''))->willReturn( - ShortUrl::createEmpty(), - ); - $delegate = $this->prophesize(RequestHandlerInterface::class); + $this->urlResolver->method('resolveEnabledShortUrl')->with( + $this->equalTo(ShortUrlIdentifier::fromShortCodeAndDomain($code, '')), + )->willReturn(ShortUrl::createEmpty()); + $delegate = $this->createMock(RequestHandlerInterface::class); $req = (new ServerRequest())->withAttribute('shortCode', $code)->withQueryParams($query); - $resp = $this->action(new QrCodeOptions(format: $defaultFormat))->process($req, $delegate->reveal()); + $resp = $this->action(new QrCodeOptions(format: $defaultFormat))->process($req, $delegate); self::assertEquals($expectedContentType, $resp->getHeaderLine('Content-Type')); } @@ -118,12 +109,12 @@ class QrCodeActionTest extends TestCase int $expectedSize, ): void { $code = 'abc123'; - $this->urlResolver->resolveEnabledShortUrl(ShortUrlIdentifier::fromShortCodeAndDomain($code, ''))->willReturn( - ShortUrl::createEmpty(), - ); - $delegate = $this->prophesize(RequestHandlerInterface::class); + $this->urlResolver->method('resolveEnabledShortUrl')->with( + $this->equalTo(ShortUrlIdentifier::fromShortCodeAndDomain($code, '')), + )->willReturn(ShortUrl::createEmpty()); + $delegate = $this->createMock(RequestHandlerInterface::class); - $resp = $this->action($defaultOptions)->process($req->withAttribute('shortCode', $code), $delegate->reveal()); + $resp = $this->action($defaultOptions)->process($req->withAttribute('shortCode', $code), $delegate); [$size] = getimagesizefromstring($resp->getBody()->__toString()); self::assertEquals($expectedSize, $size); @@ -209,12 +200,12 @@ class QrCodeActionTest extends TestCase ->withQueryParams(['size' => 250, 'roundBlockSize' => $roundBlockSize]) ->withAttribute('shortCode', $code); - $this->urlResolver->resolveEnabledShortUrl(ShortUrlIdentifier::fromShortCodeAndDomain($code, ''))->willReturn( - ShortUrl::withLongUrl('https://shlink.io'), - ); - $delegate = $this->prophesize(RequestHandlerInterface::class); + $this->urlResolver->method('resolveEnabledShortUrl')->with( + $this->equalTo(ShortUrlIdentifier::fromShortCodeAndDomain($code, '')), + )->willReturn(ShortUrl::withLongUrl('https://shlink.io')); + $delegate = $this->createMock(RequestHandlerInterface::class); - $resp = $this->action($defaultOptions)->process($req, $delegate->reveal()); + $resp = $this->action($defaultOptions)->process($req, $delegate); $image = imagecreatefromstring($resp->getBody()->__toString()); $color = imagecolorat($image, 1, 1); @@ -246,7 +237,7 @@ class QrCodeActionTest extends TestCase public function action(?QrCodeOptions $options = null): QrCodeAction { return new QrCodeAction( - $this->urlResolver->reveal(), + $this->urlResolver, new ShortUrlStringifier(['domain' => 'doma.in']), new NullLogger(), $options ?? new QrCodeOptions(), From a8f82971311a240d8f1d50ed93f99a156a028b9b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 21 Oct 2022 18:44:55 +0200 Subject: [PATCH 03/10] Migrated RedirectActionTest to use PHPUnit mocks --- .../Core/test/Action/RedirectActionTest.php | 62 ++++++++----------- 1 file changed, 27 insertions(+), 35 deletions(-) diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index f7449ad9..f17d32ba 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -6,10 +6,8 @@ namespace ShlinkioTest\Shlink\Core\Action; use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequest; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Prophecy\Argument; -use Prophecy\PhpUnit\ProphecyTrait; -use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; @@ -22,29 +20,27 @@ use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; class RedirectActionTest extends TestCase { - use ProphecyTrait; - private const LONG_URL = 'https://domain.com/foo/bar?some=thing'; private RedirectAction $action; - private ObjectProphecy $urlResolver; - private ObjectProphecy $requestTracker; - private ObjectProphecy $redirectRespHelper; + private MockObject $urlResolver; + private MockObject $requestTracker; + private MockObject $redirectRespHelper; protected function setUp(): void { - $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); - $this->requestTracker = $this->prophesize(RequestTrackerInterface::class); - $this->redirectRespHelper = $this->prophesize(RedirectResponseHelperInterface::class); + $this->urlResolver = $this->createMock(ShortUrlResolverInterface::class); + $this->requestTracker = $this->createMock(RequestTrackerInterface::class); + $this->redirectRespHelper = $this->createMock(RedirectResponseHelperInterface::class); - $redirectBuilder = $this->prophesize(ShortUrlRedirectionBuilderInterface::class); - $redirectBuilder->buildShortUrlRedirect(Argument::cetera())->willReturn(self::LONG_URL); + $redirectBuilder = $this->createMock(ShortUrlRedirectionBuilderInterface::class); + $redirectBuilder->method('buildShortUrlRedirect')->withAnyParameters()->willReturn(self::LONG_URL); $this->action = new RedirectAction( - $this->urlResolver->reveal(), - $this->requestTracker->reveal(), - $redirectBuilder->reveal(), - $this->redirectRespHelper->reveal(), + $this->urlResolver, + $this->requestTracker, + $redirectBuilder, + $this->redirectRespHelper, ); } @@ -53,38 +49,34 @@ class RedirectActionTest extends TestCase { $shortCode = 'abc123'; $shortUrl = ShortUrl::withLongUrl(self::LONG_URL); - $shortCodeToUrl = $this->urlResolver->resolveEnabledShortUrl( - ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, ''), + $this->urlResolver->expects($this->once())->method('resolveEnabledShortUrl')->with( + $this->equalTo(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, '')), )->willReturn($shortUrl); - $track = $this->requestTracker->trackIfApplicable(Argument::cetera())->will(function (): void { - }); + $this->requestTracker->expects($this->once())->method('trackIfApplicable'); $expectedResp = new Response\RedirectResponse(self::LONG_URL); - $buildResp = $this->redirectRespHelper->buildRedirectResponse(self::LONG_URL)->willReturn($expectedResp); + $this->redirectRespHelper->expects($this->once())->method('buildRedirectResponse')->with( + $this->equalTo(self::LONG_URL), + )->willReturn($expectedResp); $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); - $response = $this->action->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal()); + $response = $this->action->process($request, $this->createMock(RequestHandlerInterface::class)); self::assertSame($expectedResp, $response); - $buildResp->shouldHaveBeenCalledOnce(); - $shortCodeToUrl->shouldHaveBeenCalledOnce(); - $track->shouldHaveBeenCalledOnce(); } /** @test */ public function nextMiddlewareIsInvokedIfLongUrlIsNotFound(): void { $shortCode = 'abc123'; - $this->urlResolver->resolveEnabledShortUrl(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, '')) - ->willThrow(ShortUrlNotFoundException::class) - ->shouldBeCalledOnce(); - $this->requestTracker->trackIfApplicable(Argument::cetera())->shouldNotBeCalled(); + $this->urlResolver->expects($this->once())->method('resolveEnabledShortUrl')->with( + $this->equalTo(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, '')) + )->willThrowException(ShortUrlNotFoundException::fromNotFound(ShortUrlIdentifier::fromShortCodeAndDomain(''))); + $this->requestTracker->expects($this->never())->method('trackIfApplicable'); - $handler = $this->prophesize(RequestHandlerInterface::class); - $handle = $handler->handle(Argument::any())->willReturn(new Response()); + $handler = $this->createMock(RequestHandlerInterface::class); + $handler->expects($this->once())->method('handle')->withAnyParameters()->willReturn(new Response()); $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); - $this->action->process($request, $handler->reveal()); - - $handle->shouldHaveBeenCalledOnce(); + $this->action->process($request, $handler); } } From 29d50cabc29215e9c929650a795dcd6ed5ad9336 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 21 Oct 2022 18:47:10 +0200 Subject: [PATCH 04/10] Migrated NotFoundRedirectResolverTest to use PHPUnit mocks --- .../Config/NotFoundRedirectResolverTest.php | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/module/Core/test/Config/NotFoundRedirectResolverTest.php b/module/Core/test/Config/NotFoundRedirectResolverTest.php index 912e17a5..3cc9f691 100644 --- a/module/Core/test/Config/NotFoundRedirectResolverTest.php +++ b/module/Core/test/Config/NotFoundRedirectResolverTest.php @@ -9,10 +9,8 @@ use Laminas\Diactoros\ServerRequestFactory; use Laminas\Diactoros\Uri; use Mezzio\Router\Route; use Mezzio\Router\RouteResult; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Prophecy\Argument; -use Prophecy\PhpUnit\ProphecyTrait; -use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\UriInterface; use Psr\Http\Server\MiddlewareInterface; @@ -25,15 +23,13 @@ use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; class NotFoundRedirectResolverTest extends TestCase { - use ProphecyTrait; - private NotFoundRedirectResolver $resolver; - private ObjectProphecy $helper; + private MockObject $helper; protected function setUp(): void { - $this->helper = $this->prophesize(RedirectResponseHelperInterface::class); - $this->resolver = new NotFoundRedirectResolver($this->helper->reveal(), new NullLogger()); + $this->helper = $this->createMock(RedirectResponseHelperInterface::class); + $this->resolver = new NotFoundRedirectResolver($this->helper, new NullLogger()); } /** @@ -47,12 +43,13 @@ class NotFoundRedirectResolverTest extends TestCase string $expectedRedirectTo, ): void { $expectedResp = new Response(); - $buildResp = $this->helper->buildRedirectResponse($expectedRedirectTo)->willReturn($expectedResp); + $this->helper->expects($this->once())->method('buildRedirectResponse')->with( + $this->equalTo($expectedRedirectTo), + )->willReturn($expectedResp); $resp = $this->resolver->resolveRedirectResponse($notFoundType, $redirectConfig, $uri); self::assertSame($expectedResp, $resp); - $buildResp->shouldHaveBeenCalledOnce(); } public function provideRedirects(): iterable @@ -119,11 +116,11 @@ class NotFoundRedirectResolverTest extends TestCase public function noResponseIsReturnedIfNoConditionsMatch(): void { $notFoundType = $this->notFoundType($this->requestForRoute('foo')); + $this->helper->expects($this->never())->method('buildRedirectResponse'); $result = $this->resolver->resolveRedirectResponse($notFoundType, new NotFoundRedirectOptions(), new Uri()); self::assertNull($result); - $this->helper->buildRedirectResponse(Argument::cetera())->shouldNotHaveBeenCalled(); } private function notFoundType(ServerRequestInterface $req): NotFoundType @@ -139,7 +136,7 @@ class NotFoundRedirectResolverTest extends TestCase RouteResult::fromRoute( new Route( '', - $this->prophesize(MiddlewareInterface::class)->reveal(), + $this->createMock(MiddlewareInterface::class), ['GET'], $routeName, ), From 148f7a9cfeccdd9dfee9aa6070b6663481248854 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 21 Oct 2022 18:49:47 +0200 Subject: [PATCH 05/10] Migrated CrawlingHelperTest to use PHPUnit mocks --- .../Core/test/Crawling/CrawlingHelperTest.php | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/module/Core/test/Crawling/CrawlingHelperTest.php b/module/Core/test/Crawling/CrawlingHelperTest.php index 92901efc..7667fb93 100644 --- a/module/Core/test/Crawling/CrawlingHelperTest.php +++ b/module/Core/test/Crawling/CrawlingHelperTest.php @@ -5,39 +5,35 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Crawling; use Doctrine\ORM\EntityManagerInterface; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Prophecy\PhpUnit\ProphecyTrait; -use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Crawling\CrawlingHelper; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface; class CrawlingHelperTest extends TestCase { - use ProphecyTrait; - private CrawlingHelper $helper; - private ObjectProphecy $em; + private MockObject $em; protected function setUp(): void { - $this->em = $this->prophesize(EntityManagerInterface::class); - $this->helper = new CrawlingHelper($this->em->reveal()); + $this->em = $this->createMock(EntityManagerInterface::class); + $this->helper = new CrawlingHelper($this->em); } /** @test */ public function listCrawlableShortCodesDelegatesIntoRepository(): void { - $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $findCrawlableShortCodes = $repo->findCrawlableShortCodes()->willReturn([]); - $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); + $repo = $this->createMock(ShortUrlRepositoryInterface::class); + $repo->expects($this->once())->method('findCrawlableShortCodes')->willReturn([]); + $this->em->expects($this->once())->method('getRepository')->with($this->equalTo(ShortUrl::class))->willReturn( + $repo, + ); $result = $this->helper->listCrawlableShortCodes(); foreach ($result as $shortCode) { - // Result is a generator and therefore, it needs to be iterated + // $result is a generator and therefore, it needs to be iterated } - - $findCrawlableShortCodes->shouldHaveBeenCalledOnce(); - $getRepo->shouldHaveBeenCalledOnce(); } } From a8514a9ae4257291c3d50f41c86f35582aa9b3c5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 21 Oct 2022 19:01:41 +0200 Subject: [PATCH 06/10] Migrated DomainServiceTest to use PHPUnit mocks --- module/Core/test/Domain/DomainServiceTest.php | 87 ++++++++++--------- 1 file changed, 48 insertions(+), 39 deletions(-) diff --git a/module/Core/test/Domain/DomainServiceTest.php b/module/Core/test/Domain/DomainServiceTest.php index a4ec22f3..307fbcb3 100644 --- a/module/Core/test/Domain/DomainServiceTest.php +++ b/module/Core/test/Domain/DomainServiceTest.php @@ -5,10 +5,8 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Domain; 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 Shlinkio\Shlink\Core\Config\EmptyNotFoundRedirectConfig; use Shlinkio\Shlink\Core\Config\NotFoundRedirects; use Shlinkio\Shlink\Core\Domain\DomainService; @@ -22,15 +20,13 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; class DomainServiceTest extends TestCase { - use ProphecyTrait; - private DomainService $domainService; - private ObjectProphecy $em; + private MockObject $em; protected function setUp(): void { - $this->em = $this->prophesize(EntityManagerInterface::class); - $this->domainService = new DomainService($this->em->reveal(), 'default.com'); + $this->em = $this->createMock(EntityManagerInterface::class); + $this->domainService = new DomainService($this->em, 'default.com'); } /** @@ -39,15 +35,15 @@ class DomainServiceTest extends TestCase */ public function listDomainsDelegatesIntoRepository(array $domains, array $expectedResult, ?ApiKey $apiKey): void { - $repo = $this->prophesize(DomainRepositoryInterface::class); - $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); - $findDomains = $repo->findDomains($apiKey)->willReturn($domains); + $repo = $this->createMock(DomainRepositoryInterface::class); + $repo->expects($this->once())->method('findDomains')->with($this->equalTo($apiKey))->willReturn($domains); + $this->em->expects($this->once())->method('getRepository')->with($this->equalTo(Domain::class))->willReturn( + $repo, + ); $result = $this->domainService->listDomains($apiKey); self::assertEquals($expectedResult, $result); - $getRepo->shouldHaveBeenCalledOnce(); - $findDomains->shouldHaveBeenCalledOnce(); } public function provideExcludedDomains(): iterable @@ -109,10 +105,12 @@ class DomainServiceTest extends TestCase /** @test */ public function getDomainThrowsExceptionWhenDomainIsNotFound(): void { - $find = $this->em->find(Domain::class, '123')->willReturn(null); + $this->em->expects($this->once())->method('find')->with( + $this->equalTo(Domain::class), + $this->equalTo('123'), + )->willReturn(null); $this->expectException(DomainNotFoundException::class); - $find->shouldBeCalledOnce(); $this->domainService->getDomain('123'); } @@ -121,12 +119,14 @@ class DomainServiceTest extends TestCase public function getDomainReturnsEntityWhenFound(): void { $domain = Domain::withAuthority(''); - $find = $this->em->find(Domain::class, '123')->willReturn($domain); + $this->em->expects($this->once())->method('find')->with( + $this->equalTo(Domain::class), + $this->equalTo('123'), + )->willReturn($domain); $result = $this->domainService->getDomain('123'); self::assertSame($domain, $result); - $find->shouldHaveBeenCalledOnce(); } /** @@ -136,20 +136,23 @@ class DomainServiceTest extends TestCase public function getOrCreateAlwaysPersistsDomain(?Domain $foundDomain, ?ApiKey $apiKey): void { $authority = 'example.com'; - $repo = $this->prophesize(DomainRepositoryInterface::class); - $repo->findOneByAuthority($authority, $apiKey)->willReturn($foundDomain); - $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); - $persist = $this->em->persist($foundDomain ?? Argument::type(Domain::class)); - $flush = $this->em->flush(); + $repo = $this->createMock(DomainRepositoryInterface::class); + $repo->method('findOneByAuthority')->with($this->equalTo($authority), $this->equalTo($apiKey))->willReturn( + $foundDomain, + ); + $this->em->expects($this->once())->method('getRepository')->with($this->equalTo(Domain::class))->willReturn( + $repo, + ); + $this->em->expects($this->once())->method('persist')->with( + $foundDomain !== null ? $this->equalTo($foundDomain) : $this->isInstanceOf(Domain::class), + ); + $this->em->expects($this->once())->method('flush'); $result = $this->domainService->getOrCreate($authority, $apiKey); if ($foundDomain !== null) { self::assertSame($result, $foundDomain); } - $getRepo->shouldHaveBeenCalledOnce(); - $persist->shouldHaveBeenCalledOnce(); - $flush->shouldHaveBeenCalledOnce(); } /** @test */ @@ -158,14 +161,17 @@ class DomainServiceTest extends TestCase $authority = 'example.com'; $domain = Domain::withAuthority($authority)->setId('1'); $apiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forDomain($domain))); - $repo = $this->prophesize(DomainRepositoryInterface::class); - $repo->findOneByAuthority($authority, $apiKey)->willReturn(null); - $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); + $repo = $this->createMock(DomainRepositoryInterface::class); + $repo->method('findOneByAuthority')->with($this->equalTo($authority), $this->equalTo($apiKey))->willReturn( + null, + ); + $this->em->expects($this->once())->method('getRepository')->with($this->equalTo(Domain::class))->willReturn( + $repo, + ); + $this->em->expects($this->never())->method('persist'); + $this->em->expects($this->never())->method('flush'); $this->expectException(DomainNotFoundException::class); - $getRepo->shouldBeCalledOnce(); - $this->em->persist(Argument::cetera())->shouldNotBeCalled(); - $this->em->flush()->shouldNotBeCalled(); $this->domainService->getOrCreate($authority, $apiKey); } @@ -177,11 +183,17 @@ class DomainServiceTest extends TestCase public function configureNotFoundRedirectsConfiguresFetchedDomain(?Domain $foundDomain, ?ApiKey $apiKey): void { $authority = 'example.com'; - $repo = $this->prophesize(DomainRepositoryInterface::class); - $repo->findOneByAuthority($authority, $apiKey)->willReturn($foundDomain); - $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); - $persist = $this->em->persist($foundDomain ?? Argument::type(Domain::class)); - $flush = $this->em->flush(); + $repo = $this->createMock(DomainRepositoryInterface::class); + $repo->method('findOneByAuthority')->with($this->equalTo($authority), $this->equalTo($apiKey))->willReturn( + $foundDomain, + ); + $this->em->expects($this->once())->method('getRepository')->with($this->equalTo(Domain::class))->willReturn( + $repo, + ); + $this->em->expects($this->once())->method('persist')->with( + $foundDomain !== null ? $this->equalTo($foundDomain) : $this->isInstanceOf(Domain::class), + ); + $this->em->expects($this->once())->method('flush'); $result = $this->domainService->configureNotFoundRedirects($authority, NotFoundRedirects::withRedirects( 'foo.com', @@ -195,9 +207,6 @@ class DomainServiceTest extends TestCase self::assertEquals('foo.com', $result->baseUrlRedirect()); self::assertEquals('bar.com', $result->regular404Redirect()); self::assertEquals('baz.com', $result->invalidShortUrlRedirect()); - $getRepo->shouldHaveBeenCalledOnce(); - $persist->shouldHaveBeenCalledOnce(); - $flush->shouldHaveBeenCalledOnce(); } public function provideFoundDomains(): iterable From 230e56370a084e24071beca436f9f291474a8398 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 21 Oct 2022 19:24:39 +0200 Subject: [PATCH 07/10] Migrated NotFoundRedirectHandlerTest to use PHPUnit mocks --- module/Core/test/Action/QrCodeActionTest.php | 2 +- .../Core/test/Action/RedirectActionTest.php | 2 +- .../NotFoundRedirectHandlerTest.php | 119 +++++++++--------- 3 files changed, 61 insertions(+), 62 deletions(-) diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index 8bec70e4..06df1ce8 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -42,7 +42,7 @@ class QrCodeActionTest extends TestCase { $shortCode = 'abc123'; $this->urlResolver->expects($this->once())->method('resolveEnabledShortUrl')->with( - $this->equalTo(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, '')) + $this->equalTo(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, '')), )->willThrowException(ShortUrlNotFoundException::fromNotFound(ShortUrlIdentifier::fromShortCodeAndDomain(''))); $delegate = $this->createMock(RequestHandlerInterface::class); $delegate->expects($this->once())->method('handle')->withAnyParameters()->willReturn(new Response()); diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index f17d32ba..06b7ac40 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -69,7 +69,7 @@ class RedirectActionTest extends TestCase { $shortCode = 'abc123'; $this->urlResolver->expects($this->once())->method('resolveEnabledShortUrl')->with( - $this->equalTo(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, '')) + $this->equalTo(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, '')), )->willThrowException(ShortUrlNotFoundException::fromNotFound(ShortUrlIdentifier::fromShortCodeAndDomain(''))); $this->requestTracker->expects($this->never())->method('trackIfApplicable'); diff --git a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php index d08073e2..964f5da4 100644 --- a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php +++ b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php @@ -6,10 +6,8 @@ namespace ShlinkioTest\Shlink\Core\ErrorHandler; use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequestFactory; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Prophecy\Argument; -use Prophecy\PhpUnit\ProphecyTrait; -use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\UriInterface; use Psr\Http\Server\RequestHandlerInterface; @@ -22,31 +20,25 @@ use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; class NotFoundRedirectHandlerTest extends TestCase { - use ProphecyTrait; - private NotFoundRedirectHandler $middleware; private NotFoundRedirectOptions $redirectOptions; - private ObjectProphecy $resolver; - private ObjectProphecy $domainService; - private ObjectProphecy $next; + private MockObject $resolver; + private MockObject $domainService; + private MockObject $next; private ServerRequestInterface $req; protected function setUp(): void { $this->redirectOptions = new NotFoundRedirectOptions(); - $this->resolver = $this->prophesize(NotFoundRedirectResolverInterface::class); - $this->domainService = $this->prophesize(DomainServiceInterface::class); + $this->resolver = $this->createMock(NotFoundRedirectResolverInterface::class); + $this->domainService = $this->createMock(DomainServiceInterface::class); - $this->middleware = new NotFoundRedirectHandler( - $this->redirectOptions, - $this->resolver->reveal(), - $this->domainService->reveal(), - ); + $this->middleware = new NotFoundRedirectHandler($this->redirectOptions, $this->resolver, $this->domainService); - $this->next = $this->prophesize(RequestHandlerInterface::class); + $this->next = $this->createMock(RequestHandlerInterface::class); $this->req = ServerRequestFactory::fromGlobals()->withAttribute( NotFoundType::class, - $this->prophesize(NotFoundType::class)->reveal(), + $this->createMock(NotFoundType::class), ); } @@ -59,40 +51,49 @@ class NotFoundRedirectHandlerTest extends TestCase $expectedResp = new Response(); $setUp($this->domainService, $this->resolver); - $handle = $this->next->handle($this->req)->willReturn($expectedResp); + $this->next->expects($this->once())->method('handle')->with($this->equalTo($this->req))->willReturn( + $expectedResp, + ); - $result = $this->middleware->process($this->req, $this->next->reveal()); + $result = $this->middleware->process($this->req, $this->next); self::assertSame($expectedResp, $result); - $handle->shouldHaveBeenCalledOnce(); } public function provideNonRedirectScenarios(): iterable { - yield 'no domain' => [function (ObjectProphecy $domainService, ObjectProphecy $resolver): void { - $domainService->findByAuthority(Argument::cetera()) - ->willReturn(null) - ->shouldBeCalledOnce(); - $resolver->resolveRedirectResponse( - Argument::type(NotFoundType::class), - Argument::type(NotFoundRedirectOptions::class), - Argument::type(UriInterface::class), - )->willReturn(null)->shouldBeCalledOnce(); + yield 'no domain' => [function ( + MockObject&DomainServiceInterface $domainService, + MockObject&NotFoundRedirectResolverInterface $resolver, + ): void { + $domainService->expects($this->once())->method('findByAuthority')->withAnyParameters()->willReturn( + null, + ); + $resolver->expects($this->once())->method('resolveRedirectResponse')->with( + $this->isInstanceOf(NotFoundType::class), + $this->isInstanceOf(NotFoundRedirectOptions::class), + $this->isInstanceOf(UriInterface::class), + )->willReturn(null); }]; - yield 'non-redirecting domain' => [function (ObjectProphecy $domainService, ObjectProphecy $resolver): void { - $domainService->findByAuthority(Argument::cetera()) - ->willReturn(Domain::withAuthority('')) - ->shouldBeCalledOnce(); - $resolver->resolveRedirectResponse( - Argument::type(NotFoundType::class), - Argument::type(NotFoundRedirectOptions::class), - Argument::type(UriInterface::class), - )->willReturn(null)->shouldBeCalledOnce(); - $resolver->resolveRedirectResponse( - Argument::type(NotFoundType::class), - Argument::type(Domain::class), - Argument::type(UriInterface::class), - )->willReturn(null)->shouldBeCalledOnce(); + yield 'non-redirecting domain' => [function ( + MockObject&DomainServiceInterface $domainService, + MockObject&NotFoundRedirectResolverInterface $resolver, + ): void { + $domainService->expects($this->once())->method('findByAuthority')->withAnyParameters()->willReturn( + Domain::withAuthority(''), + ); + $resolver->expects($this->exactly(2))->method('resolveRedirectResponse')->withConsecutive( + [ + $this->isInstanceOf(NotFoundType::class), + $this->isInstanceOf(Domain::class), + $this->isInstanceOf(UriInterface::class), + ], + [ + $this->isInstanceOf(NotFoundType::class), + $this->isInstanceOf(NotFoundRedirectOptions::class), + $this->isInstanceOf(UriInterface::class), + ], + )->willReturn(null); }]; } @@ -101,19 +102,17 @@ class NotFoundRedirectHandlerTest extends TestCase { $expectedResp = new Response(); - $findDomain = $this->domainService->findByAuthority(Argument::cetera())->willReturn(null); - $resolveRedirect = $this->resolver->resolveRedirectResponse( - Argument::type(NotFoundType::class), - $this->redirectOptions, - Argument::type(UriInterface::class), + $this->domainService->expects($this->once())->method('findByAuthority')->withAnyParameters()->willReturn(null); + $this->resolver->expects($this->once())->method('resolveRedirectResponse')->with( + $this->isInstanceOf(NotFoundType::class), + $this->equalTo($this->redirectOptions), + $this->isInstanceOf(UriInterface::class), )->willReturn($expectedResp); + $this->next->expects($this->never())->method('handle'); - $result = $this->middleware->process($this->req, $this->next->reveal()); + $result = $this->middleware->process($this->req, $this->next); self::assertSame($expectedResp, $result); - $findDomain->shouldHaveBeenCalledOnce(); - $resolveRedirect->shouldHaveBeenCalledOnce(); - $this->next->handle(Argument::cetera())->shouldNotHaveBeenCalled(); } /** @test */ @@ -122,18 +121,18 @@ class NotFoundRedirectHandlerTest extends TestCase $expectedResp = new Response(); $domain = Domain::withAuthority(''); - $findDomain = $this->domainService->findByAuthority(Argument::cetera())->willReturn($domain); - $resolveRedirect = $this->resolver->resolveRedirectResponse( - Argument::type(NotFoundType::class), + $this->domainService->expects($this->once())->method('findByAuthority')->withAnyParameters()->willReturn( $domain, - Argument::type(UriInterface::class), + ); + $this->resolver->expects($this->once())->method('resolveRedirectResponse')->with( + $this->isInstanceOf(NotFoundType::class), + $this->equalTo($domain), + $this->isInstanceOf(UriInterface::class), )->willReturn($expectedResp); + $this->next->expects($this->never())->method('handle'); - $result = $this->middleware->process($this->req, $this->next->reveal()); + $result = $this->middleware->process($this->req, $this->next); self::assertSame($expectedResp, $result); - $findDomain->shouldHaveBeenCalledOnce(); - $resolveRedirect->shouldHaveBeenCalledOnce(); - $this->next->handle(Argument::cetera())->shouldNotHaveBeenCalled(); } } From d8420258352e0e1045e1a3f3c471f1687f5e6972 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 21 Oct 2022 19:25:29 +0200 Subject: [PATCH 08/10] Migrated NotFoundTemplateHandlerTest to use PHPUnit mocks --- module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php index 12865171..aa6302a3 100644 --- a/module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php +++ b/module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php @@ -56,7 +56,7 @@ class NotFoundTemplateHandlerTest extends TestCase RouteResult::fromRoute( new Route( '', - $this->prophesize(MiddlewareInterface::class)->reveal(), + $this->createMock(MiddlewareInterface::class), ['GET'], RedirectAction::class, ), From ff543b151cd05d90b982d591092ac9a1b4363347 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 21 Oct 2022 19:29:02 +0200 Subject: [PATCH 09/10] Migrated NotFoundTrackerMiddlewareTest to use PHPUnit mocks --- .../NotFoundTrackerMiddlewareTest.php | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/module/Core/test/ErrorHandler/NotFoundTrackerMiddlewareTest.php b/module/Core/test/ErrorHandler/NotFoundTrackerMiddlewareTest.php index 81fef1a6..823a7546 100644 --- a/module/Core/test/ErrorHandler/NotFoundTrackerMiddlewareTest.php +++ b/module/Core/test/ErrorHandler/NotFoundTrackerMiddlewareTest.php @@ -4,12 +4,9 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\ErrorHandler; -use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequestFactory; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Prophecy\Argument; -use Prophecy\PhpUnit\ProphecyTrait; -use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; @@ -18,35 +15,31 @@ use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; class NotFoundTrackerMiddlewareTest extends TestCase { - use ProphecyTrait; - private NotFoundTrackerMiddleware $middleware; private ServerRequestInterface $request; - private ObjectProphecy $requestTracker; - private ObjectProphecy $notFoundType; - private ObjectProphecy $handler; + private MockObject $handler; + private MockObject $requestTracker; protected function setUp(): void { - $this->notFoundType = $this->prophesize(NotFoundType::class); - $this->handler = $this->prophesize(RequestHandlerInterface::class); - $this->handler->handle(Argument::cetera())->willReturn(new Response()); - - $this->requestTracker = $this->prophesize(RequestTrackerInterface::class); - $this->middleware = new NotFoundTrackerMiddleware($this->requestTracker->reveal()); + $this->handler = $this->createMock(RequestHandlerInterface::class); + $this->requestTracker = $this->createMock(RequestTrackerInterface::class); + $this->middleware = new NotFoundTrackerMiddleware($this->requestTracker); $this->request = ServerRequestFactory::fromGlobals()->withAttribute( NotFoundType::class, - $this->notFoundType->reveal(), + $this->createMock(NotFoundType::class), ); } /** @test */ public function delegatesIntoRequestTracker(): void { - $this->middleware->process($this->request, $this->handler->reveal()); + $this->handler->expects($this->once())->method('handle')->with($this->equalTo($this->request)); + $this->requestTracker->expects($this->once())->method('trackNotFoundIfApplicable')->with( + $this->equalTo($this->request), + ); - $this->requestTracker->trackNotFoundIfApplicable($this->request)->shouldHaveBeenCalledOnce(); - $this->handler->handle($this->request)->shouldHaveBeenCalledOnce(); + $this->middleware->process($this->request, $this->handler); } } From bf0b58b344a5aaca68e393178962de2ec4de2a9a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 21 Oct 2022 19:32:25 +0200 Subject: [PATCH 10/10] Migrated NotFoundTypeResolverMiddlewareTest to use PHPUnit mocks --- .../NotFoundTypeResolverMiddlewareTest.php | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/module/Core/test/ErrorHandler/NotFoundTypeResolverMiddlewareTest.php b/module/Core/test/ErrorHandler/NotFoundTypeResolverMiddlewareTest.php index c5d9be79..c2ef419b 100644 --- a/module/Core/test/ErrorHandler/NotFoundTypeResolverMiddlewareTest.php +++ b/module/Core/test/ErrorHandler/NotFoundTypeResolverMiddlewareTest.php @@ -7,10 +7,8 @@ namespace ShlinkioTest\Shlink\Core\ErrorHandler; use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequestFactory; 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\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; @@ -18,30 +16,28 @@ use Shlinkio\Shlink\Core\ErrorHandler\NotFoundTypeResolverMiddleware; class NotFoundTypeResolverMiddlewareTest extends TestCase { - use ProphecyTrait; - private NotFoundTypeResolverMiddleware $middleware; - private ObjectProphecy $handler; + private MockObject $handler; protected function setUp(): void { $this->middleware = new NotFoundTypeResolverMiddleware(''); - $this->handler = $this->prophesize(RequestHandlerInterface::class); + $this->handler = $this->createMock(RequestHandlerInterface::class); } /** @test */ public function notFoundTypeIsAddedToRequest(): void { $request = ServerRequestFactory::fromGlobals(); - $handle = $this->handler->handle(Argument::that(function (ServerRequestInterface $req) { - Assert::assertArrayHasKey(NotFoundType::class, $req->getAttributes()); + $this->handler->expects($this->once())->method('handle')->with( + $this->callback(function (ServerRequestInterface $req): bool { + Assert::assertArrayHasKey(NotFoundType::class, $req->getAttributes()); + return true; + }), + )->willReturn(new Response()); - return true; - }))->willReturn(new Response()); - - $this->middleware->process($request, $this->handler->reveal()); + $this->middleware->process($request, $this->handler); self::assertArrayNotHasKey(NotFoundType::class, $request->getAttributes()); - $handle->shouldHaveBeenCalledOnce(); } }