Merge pull request #221 from clue-labs/coverage

SimonFrings · web-flow · commit cdef98b4cf54 · 2023-04-08T13:43:40.000+02:00
Update test suite to ensure 100% code coverage
diff --git 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: |
+          <?php
+          $metrics = simplexml_load_file('clover.xml')->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
@@ -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/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
@@ -10,6 +10,3 @@ parameters:
     ignoreErrors:
         # ignore generic usage like `PromiseInterface<ResponseInterface>` until fixed upstream
         - '/^PHPDoc tag @return contains generic type React\\Promise\\PromiseInterface<Psr\\Http\\Message\\ResponseInterface> 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/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">
     <testsuites>
         <testsuite name="Framework X test suite">
             <directory>./tests/</directory>
diff --git a/phpunit.xml.legacy b/phpunit.xml.legacy
@@ -4,7 +4,8 @@
 <phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/7.5/phpunit.xsd"
          bootstrap="vendor/autoload.php"
-         colors="true">
+         colors="true"
+         stderr="true">
     <testsuites>
         <testsuite name="Framework X test suite">
             <directory>./tests/</directory>
diff --git 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/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/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
diff --git a/tests/FiberStub.php b/tests/FiberStub.php
@@ -0,0 +1,156 @@
+<?php
+
+if (!class_exists(Fiber::class)) {
+    class Fiber
+    {
+        /** @var callable */
+        private $callback;
+
+        /** @var mixed */
+        private $return;
+
+        /** @var bool */
+        private $started = false;
+
+        /** @var bool */
+        private $terminated = false;
+
+        /** @var bool */
+        private static $halt = false;
+
+        /** @var ?Fiber */
+        private static $suspended = null;
+
+        public function __construct(callable $callback)
+        {
+            $this->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
diff --git a/tests/Io/SapiHandlerTest.php b/tests/Io/SapiHandlerTest.php

Original file line number	Diff line number	Diff line change
`@@ -30,6 +30,9 @@`
`30`	`30`	`"autoload-dev": {`
`31`	`31`	`"psr-4": {`
`32`	`32`	`"FrameworkX\\Tests\\": "tests/"`
`33`		`- }`
	`33`	`+ },`
	`34`	`+ "files": [`
	`35`	`+ "tests/FiberStub.php"`
	`36`	`+ ]`
`34`	`37`	`}`
`35`	`38`	`}`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ class FiberHandler`
`35`	`35`	`* be turned into a valid error response before returning.`
`36`	`36`	`* @throws void`
`37`	`37`	`*/`
`38`		`- public function __invoke(ServerRequestInterface $request, callable $next): mixed`
	`38`	`+ public function __invoke(ServerRequestInterface $request, callable $next)`
`39`	`39`	`{`
`40`	`40`	`$deferred = null;`
`41`	`41`	`$fiber = new \Fiber(function () use ($request, $next, &$deferred) {`