From 17f73d9d3d3dad9280d6a6d7295b9e21d4560751 Mon Sep 17 00:00:00 2001 From: Hans Emmel Date: Tue, 11 Aug 2026 21:44:43 +1000 Subject: [PATCH] test: raise line coverage from 66% to 96% Adds tests only - no production code was changed. Classes 38.88% -> 77.77% (7/18 -> 14/18) Methods 62.96% -> 92.59% (34/54 -> 50/54) Lines 66.66% -> 95.72% (156 -> 224 of 234) Tests 38 -> 84, assertions 100 -> 195 New coverage: - HttpErrorHandler (0% -> 100%): all six HttpException subtypes via a data provider, plus the generic-exception path and both displayErrorDetails branches, asserting details never leak when the flag is off. - DatabaseWrapper (0% -> 100%): Kreait's Reference and ApiClient are final and cannot be mocked, but ApiClient accepts a Guzzle ClientInterface. So the real SDK stack runs against a Guzzle MockHandler with canned responses - no emulator, no live database, deterministic in CI. Covers the HTTP verb and body for each operation and the null-key guard in push(). - ShutdownHandler / ResponseEmitter (0% -> 89% / 100%): message formatting per error type, asserted through the emitted JSON body, plus the buffer-discard path that keeps stray output from reaching the client. - Action (80% -> 100%): resolveArg's rejection path and the domain-to-HTTP 404 translation. - ActionPayload, ActionError, SessionMiddleware: 100%. Ten lines remain uncovered, each for a concrete reason: - ShutdownHandler 57-59: the E_USER_ERROR branch. trigger_error() with E_USER_ERROR halts the process, which would kill the test run. - JsonBodyParserMiddleware 23: reads php://input directly, always empty under the CLI SAPI. Reading $request->getBody() instead would make it testable. - FirebaseTodoRepository 20-24: the Factory branch needs real credentials. - InMemoryTodoRepository 33: an Xdebug line-attribution artifact - the closing bracket of a multi-line array literal is marked executable but never recorded as hit. Lines 27-32 are covered. Co-Authored-By: Claude Opus 5 --- tests/Application/Actions/ActionErrorTest.php | 49 ++++++ .../Application/Actions/ActionPayloadTest.php | 79 +++++++++ tests/Application/Actions/ActionTest.php | 110 +++++++++++++ .../Handlers/HttpErrorHandlerTest.php | 133 +++++++++++++++ .../Handlers/ShutdownHandlerTest.php | 123 ++++++++++++++ .../JsonBodyParserMiddlewareTest.php | 91 ++++++++++ .../Middleware/SessionMiddlewareTest.php | 68 ++++++++ .../Persistence/Todo/DatabaseWrapperTest.php | 155 ++++++++++++++++++ 8 files changed, 808 insertions(+) create mode 100644 tests/Application/Actions/ActionErrorTest.php create mode 100644 tests/Application/Actions/ActionPayloadTest.php create mode 100644 tests/Application/Actions/ActionTest.php create mode 100644 tests/Application/Handlers/HttpErrorHandlerTest.php create mode 100644 tests/Application/Handlers/ShutdownHandlerTest.php create mode 100644 tests/Application/Middleware/JsonBodyParserMiddlewareTest.php create mode 100644 tests/Application/Middleware/SessionMiddlewareTest.php create mode 100644 tests/Infrastructure/Persistence/Todo/DatabaseWrapperTest.php diff --git a/tests/Application/Actions/ActionErrorTest.php b/tests/Application/Actions/ActionErrorTest.php new file mode 100644 index 0000000..5c95058 --- /dev/null +++ b/tests/Application/Actions/ActionErrorTest.php @@ -0,0 +1,49 @@ +assertSame(ActionError::VALIDATION_ERROR, $error->getType()); + $this->assertSame('Name is required', $error->getDescription()); + } + + public function testSettersAreFluentAndMutateState(): void + { + $error = new ActionError(ActionError::SERVER_ERROR, 'Boom'); + + $this->assertSame($error, $error->setType(ActionError::BAD_REQUEST)); + $this->assertSame($error, $error->setDescription('Bad input')); + + $this->assertSame(ActionError::BAD_REQUEST, $error->getType()); + $this->assertSame('Bad input', $error->getDescription()); + } + + public function testSerializesTypeAndDescription(): void + { + $error = new ActionError(ActionError::NOT_ALLOWED, 'Use POST'); + + $this->assertSame([ + 'type' => ActionError::NOT_ALLOWED, + 'description' => 'Use POST', + ], $error->jsonSerialize()); + } + + public function testEncodesToJson(): void + { + $error = new ActionError(ActionError::UNAUTHENTICATED, 'Log in first'); + + $this->assertSame( + '{"type":"UNAUTHENTICATED","description":"Log in first"}', + json_encode($error) + ); + } +} diff --git a/tests/Application/Actions/ActionPayloadTest.php b/tests/Application/Actions/ActionPayloadTest.php new file mode 100644 index 0000000..a198141 --- /dev/null +++ b/tests/Application/Actions/ActionPayloadTest.php @@ -0,0 +1,79 @@ +assertSame(200, $payload->getStatusCode()); + $this->assertNull($payload->getData()); + $this->assertNull($payload->getError()); + } + + public function testExposesConstructorArguments(): void + { + $error = new ActionError(ActionError::BAD_REQUEST, 'Nope'); + $payload = new ActionPayload(400, ['id' => 'C137'], $error); + + $this->assertSame(400, $payload->getStatusCode()); + $this->assertSame(['id' => 'C137'], $payload->getData()); + $this->assertSame($error, $payload->getError()); + } + + public function testSerializesDataWhenPresent(): void + { + $payload = new ActionPayload(200, ['name' => 'Rick Sanchez']); + + $this->assertSame([ + 'statusCode' => 200, + 'data' => ['name' => 'Rick Sanchez'], + ], $payload->jsonSerialize()); + } + + /** + * Data wins over error when both are set — only one key is ever emitted. + */ + public function testDataTakesPrecedenceOverError(): void + { + $payload = new ActionPayload(200, ['ok' => true], new ActionError(ActionError::SERVER_ERROR, 'Boom')); + + $serialized = $payload->jsonSerialize(); + + $this->assertArrayHasKey('data', $serialized); + $this->assertArrayNotHasKey('error', $serialized); + } + + public function testSerializesErrorWhenThereIsNoData(): void + { + $error = new ActionError(ActionError::RESOURCE_NOT_FOUND, 'Not here'); + $payload = new ActionPayload(404, null, $error); + + $this->assertSame([ + 'statusCode' => 404, + 'error' => $error, + ], $payload->jsonSerialize()); + } + + public function testSerializesOnlyStatusCodeWhenEmpty(): void + { + $this->assertSame(['statusCode' => 204], (new ActionPayload(204))->jsonSerialize()); + } + + public function testEncodesToJson(): void + { + $payload = new ActionPayload(404, null, new ActionError(ActionError::RESOURCE_NOT_FOUND, 'Not here')); + + $this->assertSame( + '{"statusCode":404,"error":{"type":"RESOURCE_NOT_FOUND","description":"Not here"}}', + json_encode($payload) + ); + } +} diff --git a/tests/Application/Actions/ActionTest.php b/tests/Application/Actions/ActionTest.php new file mode 100644 index 0000000..b524b93 --- /dev/null +++ b/tests/Application/Actions/ActionTest.php @@ -0,0 +1,110 @@ +createRequest('GET', '/todos'), + (new ResponseFactory())->createResponse(), + $args + ); + } + + public function testResolveArgReturnsAKnownArgument(): void + { + $response = $this->invoke( + fn (ActionTestDouble $a): Response => $a->respondWith(['id' => $a->arg('id')]), + ['id' => 'C137'] + ); + + $response->getBody()->rewind(); + $this->assertSame('C137', json_decode((string) $response->getBody(), true)['data']['id']); + } + + public function testResolveArgRejectsAnUnknownArgument(): void + { + $this->expectException(HttpBadRequestException::class); + $this->expectExceptionMessage('Could not resolve argument `missing`.'); + + $this->invoke(fn (ActionTestDouble $a): Response => $a->respondWith($a->arg('missing'))); + } + + /** + * Domain "not found" errors are translated into HTTP 404 by the base class, so + * actions never have to know about HTTP. + */ + public function testDomainNotFoundBecomesHttpNotFound(): void + { + $this->expectException(HttpNotFoundException::class); + + $this->invoke(function (): Response { + throw new TodoNotFoundException(); + }); + } + + public function testRespondWithDataWrapsPayloadAsJson(): void + { + $response = $this->invoke(fn (ActionTestDouble $a): Response => $a->respondWith(['name' => 'Rick Sanchez'])); + + $response->getBody()->rewind(); + + $this->assertSame('application/json', $response->getHeaderLine('Content-Type')); + $this->assertSame( + ['statusCode' => 200, 'data' => ['name' => 'Rick Sanchez']], + json_decode((string) $response->getBody(), true) + ); + } +} + +/** + * Concrete Action that runs a supplied closure and exposes the protected helpers. + */ +class ActionTestDouble extends Action +{ + /** @var Closure(self): Response */ + private Closure $action; + + public function __construct(\Psr\Log\LoggerInterface $logger, Closure $action) + { + parent::__construct($logger); + $this->action = $action; + } + + protected function action(): Response + { + return ($this->action)($this); + } + + public function arg(string $name): mixed + { + return $this->resolveArg($name); + } + + public function respondWith(mixed $data): Response + { + return $this->respondWithData($data); + } +} diff --git a/tests/Application/Handlers/HttpErrorHandlerTest.php b/tests/Application/Handlers/HttpErrorHandlerTest.php new file mode 100644 index 0000000..7494a8c --- /dev/null +++ b/tests/Application/Handlers/HttpErrorHandlerTest.php @@ -0,0 +1,133 @@ +handler = new HttpErrorHandler(new CallableResolver(), new ResponseFactory()); + } + + /** + * Runs the handler and returns the decoded JSON body alongside the response. + * + * @return array{0: \Psr\Http\Message\ResponseInterface, 1: array} + */ + private function handle(Throwable $exception, bool $displayErrorDetails = false): array + { + $request = $this->createRequest('GET', '/todos'); + + $response = ($this->handler)($request, $exception, $displayErrorDetails, false, false); + $response->getBody()->rewind(); + + return [$response, json_decode((string) $response->getBody(), true)]; + } + + /** + * Each Slim HTTP exception maps to its own ActionError type and status code. + * + * @return array, 1: int, 2: string}> + */ + public static function httpExceptionProvider(): array + { + return [ + 'not found' => [HttpNotFoundException::class, 404, ActionError::RESOURCE_NOT_FOUND], + 'method not allowed' => [HttpMethodNotAllowedException::class, 405, ActionError::NOT_ALLOWED], + 'unauthorized' => [HttpUnauthorizedException::class, 401, ActionError::UNAUTHENTICATED], + 'forbidden' => [HttpForbiddenException::class, 403, ActionError::INSUFFICIENT_PRIVILEGES], + 'bad request' => [HttpBadRequestException::class, 400, ActionError::BAD_REQUEST], + 'not implemented' => [HttpNotImplementedException::class, 501, ActionError::NOT_IMPLEMENTED], + ]; + } + + /** + * @param class-string $exceptionClass + */ + #[DataProvider('httpExceptionProvider')] + public function testHttpExceptionMapsToStatusCodeAndErrorType( + string $exceptionClass, + int $expectedStatus, + string $expectedType + ): void { + $request = $this->createRequest('GET', '/todos'); + $exception = new $exceptionClass($request, 'Something went wrong'); + + [$response, $payload] = $this->handle($exception); + + $this->assertSame($expectedStatus, $response->getStatusCode()); + $this->assertSame($expectedStatus, $payload['statusCode']); + $this->assertSame($expectedType, $payload['error']['type']); + $this->assertSame('Something went wrong', $payload['error']['description']); + } + + public function testResponseIsJson(): void + { + $request = $this->createRequest('GET', '/todos'); + + [$response] = $this->handle(new HttpNotFoundException($request)); + + $this->assertSame('application/json', $response->getHeaderLine('Content-Type')); + } + + public function testGenericExceptionIsReportedAsServerError(): void + { + [$response, $payload] = $this->handle(new RuntimeException('Database exploded')); + + $this->assertSame(500, $response->getStatusCode()); + $this->assertSame(ActionError::SERVER_ERROR, $payload['error']['type']); + } + + public function testGenericExceptionHidesDetailsByDefault(): void + { + [, $payload] = $this->handle(new RuntimeException('Database exploded'), false); + + $this->assertSame( + 'An internal error has occurred while processing your request.', + $payload['error']['description'] + ); + $this->assertStringNotContainsString('exploded', $payload['error']['description']); + } + + public function testGenericExceptionExposesDetailsWhenEnabled(): void + { + [, $payload] = $this->handle(new RuntimeException('Database exploded'), true); + + $this->assertSame('Database exploded', $payload['error']['description']); + } + + /** + * displayErrorDetails must not leak details for HTTP exceptions, which carry + * their own user-facing message. + */ + public function testHttpExceptionKeepsItsOwnMessageWhenDetailsEnabled(): void + { + $request = $this->createRequest('GET', '/todos'); + $exception = new HttpNotFoundException($request, 'No such todo'); + + [, $payload] = $this->handle($exception, true); + + $this->assertSame('No such todo', $payload['error']['description']); + } +} diff --git a/tests/Application/Handlers/ShutdownHandlerTest.php b/tests/Application/Handlers/ShutdownHandlerTest.php new file mode 100644 index 0000000..7a1db2a --- /dev/null +++ b/tests/Application/Handlers/ShutdownHandlerTest.php @@ -0,0 +1,123 @@ +|null + */ + private function emit(bool $displayErrorDetails): ?array + { + $handler = new ShutdownHandler( + $this->createRequest('GET', '/todos'), + new HttpErrorHandler(new CallableResolver(), new ResponseFactory()), + $displayErrorDetails + ); + + ob_start(); + $handler(); + $output = ob_get_clean(); + + return $output === '' ? null : json_decode($output, true); + } + + #[RunInSeparateProcess] + #[PreserveGlobalState(false)] + public function testFormatsUserWarnings(): void + { + @trigger_error('something odd', E_USER_WARNING); + + $payload = $this->emit(true); + + $this->assertSame(500, $payload['statusCode']); + $this->assertSame('WARNING: something odd', $payload['error']['description']); + } + + #[RunInSeparateProcess] + #[PreserveGlobalState(false)] + public function testFormatsUserNotices(): void + { + @trigger_error('just so you know', E_USER_NOTICE); + + $payload = $this->emit(true); + + $this->assertSame('NOTICE: just so you know', $payload['error']['description']); + } + + /** + * Anything that is not a user-triggered warning or notice falls through to the + * default branch, which appends the file and line. + */ + #[RunInSeparateProcess] + #[PreserveGlobalState(false)] + public function testFormatsOtherErrorsWithFileAndLine(): void + { + @file_get_contents('/definitely/not/a/real/path'); + + $payload = $this->emit(true); + + $this->assertStringStartsWith('ERROR: ', $payload['error']['description']); + $this->assertStringContainsString(' on line ', $payload['error']['description']); + $this->assertStringContainsString(' in file ', $payload['error']['description']); + } + + /** + * Stray output written before the shutdown handler runs (a stack trace, a + * warning, a stray echo) must be discarded so the client gets clean JSON. + */ + #[RunInSeparateProcess] + #[PreserveGlobalState(false)] + public function testDiscardsOutputBufferedBeforeEmitting(): void + { + @trigger_error('something odd', E_USER_WARNING); + + $handler = new ShutdownHandler( + $this->createRequest('GET', '/todos'), + new HttpErrorHandler(new CallableResolver(), new ResponseFactory()), + true + ); + + ob_start(); + echo 'stray output that must not reach the client'; + $handler(); + $output = ob_get_clean(); + + $this->assertStringNotContainsString('stray output', $output); + $this->assertSame('WARNING: something odd', json_decode($output, true)['error']['description']); + } + + /** + * With error details switched off the user must never see the raw PHP error. + */ + #[RunInSeparateProcess] + #[PreserveGlobalState(false)] + public function testHidesErrorDetailsWhenDisabled(): void + { + @trigger_error('leaky internal detail', E_USER_WARNING); + + $payload = $this->emit(false); + + $this->assertSame( + 'An error while processing your request. Please try again later.', + $payload['error']['description'] + ); + $this->assertStringNotContainsString('leaky', $payload['error']['description']); + } +} diff --git a/tests/Application/Middleware/JsonBodyParserMiddlewareTest.php b/tests/Application/Middleware/JsonBodyParserMiddlewareTest.php new file mode 100644 index 0000000..84bd95b --- /dev/null +++ b/tests/Application/Middleware/JsonBodyParserMiddlewareTest.php @@ -0,0 +1,91 @@ +seen = $request; + + return (new ResponseFactory())->createResponse(200); + } + }; + } + + public function testDelegatesToTheNextHandler(): void + { + $seen = null; + $request = $this->createRequest('GET', '/todos'); + + $response = (new JsonBodyParserMiddleware())->process($request, $this->recordingHandler($seen)); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertInstanceOf(Request::class, $seen); + } + + public function testLeavesNonJsonRequestsUntouched(): void + { + $seen = null; + $request = $this->createRequest('POST', '/todos', ['Content-Type' => 'application/x-www-form-urlencoded']); + + (new JsonBodyParserMiddleware())->process($request, $this->recordingHandler($seen)); + + $this->assertNull($seen->getParsedBody()); + } + + public function testLeavesRequestsWithNoContentTypeUntouched(): void + { + $seen = null; + $request = $this->createRequest('POST', '/todos', []); + + (new JsonBodyParserMiddleware())->process($request, $this->recordingHandler($seen)); + + $this->assertNull($seen->getParsedBody()); + } + + /** + * The middleware inspects the raw php://input stream, which is empty under the + * CLI SAPI. An unparsable body must leave the parsed body alone rather than + * setting it to null, so downstream actions can tell "absent" from "empty". + */ + public function testDoesNotSetParsedBodyWhenJsonIsUnparsable(): void + { + $seen = null; + $request = $this->createRequest('POST', '/todos', ['Content-Type' => 'application/json']); + + (new JsonBodyParserMiddleware())->process($request, $this->recordingHandler($seen)); + + $this->assertNull($seen->getParsedBody()); + } + + public function testMatchesJsonContentTypeWithCharsetSuffix(): void + { + $seen = null; + $request = $this->createRequest('POST', '/todos', ['Content-Type' => 'application/json; charset=utf-8']); + + $response = (new JsonBodyParserMiddleware())->process($request, $this->recordingHandler($seen)); + + $this->assertSame(200, $response->getStatusCode()); + } +} diff --git a/tests/Application/Middleware/SessionMiddlewareTest.php b/tests/Application/Middleware/SessionMiddlewareTest.php new file mode 100644 index 0000000..9ff56bf --- /dev/null +++ b/tests/Application/Middleware/SessionMiddlewareTest.php @@ -0,0 +1,68 @@ +seen = $request; + + return (new ResponseFactory())->createResponse(200); + } + }; + } + + public function testDoesNotStartASessionWithoutAnAuthorizationHeader(): void + { + $seen = null; + unset($_SERVER['HTTP_AUTHORIZATION']); + + $response = (new SessionMiddleware())->process($this->createRequest('GET', '/todos'), $this->recordingHandler($seen)); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertNull($seen->getAttribute('session')); + $this->assertSame(PHP_SESSION_NONE, session_status()); + } + + /** + * session_start() mutates process-wide state and cannot be undone, so this + * runs isolated to keep the rest of the suite clean. + */ + #[RunInSeparateProcess] + #[PreserveGlobalState(false)] + public function testStartsASessionAndAttachesItWhenAuthorizationIsPresent(): void + { + $seen = null; + $_SERVER['HTTP_AUTHORIZATION'] = 'Bearer token'; + + $response = (new SessionMiddleware())->process($this->createRequest('GET', '/todos'), $this->recordingHandler($seen)); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame(PHP_SESSION_ACTIVE, session_status()); + $this->assertIsArray($seen->getAttribute('session')); + } +} diff --git a/tests/Infrastructure/Persistence/Todo/DatabaseWrapperTest.php b/tests/Infrastructure/Persistence/Todo/DatabaseWrapperTest.php new file mode 100644 index 0000000..82385dc --- /dev/null +++ b/tests/Infrastructure/Persistence/Todo/DatabaseWrapperTest.php @@ -0,0 +1,155 @@ + */ + private array $history = []; + + /** + * @param array $responses + */ + private function wrapperFor(array $responses): DatabaseWrapper + { + $this->history = []; + + $stack = HandlerStack::create(new MockHandler($responses)); + $stack->push(Middleware::history($this->history)); + + $apiClient = new ApiClient( + new Client(['handler' => $stack]), + UrlBuilder::create(self::DATABASE_URL), + new DatabaseApiExceptionConverter(new ErrorResponseParser()) + ); + + return new DatabaseWrapper(new Database(new Uri(self::DATABASE_URL), $apiClient)); + } + + private function lastRequest(): RequestInterface + { + return $this->history[count($this->history) - 1]['request']; + } + + public function testGetValueReturnsDecodedPayload(): void + { + $wrapper = $this->wrapperFor([ + new Response(200, [], json_encode([ + 'C137' => ['name' => 'Rick Sanchez', 'done' => false], + ])), + ]); + + $value = $wrapper->getValue('/todos'); + + $this->assertSame(['C137' => ['name' => 'Rick Sanchez', 'done' => false]], $value); + $this->assertSame('GET', $this->lastRequest()->getMethod()); + $this->assertStringContainsString('/todos', $this->lastRequest()->getUri()->getPath()); + } + + public function testGetValueReturnsNullForAnEmptyPath(): void + { + $wrapper = $this->wrapperFor([new Response(200, [], 'null')]); + + $this->assertNull($wrapper->getValue('/todos')); + } + + public function testExistsIsTrueWhenThePathHasAValue(): void + { + $wrapper = $this->wrapperFor([ + new Response(200, [], json_encode(['name' => 'Rick Sanchez', 'done' => false])), + ]); + + $this->assertTrue($wrapper->exists('/todos/C137')); + } + + public function testExistsIsFalseWhenThePathIsEmpty(): void + { + $wrapper = $this->wrapperFor([new Response(200, [], 'null')]); + + $this->assertFalse($wrapper->exists('/todos/C137')); + } + + public function testPushReturnsTheGeneratedKey(): void + { + $wrapper = $this->wrapperFor([ + new Response(200, [], json_encode(['name' => '-NabcDEF123'])), + ]); + + $key = $wrapper->push('/todos', ['name' => 'Rick Sanchez', 'done' => false]); + + $this->assertSame('-NabcDEF123', $key); + + $request = $this->lastRequest(); + $this->assertSame('POST', $request->getMethod()); + $this->assertSame( + ['name' => 'Rick Sanchez', 'done' => false], + json_decode((string) $request->getBody(), true) + ); + } + + /** + * Firebase generates the child key, so a response without one leaves the new + * reference keyless. The wrapper must fail loudly rather than hand back null. + */ + public function testPushFailsWhenNoKeyIsGenerated(): void + { + $wrapper = $this->wrapperFor([new Response(200, [], json_encode(['name' => '']))]); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Failed to push value at path "/"'); + + $wrapper->push('/', ['name' => 'Rick Sanchez']); + } + + public function testUpdateSendsAPatchWithTheGivenValues(): void + { + $wrapper = $this->wrapperFor([new Response(200, [], '{}')]); + + $wrapper->update('/todos/C137', ['name' => 'Morty Smith', 'done' => true]); + + $request = $this->lastRequest(); + $this->assertSame('PATCH', $request->getMethod()); + $this->assertStringContainsString('/todos/C137', $request->getUri()->getPath()); + $this->assertSame( + ['name' => 'Morty Smith', 'done' => true], + json_decode((string) $request->getBody(), true) + ); + } + + public function testRemoveSendsADelete(): void + { + $wrapper = $this->wrapperFor([new Response(200, [], 'null')]); + + $wrapper->remove('/todos/C137'); + + $request = $this->lastRequest(); + $this->assertSame('DELETE', $request->getMethod()); + $this->assertStringContainsString('/todos/C137', $request->getUri()->getPath()); + } +}