diff --git a/CHANGELOG.md b/CHANGELOG.md index 55963587..5a83475c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#330](https://github.com/shlinkio/shlink/issues/330) No longer allow failures on PHP 7.3 envs during project CI build. * [#335](https://github.com/shlinkio/shlink/issues/335) Renamed functional test suite to database test suite, since that better describes what it actually does. * [#346](https://github.com/shlinkio/shlink/issues/346) Extracted installer as an independent tool. +* [#261](https://github.com/shlinkio/shlink/issues/261) Increased mutation score to 70%. #### Deprecated diff --git a/composer.json b/composer.json index a8f78431..ad597380 100644 --- a/composer.json +++ b/composer.json @@ -53,8 +53,8 @@ "devster/ubench": "^2.0", "doctrine/data-fixtures": "^1.3", "filp/whoops": "^2.0", - "infection/infection": "^0.11.0", - "phpstan/phpstan": "^0.10.0", + "infection/infection": "^0.12.2", + "phpstan/phpstan": "^0.10.8", "phpunit/phpcov": "^6.0@dev || ^5.0", "phpunit/phpunit": "^8.0 || ^7.5", "roave/security-advisories": "dev-master", @@ -123,9 +123,9 @@ ], "test:unit:pretty": "phpdbg -qrr vendor/bin/phpunit --coverage-html build/coverage --order-by=random", - "infect": "infection --threads=4 --min-msi=65 --log-verbosity=default --only-covered", - "infect:ci": "infection --threads=4 --min-msi=65 --log-verbosity=default --only-covered --coverage=build", - "infect:show": "infection --threads=4 --min-msi=65 --log-verbosity=default --only-covered --show-mutations", + "infect": "infection --threads=4 --min-msi=70 --log-verbosity=default --only-covered", + "infect:ci": "infection --threads=4 --min-msi=70 --log-verbosity=default --only-covered --coverage=build", + "infect:show": "infection --threads=4 --min-msi=70 --log-verbosity=default --only-covered --show-mutations", "infect:test": [ "@test:unit:ci", "@infect:ci" diff --git a/module/CLI/test/Command/Api/ListKeysCommandTest.php b/module/CLI/test/Command/Api/ListKeysCommandTest.php index c7b081f3..6d979aa2 100644 --- a/module/CLI/test/Command/Api/ListKeysCommandTest.php +++ b/module/CLI/test/Command/Api/ListKeysCommandTest.php @@ -36,9 +36,7 @@ class ListKeysCommandTest extends TestCase new ApiKey(), ])->shouldBeCalledOnce(); - $this->commandTester->execute([ - 'command' => ListKeysCommand::NAME, - ]); + $this->commandTester->execute([]); $output = $this->commandTester->getDisplay(); $this->assertStringContainsString('Key', $output); @@ -57,7 +55,6 @@ class ListKeysCommandTest extends TestCase ])->shouldBeCalledOnce(); $this->commandTester->execute([ - 'command' => ListKeysCommand::NAME, '--enabledOnly' => true, ]); $output = $this->commandTester->getDisplay(); diff --git a/module/CLI/test/Command/ShortUrl/DeleteShortCodeCommandTest.php b/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php similarity index 79% rename from module/CLI/test/Command/ShortUrl/DeleteShortCodeCommandTest.php rename to module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php index ee514109..9e81b5fc 100644 --- a/module/CLI/test/Command/ShortUrl/DeleteShortCodeCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php @@ -11,10 +11,11 @@ use Shlinkio\Shlink\Core\Exception; use Shlinkio\Shlink\Core\Service\ShortUrl\DeleteShortUrlServiceInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; +use const PHP_EOL; use function array_pop; use function sprintf; -class DeleteShortCodeCommandTest extends TestCase +class DeleteShortUrlCommandTest extends TestCase { /** @var CommandTester */ private $commandTester; @@ -32,10 +33,8 @@ class DeleteShortCodeCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ - public function successMessageIsPrintedIfUrlIsProperlyDeleted() + /** @test */ + public function successMessageIsPrintedIfUrlIsProperlyDeleted(): void { $shortCode = 'abc123'; $deleteByShortCode = $this->service->deleteByShortCode($shortCode, false)->will(function () { @@ -51,10 +50,8 @@ class DeleteShortCodeCommandTest extends TestCase $deleteByShortCode->shouldHaveBeenCalledOnce(); } - /** - * @test - */ - public function invalidShortCodePrintsMessage() + /** @test */ + public function invalidShortCodePrintsMessage(): void { $shortCode = 'abc123'; $deleteByShortCode = $this->service->deleteByShortCode($shortCode, false)->willThrow( @@ -70,9 +67,13 @@ class DeleteShortCodeCommandTest extends TestCase /** * @test + * @dataProvider provideRetryDeleteAnswers */ - public function deleteIsRetriedWhenThresholdIsReachedAndQuestionIsAccepted() - { + public function deleteIsRetriedWhenThresholdIsReachedAndQuestionIsAccepted( + array $retryAnswer, + int $expectedDeleteCalls, + string $expectedMessage + ): void { $shortCode = 'abc123'; $deleteByShortCode = $this->service->deleteByShortCode($shortCode, Argument::type('bool'))->will( function (array $args) { @@ -83,7 +84,7 @@ class DeleteShortCodeCommandTest extends TestCase } } ); - $this->commandTester->setInputs(['yes']); + $this->commandTester->setInputs($retryAnswer); $this->commandTester->execute(['shortCode' => $shortCode]); $output = $this->commandTester->getDisplay(); @@ -92,17 +93,19 @@ class DeleteShortCodeCommandTest extends TestCase 'It was not possible to delete the short URL with short code "%s" because it has more than 10 visits.', $shortCode ), $output); - $this->assertStringContainsString( - sprintf('Short URL with short code "%s" successfully deleted.', $shortCode), - $output - ); - $deleteByShortCode->shouldHaveBeenCalledTimes(2); + $this->assertStringContainsString($expectedMessage, $output); + $deleteByShortCode->shouldHaveBeenCalledTimes($expectedDeleteCalls); } - /** - * @test - */ - public function deleteIsNotRetriedWhenThresholdIsReachedAndQuestionIsDeclined() + public function provideRetryDeleteAnswers(): iterable + { + yield 'answering yes to retry' => [['yes'], 2, 'Short URL with short code "abc123" successfully deleted.']; + yield 'answering no to retry' => [['no'], 1, 'Short URL was not deleted.']; + yield 'answering default to retry' => [[PHP_EOL], 1, 'Short URL was not deleted.']; + } + + /** @test */ + public function deleteIsNotRetriedWhenThresholdIsReachedAndQuestionIsDeclined(): void { $shortCode = 'abc123'; $deleteByShortCode = $this->service->deleteByShortCode($shortCode, false)->willThrow( diff --git a/module/Rest/src/Exception/AuthenticationException.php b/module/Rest/src/Exception/AuthenticationException.php index e1e0c9e4..2258ab30 100644 --- a/module/Rest/src/Exception/AuthenticationException.php +++ b/module/Rest/src/Exception/AuthenticationException.php @@ -3,11 +3,11 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Exception; -use Exception; +use Throwable; class AuthenticationException extends RuntimeException { - public static function expiredJWT(Exception $prev = null): self + public static function expiredJWT(?Throwable $prev = null): self { return new self('The token has expired.', -1, $prev); } diff --git a/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php b/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php index f5a9c956..bc182071 100644 --- a/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php +++ b/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php @@ -58,7 +58,7 @@ class CreateShortUrlContentNegotiationMiddleware implements MiddlewareInterface return self::JSON; } - $format = strtolower((string) $query['format']); + $format = strtolower($query['format']); return $format === 'txt' ? self::PLAIN_TEXT : self::JSON; } diff --git a/module/Rest/test/ErrorHandler/JsonErrorResponseGeneratorTest.php b/module/Rest/test/ErrorHandler/JsonErrorResponseGeneratorTest.php index 3cb288da..22821854 100644 --- a/module/Rest/test/ErrorHandler/JsonErrorResponseGeneratorTest.php +++ b/module/Rest/test/ErrorHandler/JsonErrorResponseGeneratorTest.php @@ -7,6 +7,8 @@ use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Rest\ErrorHandler\JsonErrorResponseGenerator; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequest; +use function array_map; +use function range; class JsonErrorResponseGeneratorTest extends TestCase { @@ -18,27 +20,41 @@ class JsonErrorResponseGeneratorTest extends TestCase $this->errorHandler = new JsonErrorResponseGenerator(); } - /** - * @test - */ - public function noErrorStatusReturnsInternalServerError() + /** @test */ + public function noErrorStatusReturnsInternalServerError(): void { + /** @var Response\JsonResponse $response */ $response = $this->errorHandler->__invoke(null, new ServerRequest(), new Response()); + $payload = $response->getPayload(); + $this->assertInstanceOf(Response\JsonResponse::class, $response); $this->assertEquals(500, $response->getStatusCode()); + $this->assertEquals('Internal Server Error', $payload['message']); } /** * @test + * @dataProvider provideStatus */ - public function errorStatusReturnsThatStatus() + public function errorStatusReturnsThatStatus(int $status, string $message): void { + /** @var Response\JsonResponse $response */ $response = $this->errorHandler->__invoke( null, new ServerRequest(), - (new Response())->withStatus(405) + (new Response())->withStatus($status, $message) ); + $payload = $response->getPayload(); + $this->assertInstanceOf(Response\JsonResponse::class, $response); - $this->assertEquals(405, $response->getStatusCode()); + $this->assertEquals($status, $response->getStatusCode()); + $this->assertEquals($message, $payload['message']); + } + + public function provideStatus(): iterable + { + return array_map(function (int $status) { + return [$status, 'Some message']; + }, range(400, 500, 20)); } } diff --git a/module/Rest/test/Exception/AuthenticationExceptionTest.php b/module/Rest/test/Exception/AuthenticationExceptionTest.php new file mode 100644 index 00000000..f953d40e --- /dev/null +++ b/module/Rest/test/Exception/AuthenticationExceptionTest.php @@ -0,0 +1,31 @@ +assertEquals($prev, $e->getPrevious()); + $this->assertEquals(-1, $e->getCode()); + $this->assertEquals('The token has expired.', $e->getMessage()); + } + + public function providePrev() + { + yield 'with previous exception' => [new Exception('Prev')]; + yield 'without previous exception' => [null]; + } +} diff --git a/module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php b/module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php new file mode 100644 index 00000000..74fe9fc2 --- /dev/null +++ b/module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php @@ -0,0 +1,93 @@ +assertEquals(0, $e->getCode()); + $this->assertEquals( + sprintf('Authentication verification failed with the public message "%s"', $message), + $e->getMessage() + ); + $this->assertEquals($code, $e->getErrorCode()); + $this->assertEquals($message, $e->getPublicMessage()); + $this->assertEquals($prev, $e->getPrevious()); + } + + public function provideExceptionData(): iterable + { + return array_map(function () { + return [ + $this->generateRandomString(), + $this->generateRandomString(50), + random_int(0, 1) === 1 ? new Exception('Prev') : null, + ]; + }, range(1, 10)); + } + + /** + * @test + * @dataProvider provideConstructorData + */ + public function constructCreatesExpectedException( + string $errorCode, + string $publicMessage, + string $message, + int $code, + ?Throwable $prev + ): void { + $e = new VerifyAuthenticationException($errorCode, $publicMessage, $message, $code, $prev); + + $this->assertEquals($code, $e->getCode()); + $this->assertEquals($message, $e->getMessage()); + $this->assertEquals($errorCode, $e->getErrorCode()); + $this->assertEquals($publicMessage, $e->getPublicMessage()); + $this->assertEquals($prev, $e->getPrevious()); + } + + public function provideConstructorData(): iterable + { + return array_map(function (int $i) { + return [ + $this->generateRandomString(), + $this->generateRandomString(30), + $this->generateRandomString(50), + $i, + random_int(0, 1) === 1 ? new Exception('Prev') : null, + ]; + }, range(10, 20)); + } + + /** @test */ + public function defaultConstructorValuesAreKept(): void + { + $e = new VerifyAuthenticationException('foo', 'bar'); + + $this->assertEquals(0, $e->getCode()); + $this->assertEquals('', $e->getMessage()); + $this->assertEquals('foo', $e->getErrorCode()); + $this->assertEquals('bar', $e->getPublicMessage()); + $this->assertNull($e->getPrevious()); + } +} diff --git a/module/Rest/test/Middleware/BodyParserMiddlewareTest.php b/module/Rest/test/Middleware/BodyParserMiddlewareTest.php index 31adc609..b127fd8b 100644 --- a/module/Rest/test/Middleware/BodyParserMiddlewareTest.php +++ b/module/Rest/test/Middleware/BodyParserMiddlewareTest.php @@ -26,23 +26,40 @@ class BodyParserMiddlewareTest extends TestCase /** * @test + * @dataProvider provideIgnoredRequestMethods */ - public function requestsFromOtherMethodsJustFallbackToNextMiddleware() + public function requestsFromOtherMethodsJustFallbackToNextMiddleware(string $method): void { - $request = (new ServerRequest())->withMethod('GET'); - $delegate = $this->prophesize(RequestHandlerInterface::class); - /** @var MethodProphecy $process */ - $process = $delegate->handle($request)->willReturn(new Response()); - - $this->middleware->process($request, $delegate->reveal()); - - $process->shouldHaveBeenCalledOnce(); + $request = (new ServerRequest())->withMethod($method); + $this->assertHandlingRequestJustFallsBackToNext($request); } - /** - * @test - */ - public function jsonRequestsAreJsonDecoded() + public function provideIgnoredRequestMethods(): iterable + { + yield 'with GET' => ['GET']; + yield 'with HEAD' => ['HEAD']; + yield 'with OPTIONS' => ['OPTIONS']; + } + + /** @test */ + public function requestsWithNonEmptyBodyJustFallbackToNextMiddleware(): void + { + $request = (new ServerRequest())->withParsedBody(['foo' => 'bar'])->withMethod('POST'); + $this->assertHandlingRequestJustFallsBackToNext($request); + } + + private function assertHandlingRequestJustFallsBackToNext(ServerRequestInterface $request): void + { + $nextHandler = $this->prophesize(RequestHandlerInterface::class); + $handle = $nextHandler->handle($request)->willReturn(new Response()); + + $this->middleware->process($request, $nextHandler->reveal()); + + $handle->shouldHaveBeenCalledOnce(); + } + + /** @test */ + public function jsonRequestsAreJsonDecoded(): void { $test = $this; $body = new Stream('php://temp', 'wr'); @@ -71,10 +88,8 @@ class BodyParserMiddlewareTest extends TestCase $process->shouldHaveBeenCalledOnce(); } - /** - * @test - */ - public function regularRequestsAreUrlDecoded() + /** @test */ + public function regularRequestsAreUrlDecoded(): void { $test = $this; $body = new Stream('php://temp', 'wr');