From 965551532d684456b74baf960157c0c2cef7335d Mon Sep 17 00:00:00 2001 From: Brett Parson Date: Sat, 29 Aug 2026 15:56:31 -0500 Subject: Add regression and integration test suite Port the stress-test findings into PHPUnit: realistic brett-parson.com route model, known-issue baselines, router boundary cases, and a standalone scaling benchmark (composer bench). --- README.md | 3 +- composer.json | 6 + tests/Fixtures/AdminLoggingMiddleware.php | 31 +++++ tests/Fixtures/CollidingController.php | 21 ++++ tests/Fixtures/CsrfLoggingMiddleware.php | 30 +++++ tests/Fixtures/DuplicateParamController.php | 21 ++++ tests/Fixtures/MixedVisibilityController.php | 27 +++++ tests/Fixtures/OnlyGetController.php | 21 ++++ tests/Fixtures/RealisticAdminController.php | 98 ++++++++++++++++ tests/Fixtures/RealisticPublicController.php | 72 ++++++++++++ tests/Fixtures/StatefulController.php | 23 ++++ tests/Fixtures/StatefulMiddleware.php | 26 +++++ tests/Integration/RealisticRouteTableTest.php | 153 +++++++++++++++++++++++++ tests/Regression/KnownIssuesTest.php | 158 ++++++++++++++++++++++++++ tests/RouterTest.php | 61 ++++++++++ tests/bench/bench.php | 107 +++++++++++++++++ 16 files changed, 857 insertions(+), 1 deletion(-) create mode 100644 tests/Fixtures/AdminLoggingMiddleware.php create mode 100644 tests/Fixtures/CollidingController.php create mode 100644 tests/Fixtures/CsrfLoggingMiddleware.php create mode 100644 tests/Fixtures/DuplicateParamController.php create mode 100644 tests/Fixtures/MixedVisibilityController.php create mode 100644 tests/Fixtures/OnlyGetController.php create mode 100644 tests/Fixtures/RealisticAdminController.php create mode 100644 tests/Fixtures/RealisticPublicController.php create mode 100644 tests/Fixtures/StatefulController.php create mode 100644 tests/Fixtures/StatefulMiddleware.php create mode 100644 tests/Integration/RealisticRouteTableTest.php create mode 100644 tests/Regression/KnownIssuesTest.php create mode 100644 tests/bench/bench.php diff --git a/README.md b/README.md index 97be075..f5b75a5 100644 --- a/README.md +++ b/README.md @@ -95,7 +95,8 @@ configuration loader. Those stay application concerns. ```bash composer install -vendor/bin/phpunit +composer check # lint + unit tests +composer bench # registration/dispatch scaling benchmark ``` Or, without Composer, drop a `phpunit.phar` into the repo root and run diff --git a/composer.json b/composer.json index e9f55d1..dcda42e 100644 --- a/composer.json +++ b/composer.json @@ -19,6 +19,12 @@ "require-dev": { "phpunit/phpunit": "^10.5 || ^11.0" }, + "scripts": { + "test": "phpunit", + "bench": "php tests/bench/bench.php", + "lint": "find src tests -name '*.php' -exec php -l {} +", + "check": ["@lint", "@test"] + }, "config": { "sort-packages": true } diff --git a/tests/Fixtures/AdminLoggingMiddleware.php b/tests/Fixtures/AdminLoggingMiddleware.php new file mode 100644 index 0000000..8613475 --- /dev/null +++ b/tests/Fixtures/AdminLoggingMiddleware.php @@ -0,0 +1,31 @@ +log->append('admin:before'); + $response = $next($request); + $this->log->append('admin:after'); + + return $response; + } +} diff --git a/tests/Fixtures/CollidingController.php b/tests/Fixtures/CollidingController.php new file mode 100644 index 0000000..434ffbf --- /dev/null +++ b/tests/Fixtures/CollidingController.php @@ -0,0 +1,21 @@ +log->append('csrf:before'); + $response = $next($request); + $this->log->append('csrf:after'); + + return $response; + } +} diff --git a/tests/Fixtures/DuplicateParamController.php b/tests/Fixtures/DuplicateParamController.php new file mode 100644 index 0000000..8ea3060 --- /dev/null +++ b/tests/Fixtures/DuplicateParamController.php @@ -0,0 +1,21 @@ +param('id')); + } + + #[Post('/admin/notes')] + public function notesStore(): ResponseInterface + { + return new TestResponse(200, 'notes-store'); + } + + #[Post('/admin/notes/{id}')] + public function notesUpdate(RequestInterface $request): ResponseInterface + { + return new TestResponse(200, 'notes-update:' . $request->param('id')); + } + + #[Post('/admin/notes/{id}/delete')] + public function notesDelete(): ResponseInterface + { + return new TestResponse(200, 'notes-delete'); + } + + #[Post('/admin/notes/{id}/restore')] + public function notesRestore(): ResponseInterface + { + return new TestResponse(200, 'notes-restore'); + } + + #[Get('/admin/entries/new')] + public function entriesNew(): ResponseInterface + { + return new TestResponse(200, 'entries-new'); + } + + #[Post('/admin/entries')] + public function entriesStore(): ResponseInterface + { + return new TestResponse(200, 'entries-store'); + } + + #[Post('/admin/entries/preview')] + public function entriesPreview(): ResponseInterface + { + return new TestResponse(200, 'entries-preview-literal'); + } + + #[Get('/admin/entries/{id}/edit')] + public function entriesEdit(RequestInterface $request): ResponseInterface + { + return new TestResponse(200, 'entries-edit:' . $request->param('id')); + } + + #[Post('/admin/entries/{id}')] + public function entriesUpdate(): ResponseInterface + { + return new TestResponse(200, 'entries-update'); + } +} diff --git a/tests/Fixtures/RealisticPublicController.php b/tests/Fixtures/RealisticPublicController.php new file mode 100644 index 0000000..992b17b --- /dev/null +++ b/tests/Fixtures/RealisticPublicController.php @@ -0,0 +1,72 @@ +param('slug')); + } + + #[Get('/feed.xml')] + public function feed(): ResponseInterface + { + return new TestResponse(200, 'feed'); + } + + #[Get('/robots.txt')] + public function robots(): ResponseInterface + { + return new TestResponse(200, 'robots'); + } + + #[Get('/sitemap.xml')] + public function sitemap(): ResponseInterface + { + return new TestResponse(200, 'sitemap'); + } + + #[Get('/notes')] + public function notes(): ResponseInterface + { + return new TestResponse(200, 'notes'); + } + + #[Get('/notes/{id}')] + public function noteShow(RequestInterface $request): ResponseInterface + { + return new TestResponse(200, 'note-show:' . $request->param('id')); + } +} diff --git a/tests/Fixtures/StatefulController.php b/tests/Fixtures/StatefulController.php new file mode 100644 index 0000000..bd09592 --- /dev/null +++ b/tests/Fixtures/StatefulController.php @@ -0,0 +1,23 @@ +count++; + + return $next($request); + } +} diff --git a/tests/Integration/RealisticRouteTableTest.php b/tests/Integration/RealisticRouteTableTest.php new file mode 100644 index 0000000..f93896c --- /dev/null +++ b/tests/Integration/RealisticRouteTableTest.php @@ -0,0 +1,153 @@ +log = new ArrayObject(); + $this->resolver = new FakeResolver(); + $this->resolver->bind(RealisticPublicController::class, new RealisticPublicController()); + $this->resolver->bind(RealisticAdminController::class, new RealisticAdminController()); + $this->resolver->bind(AdminLoggingMiddleware::class, new AdminLoggingMiddleware($this->log)); + $this->resolver->bind(CsrfLoggingMiddleware::class, new CsrfLoggingMiddleware($this->log)); + + $this->router = new Router($this->resolver); + $this->router->group('/admin', [AdminLoggingMiddleware::class]); + $this->router->group('/admin', [CsrfLoggingMiddleware::class], ['POST']); + $this->router->registerController(RealisticPublicController::class); + $this->router->registerController(RealisticAdminController::class); + } + + public function testRouteTableCompilesToTwentyTwoRoutes(): void + { + $loaded = count((new RouteLoader())->load(RealisticPublicController::class)) + + count((new RouteLoader())->load(RealisticAdminController::class)); + + $this->assertSame(22, $loaded); + } + + public function testLiteralPreviewBeatsParameterizedId(): void + { + $this->assertSame( + 'entries-preview-literal', + $this->dispatch('POST', '/admin/entries/preview')->body(), + ); + } + + public function testInjectsSlugAndIdParameters(): void + { + $this->assertSame('entry-show:hello-world', $this->dispatch('GET', '/entries/hello-world')->body()); + $this->assertSame('note-show:42', $this->dispatch('GET', '/notes/42')->body()); + $this->assertSame('entries-edit:7', $this->dispatch('GET', '/admin/entries/7/edit')->body()); + $this->assertSame('notes-edit:9', $this->dispatch('GET', '/admin/notes/9/edit')->body()); + } + + public function testLiteralSegmentsWithDotsMatch(): void + { + $this->assertSame('feed', $this->dispatch('GET', '/feed.xml')->body()); + $this->assertSame('robots', $this->dispatch('GET', '/robots.txt')->body()); + $this->assertSame('sitemap', $this->dispatch('GET', '/sitemap.xml')->body()); + } + + public function testAdminGroupThenCsrfGroupOrderOnPost(): void + { + $this->dispatch('POST', '/admin/entries/preview'); + + $this->assertSame( + ['admin:before', 'csrf:before', 'csrf:after', 'admin:after'], + $this->log->getArrayCopy(), + ); + } + + public function testCsrfGroupSkippedForGet(): void + { + $this->dispatch('GET', '/admin/notes'); + + $this->assertSame(['admin:before', 'admin:after'], $this->log->getArrayCopy()); + } + + public function testNotFoundStillThrows(): void + { + $this->expectException(RouteNotFoundException::class); + + $this->dispatch('GET', '/missing'); + } + + public function testFiveLayerOnionOrder(): void + { + $order = new ArrayObject(); + $mk = static function (string $name) use ($order): MiddlewareInterface { + return new class ($name, $order) implements MiddlewareInterface { + /** @param ArrayObject $order */ + public function __construct( + private readonly string $name, + private readonly ArrayObject $order, + ) { + } + + public function handle(RequestInterface $request, callable $next): ResponseInterface + { + $this->order->append($this->name . ':before'); + $response = $next($request); + $this->order->append($this->name . ':after'); + + return $response; + } + }; + }; + + $core = static function (RequestInterface $request) use ($order): ResponseInterface { + $order->append('core'); + + return new TestResponse(200, 'ok'); + }; + + $pipeline = MiddlewarePipeline::compose([$mk('A'), $mk('B'), $mk('C'), $mk('D'), $mk('E')], $core); + $pipeline(new TestRequest('GET', '/')); + + $this->assertSame( + ['A:before', 'B:before', 'C:before', 'D:before', 'E:before', 'core', 'E:after', 'D:after', 'C:after', 'B:after', 'A:after'], + $order->getArrayCopy(), + ); + } + + private function dispatch(string $method, string $path): ResponseInterface + { + return $this->router->dispatch(new TestRequest($method, $path)); + } +} diff --git a/tests/Regression/KnownIssuesTest.php b/tests/Regression/KnownIssuesTest.php new file mode 100644 index 0000000..5a6c6f7 --- /dev/null +++ b/tests/Regression/KnownIssuesTest.php @@ -0,0 +1,158 @@ +bind($class, $controller); + + return new Router($resolver); + } + + /** + * Baseline: duplicated parameter names produce a route that can never + * match, and emit a PHP warning on every match attempt. + * + * Desired (miniroute-uhs.1): registration throws + * RouteRegistrationException. Flip this test when the fix lands. + */ + public function testDuplicateParamNamesAreADeadRoute(): void + { + $router = $this->routerWith(new DuplicateParamController(), DuplicateParamController::class); + $router->registerController(DuplicateParamController::class); + + $warnings = []; + set_error_handler(static function (int $errno, string $errstr) use (&$warnings): bool { + $warnings[] = $errstr; + + return true; + }); + + try { + $router->dispatch(new TestRequest('GET', '/dup/1/x/2')); + $matched = true; + } catch (RouteNotFoundException) { + $matched = false; + } finally { + restore_error_handler(); + } + + $this->assertFalse($matched, 'duplicate-param route must not match'); + $this->assertNotEmpty($warnings, 'matching a duplicate-param route emits a preg warning'); + } + + /** + * Baseline: a wrong method on an existing path is a 404, with no way to + * distinguish method-not-allowed. + * + * Desired (miniroute-uhs.2): MethodNotAllowedException carrying the + * allowed methods, so the app can render 405 + Allow. Flip this test. + */ + public function testWrongMethodOnExistingPathIsNotFound(): void + { + $router = $this->routerWith(new OnlyGetController(), OnlyGetController::class); + $router->registerController(OnlyGetController::class); + + $this->expectException(RouteNotFoundException::class); + + $router->dispatch(new TestRequest('POST', '/only')); + } + + /** + * Baseline: HEAD requests never map to GET handlers. + * + * Desired (miniroute-uhs.3): HEAD dispatches to the GET handler and the + * app's response layer suppresses the body. Flip this test. + */ + public function testHeadDoesNotMapToGet(): void + { + $router = $this->routerWith(new OnlyGetController(), OnlyGetController::class); + $router->registerController(OnlyGetController::class); + + $this->expectException(RouteNotFoundException::class); + + $router->dispatch(new TestRequest('HEAD', '/only')); + } + + /** + * Baseline: route attributes on non-public methods are silently skipped + * by the loader. + * + * Desired (miniroute-uhs.4): loading a controller with a route attribute + * on a non-public method throws RouteRegistrationException. Flip this test. + */ + public function testNonPublicMethodsWithRouteAttributesAreSkipped(): void + { + $routes = (new RouteLoader())->load(MixedVisibilityController::class); + + $this->assertCount(1, $routes); + $this->assertSame('/pub', $routes[0]->path); + } + + /** + * Baseline: a resolver returning the same middleware instance across + * requests accumulates state (singleton-container leak). This is a usage + * contract, not a kernel bug: middleware must be stateless or + * per-request scoped. ADR work tracked in miniroute-k4i.3. + */ + public function testSingletonMiddlewareAccumulatesStateAcrossRequests(): void + { + $stateful = new StatefulMiddleware(); + + $resolver = new FakeResolver(); + $resolver->bind(StatefulController::class, new StatefulController()); + $resolver->bind(StatefulMiddleware::class, $stateful); + + $router = new Router($resolver); + $router->registerController(StatefulController::class); + + $router->dispatch(new TestRequest('GET', '/stateful')); + $router->dispatch(new TestRequest('GET', '/stateful')); + + $this->assertSame(2, $stateful->count, 'state leaks across dispatches with a shared instance'); + } + + /** + * Baseline: {param} segments are greedy — any single segment matches, so + * a typo'd URL gets a 200 from the parameterized route, not a 404. + * + * Documented behavior, no kernel fix planned: controllers validate the + * captured value (the blog controller 404s on an unknown slug). Decision + * recorded in miniroute-k4i.3 (param constraints rejected). + */ + public function testParamRoutesAreGreedy(): void + { + $router = $this->routerWith(new FakeController(), FakeController::class); + $router->registerController(FakeController::class); + + $response = $router->dispatch(new TestRequest('GET', '/entries/typo-of-known-path')); + + $this->assertSame(200, $response->status()); + $this->assertSame('show:typo-of-known-path', $response->body()); + } +} diff --git a/tests/RouterTest.php b/tests/RouterTest.php index b8d4d49..3279ede 100644 --- a/tests/RouterTest.php +++ b/tests/RouterTest.php @@ -8,10 +8,12 @@ use ArrayObject; use BrettParson\MiniRoute\Routing\RouteNotFoundException; use BrettParson\MiniRoute\Routing\RouteRegistrationException; use BrettParson\MiniRoute\Routing\Router; +use BrettParson\MiniRoute\Tests\Fixtures\CollidingController; use BrettParson\MiniRoute\Tests\Fixtures\FakeController; use BrettParson\MiniRoute\Tests\Fixtures\FakeMiddleware; use BrettParson\MiniRoute\Tests\Fixtures\FakeResolver; use BrettParson\MiniRoute\Tests\Fixtures\TestRequest; +use BrettParson\MiniRoute\Tests\Fixtures\TestResponse; use PHPUnit\Framework\TestCase; final class RouterTest extends TestCase @@ -78,4 +80,63 @@ final class RouterTest extends TestCase $this->expectException(RouteNotFoundException::class); $router->dispatch(new TestRequest('GET', '/missing')); } + + public function testNotFoundHandlerIsCalledWhenSet(): void + { + [$router] = $this->makeRouter(); + $router->registerController(FakeController::class); + $router->setNotFoundHandler(static fn (): TestResponse => new TestResponse(404, 'custom-nf')); + + $response = $router->dispatch(new TestRequest('GET', '/missing')); + + $this->assertSame(404, $response->status()); + $this->assertSame('custom-nf', $response->body()); + } + + public function testTrailingSlashIsNormalized(): void + { + [$router] = $this->makeRouter(); + $router->registerController(FakeController::class); + + $response = $router->dispatch(new TestRequest('GET', '/entries/')); + + $this->assertSame('index', $response->body()); + } + + public function testMethodIsCaseInsensitive(): void + { + [$router] = $this->makeRouter(); + $router->registerController(FakeController::class); + + $response = $router->dispatch(new TestRequest('get', '/entries')); + + $this->assertSame('index', $response->body()); + } + + public function testLeadingDoubleSlashDoesNotMatch(): void + { + [$router] = $this->makeRouter(); + $router->registerController(FakeController::class); + + $this->expectException(RouteNotFoundException::class); + $router->dispatch(new TestRequest('GET', '//entries')); + } + + public function testPathsAreCaseSensitive(): void + { + [$router] = $this->makeRouter(); + $router->registerController(FakeController::class); + + $this->expectException(RouteNotFoundException::class); + $router->dispatch(new TestRequest('GET', '/Entries')); + } + + public function testDuplicateRouteAcrossControllersThrows(): void + { + [$router] = $this->makeRouter(); + $router->registerController(FakeController::class); + + $this->expectException(RouteRegistrationException::class); + $router->registerController(CollidingController::class); + } } diff --git a/tests/bench/bench.php b/tests/bench/bench.php new file mode 100644 index 0000000..b7ae94e --- /dev/null +++ b/tests/bench/bench.php @@ -0,0 +1,107 @@ +dispatch(new TestRequest($method, $path))->body(); + } catch (RouteNotFoundException) { + return null; + } +} + +function benchMs(float $start): float +{ + return (microtime(true) - $start) * 1000; +} + +echo "miniroute benchmark\n"; +echo str_repeat('-', 64) . "\n"; + +// ── Realistic table: registration + 2000 dispatches ──────────────────────── + +$resolver = new FakeResolver(); +$resolver->bind(RealisticPublicController::class, new RealisticPublicController()); +$resolver->bind(RealisticAdminController::class, new RealisticAdminController()); +$resolver->bind(AdminLoggingMiddleware::class, new AdminLoggingMiddleware(new ArrayObject())); +$resolver->bind(CsrfLoggingMiddleware::class, new CsrfLoggingMiddleware(new ArrayObject())); + +$real = new Router($resolver); +$real->group('/admin', [AdminLoggingMiddleware::class]); +$real->group('/admin', [CsrfLoggingMiddleware::class], ['POST']); + +$start = microtime(true); +$real->registerController(RealisticPublicController::class); +$real->registerController(RealisticAdminController::class); +$regMs = benchMs($start); + +$paths = ['/entries', '/entries/hello-world', '/notes/42', '/admin', '/admin/notes', '/admin/entries/preview', '/feed.xml', '/robots.txt', '/sitemap.xml']; +$start = microtime(true); +for ($i = 0; $i < 2000; $i++) { + benchHit($real, 'GET', $paths[$i % count($paths)]); +} +$dispatchMs = benchMs($start); + +printf("realistic (22 routes): register = %8.3fms | 2000 dispatches = %8.2fms (%0.5fms each)\n", $regMs, $dispatchMs, $dispatchMs / 2000); + +// ── Scaling: synthetic routes at 100 / 300 / 600 ─────────────────────────── + +printf("\n%-22s %-20s %s\n", 'routes', 'register', 'leaf dispatch (each)'); + +foreach ([100, 300, 600] as $n) { + $resolver = new FakeResolver(); + $bench = new Router($resolver); + + $start = microtime(true); + for ($i = 0; $i < $n; $i++) { + $class = 'BenchRoute' . $n . '_' . $i; + eval('final class ' . $class . ' { #[' . Get::class . '("/bench/' . $n . '/' . $i . '")] public function run(): ' . ResponseInterface::class . ' { return new ' . TestResponse::class . '(200, "b"); } }'); + $resolver->bind($class, new $class()); + $bench->registerController($class); + } + $regMs = benchMs($start); + + $start = microtime(true); + for ($i = 0; $i < 200; $i++) { + benchHit($bench, 'GET', '/bench/' . $n . '/' . ($n - 1)); + } + $dispatchMs = benchMs($start) / 200; + + printf("%-22d %8.1fms %s %0.5fms\n", $n, $regMs, str_repeat(' ', 11), $dispatchMs); +} + +echo str_repeat('-', 64) . "\n"; -- cgit v1.2.3