From b9c6b639181089a7236c925668fe0c74d2dbb083 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Tue, 4 Apr 2023 19:10:24 +0200 Subject: [PATCH 1/3] Add 100% code coverage for HTTP socket server --- src/App.php | 18 ++++------ tests/AppTest.php | 84 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 12 deletions(-) diff --git a/src/App.php b/src/App.php index 2799a62..9a03917 100644 --- a/src/App.php +++ b/src/App.php @@ -242,18 +242,11 @@ private function runLoop(): void $this->sapi->log('Listening on ' . \str_replace('tcp:', 'http:', (string) $socket->getAddress())); - $http->on('error', function (\Exception $e) { - $orig = $e; - $message = 'Error: ' . $e->getMessage(); - while (($e = $e->getPrevious()) !== null) { - $message .= '. Previous: ' . $e->getMessage(); - } - - $this->sapi->log($message); - - \fwrite(STDERR, (string)$orig); + $http->on('error', function (\Exception $e): void { + $this->sapi->log('HTTP error: ' . $e->getMessage()); }); + // @codeCoverageIgnoreStart try { Loop::addSignal(\defined('SIGINT') ? \SIGINT : 2, $f1 = function () use ($socket) { if (\PHP_VERSION_ID >= 70200 && \stream_isatty(\STDIN)) { @@ -270,9 +263,10 @@ private function runLoop(): void $socket->close(); Loop::stop(); }); - } catch (\BadMethodCallException $e) { // @codeCoverageIgnoreStart + } catch (\BadMethodCallException $e) { $this->sapi->log('Notice: No signal handler support, installing ext-ev or ext-pcntl recommended for production use.'); - } // @codeCoverageIgnoreEnd + } + // @codeCoverageIgnoreEnd do { Loop::run(); diff --git a/tests/AppTest.php b/tests/AppTest.php index 7a2218e..bc20eba 100644 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -30,6 +30,8 @@ use React\Promise\Deferred; use React\Promise\Promise; use React\Promise\PromiseInterface; +use React\Socket\ConnectionInterface; +use React\Socket\Connector; use ReflectionMethod; use ReflectionProperty; use function React\Async\await; @@ -718,6 +720,88 @@ public function testRunWillRestartLoopUntilSocketIsClosed(): void $app->run(); } + public function testRunWillListenForHttpRequestAndSendBackHttpResponseOverSocket(): void + { + $socket = stream_socket_server('127.0.0.1:0'); + assert(is_resource($socket)); + $addr = stream_socket_get_name($socket, false); + assert(is_string($addr)); + fclose($socket); + + $container = new Container([ + 'X_LISTEN' => $addr + ]); + + $app = new App($container); + + Loop::futureTick(function () use ($addr): void { + $connector = new Connector(); + $connector->connect($addr)->then(function (ConnectionInterface $connection): void { + $connection->on('data', function (string $data): void { + $this->assertStringStartsWith("HTTP/1.0 404 Not Found\r\n", $data); + }); + + // lovely: remove socket server on client connection close to terminate loop + $connection->on('close', function (): void { + $resources = get_resources(); + end($resources); + prev($resources); + $socket = prev($resources); + assert(is_resource($socket)); + + Loop::removeReadStream($socket); + fclose($socket); + + Loop::stop(); + }); + + $connection->write("GET /unknown HTTP/1.0\r\nHost: localhost\r\n\r\n"); + }); + }); + + $this->expectOutputRegex('/' . preg_quote('Listening on http://' . $addr . PHP_EOL, '/') . '.*/'); + $app->run(); + } + + public function testRunWillReportHttpErrorForInvalidClientRequest(): void + { + $socket = stream_socket_server('127.0.0.1:0'); + assert(is_resource($socket)); + $addr = stream_socket_get_name($socket, false); + assert(is_string($addr)); + fclose($socket); + + $container = new Container([ + 'X_LISTEN' => $addr + ]); + + $app = new App($container); + + Loop::futureTick(function () use ($addr): void { + $connector = new Connector(); + $connector->connect($addr)->then(function (ConnectionInterface $connection): void { + $connection->write("not a valid HTTP request\r\n\r\n"); + + // lovely: remove socket server on client connection close to terminate loop + $connection->on('close', function (): void { + $resources = get_resources(); + end($resources); + prev($resources); + $socket = prev($resources); + assert(is_resource($socket)); + + Loop::removeReadStream($socket); + fclose($socket); + + Loop::stop(); + }); + }); + }); + + $this->expectOutputRegex('/HTTP error: .*' . PHP_EOL . '$/'); + $app->run(); + } + /** * @requires function pcntl_signal * @requires function posix_kill From e450527cab7e2ad413c36fbfdd98644807e7b23b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sun, 26 Mar 2023 22:36:25 +0200 Subject: [PATCH 2/3] Add `FiberStub` to achieve 100% code coverage for PHP < 8.1 --- composer.json | 5 +- phpstan.neon.dist | 3 - src/Io/FiberHandler.php | 2 +- tests/FiberStub.php | 156 ++++++++++++++++++++++++++++++++++ tests/Io/FiberHandlerTest.php | 27 ++++-- 5 files changed, 181 insertions(+), 12 deletions(-) create mode 100644 tests/FiberStub.php diff --git a/composer.json b/composer.json index 29a74ff..d8eebf1 100644 --- a/composer.json +++ b/composer.json @@ -30,6 +30,9 @@ "autoload-dev": { "psr-4": { "FrameworkX\\Tests\\": "tests/" - } + }, + "files": [ + "tests/FiberStub.php" + ] } } diff --git a/phpstan.neon.dist b/phpstan.neon.dist index c4587eb..8acd846 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -10,6 +10,3 @@ parameters: ignoreErrors: # ignore generic usage like `PromiseInterface` until fixed upstream - '/^PHPDoc tag @return contains generic type React\\Promise\\PromiseInterface but interface React\\Promise\\PromiseInterface is not generic\.$/' - # ignore unknown `Fiber` class (PHP 8.1+) - - '/^Instantiated class Fiber not found\.$/' - - '/^Call to method (start|isTerminated|getReturn)\(\) on an unknown class Fiber\.$/' diff --git a/src/Io/FiberHandler.php b/src/Io/FiberHandler.php index 819360a..1a36740 100644 --- a/src/Io/FiberHandler.php +++ b/src/Io/FiberHandler.php @@ -35,7 +35,7 @@ class FiberHandler * be turned into a valid error response before returning. * @throws void */ - public function __invoke(ServerRequestInterface $request, callable $next): mixed + public function __invoke(ServerRequestInterface $request, callable $next) { $deferred = null; $fiber = new \Fiber(function () use ($request, $next, &$deferred) { diff --git a/tests/FiberStub.php b/tests/FiberStub.php new file mode 100644 index 0000000..c6c8222 --- /dev/null +++ b/tests/FiberStub.php @@ -0,0 +1,156 @@ +callback = $callback; + } + + /** + * @param mixed ...$args + * @return mixed + * @throws \Throwable + */ + public function start(...$args) + { + if ($this->started) { + throw new \FiberError(); + } + $this->started = true; + + if (self::$halt) { + assert(self::$suspended === null); + self::$suspended = $this; + return null; + } + + try { + return $this->return = ($this->callback)(...$args); + } finally { + $this->terminated = true; + } + } + + /** + * @param mixed $value + * @return mixed + * @throws \BadMethodCallException + */ + public function resume($value = null) + { + throw new \BadMethodCallException(); + } + + /** + * @param Throwable $exception + * @return mixed + * @throws \BadMethodCallException + */ + public function throw(Throwable $exception) + { + throw new \BadMethodCallException(); + } + + /** + * @return mixed + * @throws FiberError + */ + public function getReturn() + { + if (!$this->terminated) { + throw new \FiberError(); + } + + return $this->return; + } + + public function isStarted(): bool + { + return $this->started; + } + + public function isSuspended(): bool + { + return false; + } + + public function isRunning(): bool + { + return $this->started && !$this->terminated; + } + + public function isTerminated(): bool + { + return $this->terminated; + } + + /** + * @param mixed $value + * @return mixed + * @throws \Throwable + */ + public static function suspend($value = null) + { + throw new \BadMethodCallException(); + } + + public static function getCurrent(): ?Fiber + { + return null; + } + + /** + * @internal + */ + public static function mockSuspend(): void + { + assert(self::$halt === false); + self::$halt = true; + } + + /** + * @internal + * @throws void + */ + public static function mockResume(): void + { + assert(self::$halt === true); + assert(self::$suspended instanceof self); + + $fiber = self::$suspended; + assert($fiber->started); + assert(!$fiber->terminated); + + self::$halt = false; + self::$suspended = null; + + /** @throws void */ + $fiber->return = ($fiber->callback)(); + $fiber->terminated = true; + } + } + + final class FiberError extends Error { + + } +} diff --git a/tests/Io/FiberHandlerTest.php b/tests/Io/FiberHandlerTest.php index 15c2dd1..6b8d23c 100644 --- a/tests/Io/FiberHandlerTest.php +++ b/tests/Io/FiberHandlerTest.php @@ -14,13 +14,6 @@ class FiberHandlerTest extends TestCase { - public function setUp(): void - { - if (PHP_VERSION_ID < 80100 || !function_exists('React\Async\async')) { - $this->markTestSkipped('Requires PHP 8.1+ with react/async 4+'); - } - } - public function testInvokeWithHandlerReturningResponseReturnsSameResponse(): void { $handler = new FiberHandler(); @@ -132,6 +125,11 @@ public function testInvokeWithHandlerReturningResponseAfterAwaitingResolvedPromi public function testInvokeWithHandlerReturningResponseAfterAwaitingPendingPromiseReturnsPromiseResolvingWithSameResponse(): void { + // work around lack of actual fibers in PHP < 8.1 + if (\method_exists(\Fiber::class, 'mockSuspend')) { + \Fiber::mockSuspend(); + } + $handler = new FiberHandler(); $request = new ServerRequest('GET', 'http://example.com/'); @@ -140,6 +138,16 @@ public function testInvokeWithHandlerReturningResponseAfterAwaitingPendingPromis $deferred = new Deferred(); $promise = $handler($request, function () use ($deferred) { + // going the extra mile if using reactphp/async < 4 on PHP 8.1+ + if (PHP_VERSION_ID >= 80100 && !function_exists('React\Async\async')) { + $fiber = \Fiber::getCurrent(); + assert($fiber instanceof \Fiber); + $deferred->promise()->then(function () use ($fiber): void { + $fiber->resume(); + }); + \Fiber::suspend(); + } + return await($deferred->promise()); }); @@ -155,6 +163,11 @@ public function testInvokeWithHandlerReturningResponseAfterAwaitingPendingPromis $deferred->resolve($response); + // work around lack of actual fibers in PHP < 8.1 + if (\method_exists(\Fiber::class, 'mockResume')) { + \Fiber::mockResume(); + } + $this->assertSame($response, $ret); } } From d0470542567c9e07c45b68cdcd06e3bc75844f45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sun, 19 Mar 2023 19:41:26 +0100 Subject: [PATCH 3/3] Update test suite to ensure 100% code coverage --- .github/workflows/ci.yml | 10 ++++++++-- README.md | 11 ++++++++++- phpunit.xml.dist | 3 ++- phpunit.xml.legacy | 3 ++- tests/Io/SapiHandlerTest.php | 26 +++++++++++++------------- 5 files changed, 35 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7799c6b..4e0a72c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,10 +29,16 @@ jobs: coverage: xdebug ini-file: development - run: composer install - - run: vendor/bin/phpunit --coverage-text --stderr + - run: vendor/bin/phpunit --coverage-text --coverage-clover=clover.xml if: ${{ matrix.php >= 7.3 }} - - run: vendor/bin/phpunit --coverage-text --stderr -c phpunit.xml.legacy + - run: vendor/bin/phpunit --coverage-text --coverage-clover=clover.xml -c phpunit.xml.legacy if: ${{ matrix.php < 7.3 }} + - name: Check 100% code coverage + shell: php {0} + run: | + project->metrics; + exit((int) $metrics['statements'] === (int) $metrics['coveredstatements'] ? 0 : 1); PHPStan: name: PHPStan (PHP ${{ matrix.php }}) diff --git a/README.md b/README.md index 9f3d8c3..7f70d34 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,7 @@ # Framework X [![CI status](https://github.com/clue-access/framework-x/workflows/CI/badge.svg)](https://github.com/clue-access/framework-x/actions) +[![code coverage](https://img.shields.io/badge/code%20coverage-100%25-success)](#tests) Framework X – the simple and fast micro framework for building reactive web applications that run anywhere. @@ -115,7 +116,15 @@ $ composer install To run the test suite, go to the project root and run: ```bash -$ vendor/bin/phpunit --stderr +$ vendor/bin/phpunit +``` + +The test suite is set up to always ensure 100% code coverage across all +supported environments. If you have the Xdebug extension installed, you can also +generate a code coverage report locally like this: + +```bash +$ XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-text ``` Additionally, you can run some simple acceptance tests to verify the framework diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 26708a1..b995290 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -6,7 +6,8 @@ bootstrap="vendor/autoload.php" cacheResult="false" colors="true" - convertDeprecationsToExceptions="true"> + convertDeprecationsToExceptions="true" + stderr="true"> ./tests/ diff --git a/phpunit.xml.legacy b/phpunit.xml.legacy index 5bab1e8..a35d140 100644 --- a/phpunit.xml.legacy +++ b/phpunit.xml.legacy @@ -4,7 +4,8 @@ + colors="true" + stderr="true"> ./tests/ diff --git a/tests/Io/SapiHandlerTest.php b/tests/Io/SapiHandlerTest.php index 89c15c8..71a5f15 100644 --- a/tests/Io/SapiHandlerTest.php +++ b/tests/Io/SapiHandlerTest.php @@ -141,7 +141,7 @@ public function testRequestFromGlobalsWithConnectProxy(): void public function testSendResponseSendsEmptyResponseWithNoHeadersAndEmptyBodyAndAssignsNoContentTypeAndEmptyContentLength(): void { if (headers_sent() || !function_exists('xdebug_get_headers')) { - $this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled'); + $this->markTestSkipped('Test requires running PHPUnit with Xdebug enabled'); } header_remove(); @@ -158,7 +158,7 @@ public function testSendResponseSendsEmptyResponseWithNoHeadersAndEmptyBodyAndAs public function testSendResponseSendsJsonResponseWithGivenHeadersAndBodyAndAssignsMatchingContentLength(): void { if (headers_sent() || !function_exists('xdebug_get_headers')) { - $this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled'); + $this->markTestSkipped('Test requires running PHPUnit with Xdebug enabled'); } header_remove(); @@ -178,7 +178,7 @@ public function testSendResponseSendsJsonResponseWithGivenHeadersAndBodyAndAssig public function testSendResponseSendsJsonResponseWithGivenHeadersAndMatchingContentLengthButEmptyBodyForHeadRequest(): void { if (headers_sent() || !function_exists('xdebug_get_headers')) { - $this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled'); + $this->markTestSkipped('Test requires running PHPUnit with Xdebug enabled'); } header_remove(); @@ -196,7 +196,7 @@ public function testSendResponseSendsJsonResponseWithGivenHeadersAndMatchingCont public function testSendResponseSendsEmptyBodyWithGivenHeadersAndAssignsNoContentLengthForNoContentResponse(): void { if (headers_sent() || !function_exists('xdebug_get_headers')) { - $this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled'); + $this->markTestSkipped('Test requires running PHPUnit with Xdebug enabled'); } header_remove(); @@ -213,7 +213,7 @@ public function testSendResponseSendsEmptyBodyWithGivenHeadersAndAssignsNoConten public function testSendResponseSendsEmptyBodyWithGivenHeadersButWithoutExplicitContentLengthForNoContentResponse(): void { if (headers_sent() || !function_exists('xdebug_get_headers')) { - $this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled'); + $this->markTestSkipped('Test requires running PHPUnit with Xdebug enabled'); } header_remove(); @@ -230,7 +230,7 @@ public function testSendResponseSendsEmptyBodyWithGivenHeadersButWithoutExplicit public function testSendResponseSendsEmptyBodyWithGivenHeadersAndAssignsContentLengthForNotModifiedResponse(): void { if (headers_sent() || !function_exists('xdebug_get_headers')) { - $this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled'); + $this->markTestSkipped('Test requires running PHPUnit with Xdebug enabled'); } header_remove(); @@ -247,7 +247,7 @@ public function testSendResponseSendsEmptyBodyWithGivenHeadersAndAssignsContentL public function testSendResponseSendsEmptyBodyWithGivenHeadersAndExplicitContentLengthForNotModifiedResponse(): void { if (headers_sent() || !function_exists('xdebug_get_headers')) { - $this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled'); + $this->markTestSkipped('Test requires running PHPUnit with Xdebug enabled'); } header_remove(); @@ -264,7 +264,7 @@ public function testSendResponseSendsEmptyBodyWithGivenHeadersAndExplicitContent public function testSendResponseSendsStreamingResponseWithNoHeadersAndBodyFromStreamData(): void { if (headers_sent() || !function_exists('xdebug_get_headers')) { - $this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled'); + $this->markTestSkipped('Test requires running PHPUnit with Xdebug enabled'); } header_remove(); @@ -287,7 +287,7 @@ public function testSendResponseSendsStreamingResponseWithNoHeadersAndBodyFromSt public function testSendResponseClosesStreamingResponseAndSendsResponseWithNoHeadersAndBodyForHeadRequest(): void { if (headers_sent() || !function_exists('xdebug_get_headers')) { - $this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled'); + $this->markTestSkipped('Test requires running PHPUnit with Xdebug enabled'); } header_remove(); @@ -307,7 +307,7 @@ public function testSendResponseClosesStreamingResponseAndSendsResponseWithNoHea public function testSendResponseClosesStreamingResponseAndSendsResponseWithNoHeadersAndBodyForNotModifiedResponse(): void { if (headers_sent() || !function_exists('xdebug_get_headers')) { - $this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled'); + $this->markTestSkipped('Test requires running PHPUnit with Xdebug enabled'); } header_remove(); @@ -326,7 +326,7 @@ public function testSendResponseClosesStreamingResponseAndSendsResponseWithNoHea public function testSendResponseClosesStreamingResponseAndSendsResponseWithNoHeadersAndBodyForNoContentResponse(): void { if (headers_sent() || !function_exists('xdebug_get_headers')) { - $this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled'); + $this->markTestSkipped('Test requires running PHPUnit with Xdebug enabled'); } header_remove(); @@ -345,7 +345,7 @@ public function testSendResponseClosesStreamingResponseAndSendsResponseWithNoHea public function testSendResponseSendsStreamingResponseWithNoHeadersAndBodyFromStreamDataAndNoBufferHeaderForNginxServer(): void { if (headers_sent() || !function_exists('xdebug_get_headers')) { - $this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled'); + $this->markTestSkipped('Test requires running PHPUnit with Xdebug enabled'); } header_remove(); @@ -366,7 +366,7 @@ public function testSendResponseSendsStreamingResponseWithNoHeadersAndBodyFromSt public function testSendResponseSetsMultipleCookieHeaders(): void { if (headers_sent() || !function_exists('xdebug_get_headers')) { - $this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled'); + $this->markTestSkipped('Test requires running PHPUnit with Xdebug enabled'); } header_remove();