diff --git a/packages/core/src/Application.php b/packages/core/src/Application.php index 3b9344c2..6aeba1a3 100644 --- a/packages/core/src/Application.php +++ b/packages/core/src/Application.php @@ -87,6 +87,8 @@ class Application /** @var ?Router */ private ?object $_router = null; + private bool $shutDown = false; + /** @var Router */ public object $router { get => $this->_router ?? throw new RuntimeException( @@ -277,6 +279,40 @@ public function initialize( $this->bootstrapErrorHandler->unregister(); } + /** + * Undo the global state boot installed, so the process can go on without this application. + * + * Runs every module's `shutdown` callback (from module.php) in reverse module load order, + * the mirror of the boot callbacks, so handlers pushed onto PHP's global stacks come off + * in the order they went on. The errors modules unregister their error and exception + * handlers here. Then removes the bootstrap error handler if a failed boot left it installed. + * + * Call it when the application is done but the process is not: marko/testing calls it + * after each test that booted one, and a long-running worker can call it before it exits + * or reboots. Safe to call more than once; only the first call does anything. + * + * @throws BindingException|CircularDependencyException|ReflectionException|PluginException|ContainerExceptionInterface + */ + public function shutdown(): void + { + if ($this->shutDown) { + return; + } + + $this->shutDown = true; + + // initialize() never ran (or failed before the container existed): nothing booted + if (isset($this->container)) { + foreach (array_reverse($this->modules) as $module) { + if ($module->shutdown !== null) { + $this->container->call($module->shutdown); + } + } + } + + $this->bootstrapErrorHandler->unregister(); + } + /** * Register PSR-4 autoloaders for non-vendor modules. * diff --git a/packages/core/src/Module/ManifestParser.php b/packages/core/src/Module/ManifestParser.php index 48928d20..cbfaf539 100644 --- a/packages/core/src/Module/ManifestParser.php +++ b/packages/core/src/Module/ManifestParser.php @@ -40,7 +40,7 @@ public function parse( * Rebuild a manifest from its cached composer data and the module's live module.php. * * Never reads composer.json. module.php is still required, so its closures - * (bindings, boot callbacks) stay live. + * (bindings, boot and shutdown callbacks) stay live. * * @throws ModuleException If module.php has syntax errors or does not return an array */ @@ -93,6 +93,7 @@ private function build( globalMiddleware: $moduleData['globalMiddleware'] ?? [], extra: $extra, discovery: $moduleData['discovery'] ?? [], + shutdown: $moduleData['shutdown'] ?? null, ); } diff --git a/packages/core/src/Module/ModuleDiscovery.php b/packages/core/src/Module/ModuleDiscovery.php index 64b92c28..12e78321 100644 --- a/packages/core/src/Module/ModuleDiscovery.php +++ b/packages/core/src/Module/ModuleDiscovery.php @@ -171,6 +171,7 @@ private function withPathAndSource( globalMiddleware: $manifest->globalMiddleware, extra: $manifest->extra, discovery: $manifest->discovery, + shutdown: $manifest->shutdown, ); } diff --git a/packages/core/src/Module/ModuleManifest.php b/packages/core/src/Module/ModuleManifest.php index 5f851645..db96c37c 100644 --- a/packages/core/src/Module/ModuleManifest.php +++ b/packages/core/src/Module/ModuleManifest.php @@ -10,7 +10,7 @@ * Value object representing a discovered module. * * Combines data from composer.json (name, version, require) - * with Marko-specific config from module.php (enabled, sequence, bindings, boot). + * with Marko-specific config from module.php (enabled, sequence, bindings, boot, shutdown). */ readonly class ModuleManifest { @@ -29,6 +29,7 @@ * @param string $source Discovery source: vendor, modules, or app * @param array $autoload PSR-4 autoload configuration from composer.json (namespace => path) * @param Closure|null $boot Boot callback to run after bindings are registered (from module.php). Parameters are auto-injected from the container — type-hint any registered dependency, including ContainerInterface. + * @param Closure|null $shutdown Shutdown callback run by Application::shutdown() (from module.php), in reverse module load order, to undo what the boot callback set up (global handlers, open resources). Parameters are auto-injected like boot's. * @param array $globalMiddleware Global HTTP middleware classes declared by this module (from module.php). Order across modules follows DependencyResolver (composer require + sequence: { after, before }); order within a module follows array declaration order. * @param array $extra Raw extra data from composer.json (vendor/Marko-specific metadata) * @param array $discovery Discovery cache contributors declared by this module (from module.php); each must implement DiscoveryCacheContributorInterface @@ -49,5 +50,6 @@ public function __construct( public array $globalMiddleware = [], public array $extra = [], public array $discovery = [], + public ?Closure $shutdown = null, ) {} } diff --git a/packages/core/tests/Unit/ApplicationShutdownTest.php b/packages/core/tests/Unit/ApplicationShutdownTest.php new file mode 100644 index 00000000..db777c32 --- /dev/null +++ b/packages/core/tests/Unit/ApplicationShutdownTest.php @@ -0,0 +1,163 @@ + [], 'omega' => ['after' => ['app/alpha']]] as $name => $sequence) { + file_put_contents("$base/app/$name/composer.json", json_encode([ + 'name' => "app/$name", + 'extra' => ['marko' => ['module' => true]], + ])); + $sequenceCode = var_export($sequence, true); + file_put_contents("$base/app/$name/module.php", << $sequenceCode, + 'boot' => function (): void { \$GLOBALS['$key'][] = '$name boot'; }, + 'shutdown' => function (AppEnvironment \$appEnvironment): void { + \$GLOBALS['$key'][] = '$name shutdown (' . \$appEnvironment::class . ')'; + }, + ]; + PHP); + } + + return ['base' => $base, 'key' => $key]; +} + +function applicationShutdownCleanup( + string $dir, +): void { + if (!is_dir($dir)) { + return; + } + + $items = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($dir, RecursiveDirectoryIterator::SKIP_DOTS), + RecursiveIteratorIterator::CHILD_FIRST, + ); + + foreach ($items as $item) { + $item->isDir() ? rmdir($item->getPathname()) : unlink($item->getPathname()); + } + + rmdir($dir); +} + +/** + * The active exception handlers, bottom of the stack first, read without changing them. + * + * @return list + */ +function applicationShutdownExceptionHandlers(): array +{ + $handlers = []; + + while (($handler = set_exception_handler(null)) !== null) { + restore_exception_handler(); + $handlers[] = $handler; + restore_exception_handler(); + } + + restore_exception_handler(); + $handlers = array_reverse($handlers); + + foreach ($handlers as $handler) { + set_exception_handler($handler); + } + + return $handlers; +} + +describe('Application::shutdown()', function (): void { + beforeEach(function (): void { + $this->savedEnv = []; + + foreach (['APP_ENV', 'MARKO_ENV', 'DISCOVERY_CACHE_ENABLED'] as $key) { + $this->savedEnv[$key] = [array_key_exists($key, $_ENV) ? $_ENV[$key] : null, getenv($key)]; + unset($_ENV[$key]); + putenv($key); + } + + $_ENV['APP_ENV'] = 'production'; + $this->project = applicationShutdownProject(); + $base = $this->project['base']; + $this->app = new Application(vendorPath: "$base/vendor", modulesPath: "$base/modules", appPath: "$base/app"); + }); + + afterEach(function (): void { + foreach ($this->savedEnv as $key => [$env, $process]) { + unset($_ENV[$key]); + putenv($key); + + if ($env !== null) { + $_ENV[$key] = $env; + } + + if ($process !== false) { + putenv("$key=$process"); + } + } + + unset($GLOBALS[$this->project['key']]); + applicationShutdownCleanup($this->project['base']); + }); + + it('runs every module shutdown callback in reverse load order, with injected parameters', function (): void { + $this->app->initialize(false); + $this->app->shutdown(); + + expect($GLOBALS[$this->project['key']])->toBe([ + 'alpha boot', + 'omega boot', + 'omega shutdown (' . AppEnvironment::class . ')', + 'alpha shutdown (' . AppEnvironment::class . ')', + ]); + }); + + it('runs the shutdown callbacks only once when called again', function (): void { + $this->app->initialize(false); + $this->app->shutdown(); + $this->app->shutdown(); + + expect(array_filter( + $GLOBALS[$this->project['key']], + fn (string $entry): bool => str_contains($entry, 'shutdown'), + ))->toHaveCount(2); + }); + + it('does nothing for an application that was never initialized', function (): void { + $this->app->shutdown(); + + expect($GLOBALS[$this->project['key']] ?? [])->toBe([]); + }); + + it('removes the bootstrap error handler when it is still installed', function (): void { + $before = applicationShutdownExceptionHandlers(); + $this->app->bootstrapErrorHandler->register(); + + $this->app->shutdown(); + + expect(applicationShutdownExceptionHandlers())->toBe($before); + }); +}); diff --git a/packages/core/tests/Unit/Module/ManifestParserCachedTest.php b/packages/core/tests/Unit/Module/ManifestParserCachedTest.php index 32dc418d..1d2aa4a5 100644 --- a/packages/core/tests/Unit/Module/ManifestParserCachedTest.php +++ b/packages/core/tests/Unit/Module/ManifestParserCachedTest.php @@ -82,7 +82,7 @@ function cachedManifestCleanup( it('keeps module.php closures live when building a manifest from a cached module', function (): void { $dir = cachedManifestModule( - " fn (): string => 'booted', 'bindings' => ['A' => fn (): string => 'b']];", + " fn (): string => 'booted', 'shutdown' => fn (): string => 'shut down', 'bindings' => ['A' => fn (): string => 'b']];", ); $manifest = (new ManifestParser())->parseCached(new CachedModule( @@ -95,6 +95,8 @@ function cachedManifestCleanup( expect($manifest->boot)->toBeInstanceOf(Closure::class) ->and(($manifest->boot)())->toBe('booted') + ->and($manifest->shutdown)->toBeInstanceOf(Closure::class) + ->and(($manifest->shutdown)())->toBe('shut down') ->and($manifest->bindings['A'])->toBeInstanceOf(Closure::class); }); diff --git a/packages/docs-markdown/docs/packages/core.md b/packages/docs-markdown/docs/packages/core.md index 49b7aec8..ba877156 100644 --- a/packages/docs-markdown/docs/packages/core.md +++ b/packages/docs-markdown/docs/packages/core.md @@ -103,7 +103,7 @@ Create a directory in `app/` with a `composer.json`: app/ mymodule/ composer.json # Required: name, autoload - module.php # Optional: enabled, sequence, bindings, singletons, boot, globalMiddleware, discovery + module.php # Optional: enabled, sequence, bindings, singletons, boot, shutdown, globalMiddleware, discovery src/ MyService.php ``` @@ -160,6 +160,26 @@ readonly class CheckWidgetSetup Under PHP-FPM every request is a boot, so keep these observers cheap. `marko/authorization` uses this event to [check `#[Can]` routes at boot](/docs/packages/authorization/#cost-on-routes-without-can). +#### Shutting down + +A `boot` callback that changes process-wide state (pushes a global error or exception handler, opens a resource) has a matching `shutdown` callback that undoes it. `Application::shutdown()` runs every module's `shutdown` callback in reverse module load order, so handlers come off PHP's stacks in the reverse order they went on. Parameters are auto-injected like `boot`'s: + +```php title="module.php" +return [ + 'singletons' => [ + ErrorHandlerInterface::class, // shared, so shutdown gets the instance boot registered + ], + 'boot' => function (ErrorHandlerInterface $handler): void { + $handler->register(); + }, + 'shutdown' => function (ErrorHandlerInterface $handler): void { + $handler->unregister(); + }, +]; +``` + +Call `$app->shutdown()` when the application is done but the PHP process is not. [marko/testing](/docs/packages/testing/#shutdown-after-each-test) does it after every test that booted an application, so the test leaves PHP's error and exception handlers as it found them. A long-running worker can call it before it exits or boots a fresh application. Only the first call does anything. It also removes the [bootstrap error handler](#errors-during-boot) if a failed boot left it installed. Under PHP-FPM there is nothing to call: the process ends with the request. + ### Application Environment `AppEnvironment` is the single answer to "which environment is this application running in?". `Application` registers one shared instance in the container at boot, so any class or boot callback can type-hint it: diff --git a/packages/docs-markdown/docs/packages/errors-advanced.md b/packages/docs-markdown/docs/packages/errors-advanced.md index 4ebe7887..814583a9 100644 --- a/packages/docs-markdown/docs/packages/errors-advanced.md +++ b/packages/docs-markdown/docs/packages/errors-advanced.md @@ -11,7 +11,7 @@ Pretty error pages with syntax-highlighted code, stack traces, and request detai composer require marko/errors-advanced ``` -This replaces the default `marko/errors-simple` handler with a more detailed error display. The package binds `AdvancedErrorHandler` to `ErrorHandlerInterface` and registers it automatically via its `module.php` boot callback. +This replaces the default `marko/errors-simple` handler with a more detailed error display. The package binds `AdvancedErrorHandler` to `ErrorHandlerInterface` and registers it automatically via its `module.php` boot callback. Like errors-simple, it binds the handler as a singleton and unregisters it from its `shutdown` callback when [`Application::shutdown()`](/docs/packages/core/#shutting-down) runs. ## Usage diff --git a/packages/docs-markdown/docs/packages/errors-simple.md b/packages/docs-markdown/docs/packages/errors-simple.md index 65a869f9..c0731ff2 100644 --- a/packages/docs-markdown/docs/packages/errors-simple.md +++ b/packages/docs-markdown/docs/packages/errors-simple.md @@ -20,7 +20,7 @@ Non-fatal PHP errors (warnings, notices, deprecations) are reported via `error_l composer require marko/errors-simple ``` -The handler registers automatically via module boot --- no configuration required. +The handler registers automatically via module boot --- no configuration required. `ErrorHandlerInterface` is bound as a singleton, and the module's `shutdown` callback unregisters the handler when [`Application::shutdown()`](/docs/packages/core/#shutting-down) runs (marko/testing does this after every test that booted the application). ## Usage diff --git a/packages/docs-markdown/docs/packages/testing.md b/packages/docs-markdown/docs/packages/testing.md index 7e1aa5f5..71c6487d 100644 --- a/packages/docs-markdown/docs/packages/testing.md +++ b/packages/docs-markdown/docs/packages/testing.md @@ -502,6 +502,17 @@ $client->withCookie('locale', 'nl')->get('/')->assertSee('Welkom'); Use `TestClient::forApplication($app)` to wrap an `Application` you booted yourself. `TestClient::boot()` throws when the base path does not exist. +### Shutdown after each test + +Booting runs every module's `boot` callback, and an errors module ([marko/errors-simple](/docs/packages/errors-simple/) or [marko/errors-advanced](/docs/packages/errors-advanced/)) uses it to install a global error handler and exception handler. Left in place, they would make PHPUnit report every HTTP test as risky ("did not remove its own error handlers") and pile up within a process. + +`marko/testing` owns the boot, so it owns the teardown. Every application booted by `TestClient::boot()` or `TestDatabase::boot()` is tracked in `Marko\Testing\BootedApplications`, and after each test it calls [`Application::shutdown()`](/docs/packages/core/#shutting-down) on each one. That runs the modules' `shutdown` callbacks, which remove the handlers, so the test leaves the global error and exception handler stacks exactly as it found them. This happens automatically: + +- **Pest**: `marko/testing` declares a second Pest plugin, `Marko\Testing\Pest\ShutdownApplicationsPlugin`, which runs the shutdown after every test, plain `it()` tests included. There is nothing to add to `Pest.php`. +- **PHPUnit**: `Marko\Testing\TestCase::tearDown()` runs it. If you override `tearDown()`, call `parent::tearDown()`. + +An application you boot yourself, with `Application::boot()` or for `TestClient::forApplication()`, is not tracked. Call `$app->shutdown()` when the test is done with it. + ### Pest setup Create one client per test, so state from one test (cookies, `actingAs()`, headers) never reaches the next: @@ -748,7 +759,7 @@ Database tests need `marko/database` and a driver. `marko/testing` only suggests composer require --dev marko/database-pgsql # or marko/database-mysql ``` -`TestDatabase::boot($basePath)` boots your application once per process (the same `Application::boot()` that `TestClient::boot()` uses) and applies the committed migrations once with `Migrator::migrate()`. Every later call for the same base path returns the same instance, so a suite pays for boot and migrations once, not once per test. Pair it with one of two isolation strategies: +`TestDatabase::boot($basePath)` boots your application once per process (the same `Application::boot()` that `TestClient::boot()` uses) and applies the committed migrations once with `Migrator::migrate()`. Every later call for the same base path returns the same instance, so a suite pays for boot and migrations once, not once per test. The application is [shut down](#shutdown-after-each-test) after the test that first booted it, which removes its global error handlers; it stays usable, so later tests keep sharing it. Pair it with one of two isolation strategies: | Strategy | Per test | Use it for | |----------|----------|------------| @@ -890,7 +901,7 @@ The expectations register automatically. `marko/testing` declares a Pest plugin The expectations need Pest 4 (`composer require --dev pestphp/pest`). The fakes and their `assert*()` methods work with plain PHPUnit. :::note -If an expectation such as `toHavePushed` is reported as an undefined method, Pest's plugin list is stale. Run `composer dump-autoload` to regenerate `vendor/pest-plugins.json`, and make sure `pestphp/pest-plugin` is allowed under `config.allow-plugins`. +If an expectation such as `toHavePushed` is reported as an undefined method, or tests that boot the application are reported as risky, Pest's plugin list is stale. Run `composer dump-autoload` to regenerate `vendor/pest-plugins.json`, and make sure `pestphp/pest-plugin` is allowed under `config.allow-plugins`. ::: Use them in tests: @@ -1351,6 +1362,13 @@ public function assertNotProduction(): void; public function assertDisposable(string $operation): void; ``` +### BootedApplications + +```php +public static function track(Application $application): void; +public static function shutdownAll(): void; +``` + ### RefreshDatabase ```php diff --git a/packages/errors-advanced/module.php b/packages/errors-advanced/module.php index 4c06491d..f301be5b 100644 --- a/packages/errors-advanced/module.php +++ b/packages/errors-advanced/module.php @@ -28,6 +28,10 @@ ); }, ], + // Shared, so the shutdown callback unregisters the instance boot registered + 'singletons' => [ + ErrorHandlerInterface::class, + ], 'boot' => function (ContainerInterface $container, BootstrapErrorHandler $bootstrapErrorHandler) { // Get the error handler and register it in place of core's bootstrap // handler, so the bootstrap handler is not left registered underneath @@ -35,4 +39,9 @@ $bootstrapErrorHandler->unregister(); $handler->register(); }, + // Application::shutdown() runs this (marko/testing does after every test that booted an + // application). It pops exactly the handlers boot registered, from the same shared instance. + 'shutdown' => function (ErrorHandlerInterface $handler): void { + $handler->unregister(); + }, ]; diff --git a/packages/errors-advanced/tests/Unit/HttpAwareAdvancedErrorHandlerTest.php b/packages/errors-advanced/tests/Unit/HttpAwareAdvancedErrorHandlerTest.php index c8f4cb5a..ab8fd4a5 100644 --- a/packages/errors-advanced/tests/Unit/HttpAwareAdvancedErrorHandlerTest.php +++ b/packages/errors-advanced/tests/Unit/HttpAwareAdvancedErrorHandlerTest.php @@ -4,6 +4,7 @@ namespace Marko\ErrorsAdvanced\Tests\Unit\HttpAware; +use Closure; use Marko\Core\Container\Container; use Marko\Core\Container\ContainerInterface; use Marko\Core\Container\PreferenceRegistry; @@ -102,6 +103,55 @@ static function () use ($moduleHandler): void { ->and($bootstrapErrorHandler->isActive())->toBeFalse(); }); + it('removes the handlers its boot callback registered from its shutdown callback', function (): void { + $module = require dirname(__DIR__, 2) . '/module.php'; + $stacks = static function (): array { + $read = static function (Closure $set, Closure $restore): array { + $handlers = []; + + while (($handler = $set(null)) !== null) { + $restore(); + $handlers[] = $handler; + $restore(); + } + + $restore(); + $handlers = array_reverse($handlers); + + foreach ($handlers as $handler) { + $set($handler); + } + + return $handlers; + }; + + return [ + $read(set_error_handler(...), restore_error_handler(...)), + $read(set_exception_handler(...), restore_exception_handler(...)), + ]; + }; + $container = new Container(new PreferenceRegistry()); + $container->instance(ContainerInterface::class, $container); + $container->instance(ClockInterface::class, new FakeClock()); + $container->instance(BootstrapErrorHandler::class, new BootstrapErrorHandler()); + + foreach ($module['bindings'] as $abstract => $concrete) { + $container->bind($abstract, $concrete); + } + + foreach ($module['singletons'] ?? [] as $abstract) { + $container->singleton($abstract); + } + + $before = $stacks(); + $container->call($module['boot']); + $booted = $stacks(); + $container->call($module['shutdown']); + + expect($booted)->not->toBe($before) + ->and($stacks())->toBe($before); + }); + it('resolves ErrorHandlerInterface from the module bindings', function (): void { $module = require dirname(__DIR__, 2) . '/module.php'; $container = new Container(new PreferenceRegistry()); diff --git a/packages/errors-simple/module.php b/packages/errors-simple/module.php index 01b3512c..928a3c16 100644 --- a/packages/errors-simple/module.php +++ b/packages/errors-simple/module.php @@ -14,6 +14,10 @@ 'bindings' => [ ErrorHandlerInterface::class => SimpleErrorHandler::class, ], + // Shared, so the shutdown callback unregisters the instance boot registered + 'singletons' => [ + ErrorHandlerInterface::class, + ], 'boot' => function (ContainerInterface $container, BootstrapErrorHandler $bootstrapErrorHandler) { // Get the error handler and register it in place of core's bootstrap // handler, so the bootstrap handler is not left registered underneath @@ -21,4 +25,9 @@ $bootstrapErrorHandler->unregister(); $handler->register(); }, + // Application::shutdown() runs this (marko/testing does after every test that booted an + // application). It pops exactly the handlers boot registered, from the same shared instance. + 'shutdown' => function (ErrorHandlerInterface $handler): void { + $handler->unregister(); + }, ]; diff --git a/packages/errors-simple/tests/Unit/PackageSetupTest.php b/packages/errors-simple/tests/Unit/PackageSetupTest.php index e4e17fb1..2af94621 100644 --- a/packages/errors-simple/tests/Unit/PackageSetupTest.php +++ b/packages/errors-simple/tests/Unit/PackageSetupTest.php @@ -13,6 +13,8 @@ use Marko\ErrorsSimple\Formatters\BasicHtmlFormatter; use Marko\ErrorsSimple\Formatters\TextFormatter; use Marko\ErrorsSimple\SimpleErrorHandler; +use Marko\Testing\Fake\FakeClock; +use Psr\Clock\ClockInterface; describe('Package Setup', function () { $composerJsonPath = dirname(__DIR__, 2) . '/composer.json'; @@ -93,6 +95,55 @@ static function () use ($moduleHandler): void { ->and($bootstrapErrorHandler->isActive())->toBeFalse(); }); + it('removes the handlers its boot callback registered from its shutdown callback', function () use ($modulePath) { + $module = require $modulePath; + $stacks = static function (): array { + $read = static function (Closure $set, Closure $restore): array { + $handlers = []; + + while (($handler = $set(null)) !== null) { + $restore(); + $handlers[] = $handler; + $restore(); + } + + $restore(); + $handlers = array_reverse($handlers); + + foreach ($handlers as $handler) { + $set($handler); + } + + return $handlers; + }; + + return [ + $read(set_error_handler(...), restore_error_handler(...)), + $read(set_exception_handler(...), restore_exception_handler(...)), + ]; + }; + $container = new Container(new PreferenceRegistry()); + $container->instance(ContainerInterface::class, $container); + $container->instance(ClockInterface::class, new FakeClock()); + $container->instance(BootstrapErrorHandler::class, new BootstrapErrorHandler()); + + foreach ($module['bindings'] as $abstract => $concrete) { + $container->bind($abstract, $concrete); + } + + foreach ($module['singletons'] ?? [] as $abstract) { + $container->singleton($abstract); + } + + $before = $stacks(); + $container->call($module['boot']); + $booted = $stacks(); + $container->call($module['shutdown']); + + expect($booted)->not->toBe($before) + ->and($stacks())->toBe($before); + }); + it('exports SimpleErrorHandler', function () { expect(class_exists(SimpleErrorHandler::class))->toBeTrue(); }); diff --git a/packages/testing/composer.json b/packages/testing/composer.json index 92d037c7..65d4fa16 100644 --- a/packages/testing/composer.json +++ b/packages/testing/composer.json @@ -60,7 +60,8 @@ }, "pest": { "plugins": [ - "Marko\\Testing\\Pest\\ExpectationsPlugin" + "Marko\\Testing\\Pest\\ExpectationsPlugin", + "Marko\\Testing\\Pest\\ShutdownApplicationsPlugin" ] } } diff --git a/packages/testing/src/BootedApplications.php b/packages/testing/src/BootedApplications.php new file mode 100644 index 00000000..d2e3d1a5 --- /dev/null +++ b/packages/testing/src/BootedApplications.php @@ -0,0 +1,60 @@ + Keyed by object id, in boot order */ + private static array $applications = []; + + public static function track( + Application $application, + ): void { + self::$applications[spl_object_id($application)] = $application; + } + + /** + * Shut down every tracked application, the most recently booted first (handlers come off + * PHP's stacks in the reverse order they went on), and forget them. + * + * Every application is shut down even when one throws; the first failure is rethrown after. + * + * @throws Throwable + */ + public static function shutdownAll(): void + { + $applications = array_reverse(self::$applications); + self::$applications = []; + $failure = null; + + foreach ($applications as $application) { + try { + $application->shutdown(); + } catch (Throwable $throwable) { + $failure ??= $throwable; + } + } + + if ($failure !== null) { + throw $failure; + } + } +} diff --git a/packages/testing/src/Database/TestDatabase.php b/packages/testing/src/Database/TestDatabase.php index d962907c..11256201 100644 --- a/packages/testing/src/Database/TestDatabase.php +++ b/packages/testing/src/Database/TestDatabase.php @@ -24,6 +24,7 @@ use Marko\Database\Testing\DatabaseTestHelper; use Marko\Routing\Exceptions\RouteConflictException; use Marko\Routing\Exceptions\RouteException; +use Marko\Testing\BootedApplications; use Marko\Testing\Exceptions\DatabaseTestException; use Marko\Testing\Http\TestClient; use Psr\Container\ContainerExceptionInterface; @@ -102,6 +103,10 @@ public static function boot( // Booting reads the project's .env, so the environment is checked // after boot and before anything touches the database. $application = Application::boot($basePath); + // Shut down after this test, even when one of the checks below refuses it. The + // application stays shared and usable for later tests; shutting it down only removes + // the global error and exception handlers its boot installed. + BootedApplications::track($application); $booting = new self($application); $booting->assertNotProduction(); diff --git a/packages/testing/src/Http/TestClient.php b/packages/testing/src/Http/TestClient.php index 3dfdf8f5..2adf5910 100644 --- a/packages/testing/src/Http/TestClient.php +++ b/packages/testing/src/Http/TestClient.php @@ -31,6 +31,7 @@ use Marko\Routing\Http\Request; use Marko\Routing\Http\Response; use Marko\Routing\Http\UploadedFile; +use Marko\Testing\BootedApplications; use Marko\Testing\Exceptions\AssertionFailedException; use Marko\Testing\Exceptions\TestClientException; use Marko\Testing\Fake\FakeGuard; @@ -104,16 +105,23 @@ public function __construct( /** * Boot the application at $basePath (the project root holding vendor/, app/ and config/). * + * The application is shut down after the test (see {@see BootedApplications}), which + * removes the global error and exception handlers its boot installed. + * * @throws ModuleException|CircularDependencyException|BindingConflictException|BindingException|PluginException|PreferenceConflictException|EventException|ContainerExceptionInterface|RouteException|RouteConflictException|CommandException|ReflectionException|RuntimeException|DiscoveryCacheException */ public static function boot( string $basePath, ): self { - return new self(Application::boot($basePath)); + $application = Application::boot($basePath); + BootedApplications::track($application); + + return new self($application); } /** - * Send requests through an application that is already booted. + * Send requests through an application that is already booted. The caller booted it, + * so the caller shuts it down: it is not tracked for the after-test shutdown. */ public static function forApplication( Application $application, diff --git a/packages/testing/src/Pest/ShutdownApplicationsPlugin.php b/packages/testing/src/Pest/ShutdownApplicationsPlugin.php new file mode 100644 index 00000000..afe3adaa --- /dev/null +++ b/packages/testing/src/Pest/ShutdownApplicationsPlugin.php @@ -0,0 +1,27 @@ +afterEach() applies only to the paths passed to + * in()), so this registers a PHPUnit event subscriber instead. It covers plain it() tests that + * call TestClient::boot() with nothing in Pest.php, as well as tests extending + * Marko\Testing\TestCase. + * + * Declared in composer.json under extra.pest.plugins, like ExpectationsPlugin. Pest boots it in + * the main process and in every --parallel worker, before PHPUnit's event facade is sealed. + */ +class ShutdownApplicationsPlugin implements Bootable +{ + public function boot(): void + { + Facade::instance()->registerSubscriber(new ShutdownApplicationsSubscriber()); + } +} diff --git a/packages/testing/src/Pest/ShutdownApplicationsSubscriber.php b/packages/testing/src/Pest/ShutdownApplicationsSubscriber.php new file mode 100644 index 00000000..9351e24a --- /dev/null +++ b/packages/testing/src/Pest/ShutdownApplicationsSubscriber.php @@ -0,0 +1,27 @@ +registerModuleAutoloaders(); } + /** + * Shut down every application this test booted through TestClient::boot() or + * TestDatabase::boot(), so the test leaves PHP's error and exception handlers as it found them. + * + * @throws Throwable + */ + protected function tearDown(): void + { + BootedApplications::shutdownAll(); + + parent::tearDown(); + } + /** * Register PSR-4 autoloaders for app/* and modules/* modules. * diff --git a/packages/testing/tests/Feature/ErrorHandlerTeardownTest.php b/packages/testing/tests/Feature/ErrorHandlerTeardownTest.php new file mode 100644 index 00000000..6f35ed67 --- /dev/null +++ b/packages/testing/tests/Feature/ErrorHandlerTeardownTest.php @@ -0,0 +1,97 @@ +application()->container->get(ErrorHandlerInterface::class); + $booted = globalHandlerStacks(); + + expect(count($booted['errorHandlers']))->toBe(count($before['errorHandlers']) + 1) + ->and(count($booted['exceptionHandlers']))->toBe(count($before['exceptionHandlers']) + 1) + ->and(array_last($booted['errorHandlers']))->toBe([$handler, 'handleError']) + ->and(array_last($booted['exceptionHandlers']))->toBe([$handler, 'handleException']); + }); + + it('leaves the error and exception handler stacks as it found them after TestClient::boot()', function (): void { + $before = globalHandlerStacks(); + + TestClient::boot(errorsAppPath()); + BootedApplications::shutdownAll(); + + expect(globalHandlerStacks())->toBe($before); + }); + + it('does not stack handlers across repeated boots', function (): void { + $before = globalHandlerStacks(); + + foreach (range(1, 3) as $ignored) { + TestClient::boot(errorsAppPath()); + BootedApplications::shutdownAll(); + + expect(globalHandlerStacks())->toBe($before); + } + }); + + it('removes the handlers of every client booted in one test', function (): void { + $before = globalHandlerStacks(); + + TestClient::boot(errorsAppPath()); + TestClient::boot(errorsAppPath()); + TestClient::boot(errorsAppPath()); + BootedApplications::shutdownAll(); + + expect(globalHandlerStacks())->toBe($before); + }); + + it('leaves the handler stacks as it found them after a refused TestDatabase::boot()', function (): void { + $before = globalHandlerStacks(); + + // Boots (installing the handlers), then refuses: the fixture has no database driver + expect(fn () => withAppEnv('testing', fn () => TestDatabase::boot(errorsAppPath()))) + ->toThrow(DatabaseTestException::class, 'No database driver is installed'); + BootedApplications::shutdownAll(); + + expect(globalHandlerStacks())->toBe($before); + }); + + it('removes the handlers from the TestCase tearDown', function (): void { + $before = globalHandlerStacks(); + $testCase = new class ('test') extends TestCase + { + public function tearDown(): void + { + parent::tearDown(); + } + }; + + TestClient::boot(errorsAppPath()); + $testCase->tearDown(); + + expect(globalHandlerStacks())->toBe($before); + }); + +}); diff --git a/packages/testing/tests/Feature/PestPluginRegistrationTest.php b/packages/testing/tests/Feature/PestPluginRegistrationTest.php index 5df4c3d5..45a7d502 100644 --- a/packages/testing/tests/Feature/PestPluginRegistrationTest.php +++ b/packages/testing/tests/Feature/PestPluginRegistrationTest.php @@ -120,6 +120,17 @@ function removeFixturePestCacheDirectory(string $directory): void expect(str_contains($output, 'fails toHavePushed with an assertion failure when nothing was pushed')) ->toBeTrue($output) - ->and(str_contains($output, '2 passed'))->toBeTrue($output) + ->and(str_contains($output, '3 passed'))->toBeTrue($output) + ->and($exitCode)->toBe(0, $output); +}); + +it('shuts down an application a plain it() test booted, so the test is not risky', function (): void { + ['exitCode' => $exitCode, 'output' => $output] = runFixturePestProject(); + + expect(str_contains($output, 'boots an application without leaving its error handlers installed')) + ->toBeTrue($output) + ->and(str_contains($output, 'did not remove its own error handlers'))->toBeFalse($output) + ->and(str_contains($output, 'did not remove its own exception handlers'))->toBeFalse($output) + ->and(str_contains($output, 'risky'))->toBeFalse($output) ->and($exitCode)->toBe(0, $output); }); diff --git a/packages/testing/tests/Helpers.php b/packages/testing/tests/Helpers.php index d5adbeed..3cbb58eb 100644 --- a/packages/testing/tests/Helpers.php +++ b/packages/testing/tests/Helpers.php @@ -42,6 +42,49 @@ function removeHttpAppSessions(): void rmdir($directory); } +/** + * The fixture project whose boot installs global handlers: the real + * marko/errors-simple module wiring registers an error handler and an + * exception handler from its boot callback. + */ +function errorsAppPath(): string +{ + return __DIR__ . '/fixtures/errors-app'; +} + +/** + * PHP's error and exception handler stacks, bottom first, read without + * changing them (PHP has no getter, so each handler is popped and pushed back). + * + * @return array{errorHandlers: list, exceptionHandlers: list} + */ +function globalHandlerStacks(): array +{ + $read = static function (Closure $set, Closure $restore): array { + $handlers = []; + + while (($handler = $set(null)) !== null) { + $restore(); + $handlers[] = $handler; + $restore(); + } + + $restore(); + $handlers = array_reverse($handlers); + + foreach ($handlers as $handler) { + $set($handler); + } + + return $handlers; + }; + + return [ + 'errorHandlers' => $read(set_error_handler(...), restore_error_handler(...)), + 'exceptionHandlers' => $read(set_exception_handler(...), restore_exception_handler(...)), + ]; +} + /** * The fixture project the TestDatabase tests boot: an app module that binds * an in-memory recording connection the way a driver package would. diff --git a/packages/testing/tests/Unit/BootedApplicationsTest.php b/packages/testing/tests/Unit/BootedApplicationsTest.php new file mode 100644 index 00000000..6741c73c --- /dev/null +++ b/packages/testing/tests/Unit/BootedApplicationsTest.php @@ -0,0 +1,87 @@ + $log + */ +function recordingApplication( + string $name, + ArrayObject $log, + ?Throwable $throws = null, +): Application { + return new class ($name, $log, $throws) extends Application + { + /** + * @param ArrayObject $log + */ + public function __construct( + private readonly string $name, + private readonly ArrayObject $log, + private readonly ?Throwable $throws, + ) { + parent::__construct(); + } + + public function shutdown(): void + { + $this->log[] = $this->name; + + if ($this->throws !== null) { + throw $this->throws; + } + } + }; +} + +describe('BootedApplications', function (): void { + afterEach(function (): void { + BootedApplications::shutdownAll(); + }); + + it('shuts down every tracked application, the most recently booted first', function (): void { + $log = new ArrayObject(); + BootedApplications::track(recordingApplication('first', $log)); + BootedApplications::track(recordingApplication('second', $log)); + + BootedApplications::shutdownAll(); + + expect($log->getArrayCopy())->toBe(['second', 'first']); + }); + + it('forgets an application once it has been shut down', function (): void { + $log = new ArrayObject(); + BootedApplications::track(recordingApplication('only', $log)); + + BootedApplications::shutdownAll(); + BootedApplications::shutdownAll(); + + expect($log->getArrayCopy())->toBe(['only']); + }); + + it('tracks the same application once however often it is tracked', function (): void { + $log = new ArrayObject(); + $application = recordingApplication('shared', $log); + BootedApplications::track($application); + BootedApplications::track($application); + + BootedApplications::shutdownAll(); + + expect($log->getArrayCopy())->toBe(['shared']); + }); + + it('still shuts down the others when one throws, then rethrows the first failure', function (): void { + $log = new ArrayObject(); + BootedApplications::track(recordingApplication('first', $log, new RuntimeException('first failed'))); + BootedApplications::track(recordingApplication('second', $log, new LogicException('second failed'))); + BootedApplications::track(recordingApplication('third', $log)); + + expect(fn () => BootedApplications::shutdownAll())->toThrow(LogicException::class, 'second failed') + ->and($log->getArrayCopy())->toBe(['third', 'second', 'first']); + }); +}); diff --git a/packages/testing/tests/Unit/Pest/ShutdownApplicationsPluginTest.php b/packages/testing/tests/Unit/Pest/ShutdownApplicationsPluginTest.php new file mode 100644 index 00000000..54b029ac --- /dev/null +++ b/packages/testing/tests/Unit/Pest/ShutdownApplicationsPluginTest.php @@ -0,0 +1,31 @@ +toContain(ShutdownApplicationsPlugin::class); +}); + +it('implements the Pest Bootable plugin contract', function (): void { + expect(new ShutdownApplicationsPlugin())->toBeInstanceOf(Bootable::class); +}); + +it('subscribes to the event PHPUnit emits after tearDown and before its handler check', function (): void { + expect(new ShutdownApplicationsSubscriber())->toBeInstanceOf(AfterTestMethodFinishedSubscriber::class); +}); diff --git a/packages/testing/tests/fixtures/errors-app/vendor/autoload.php b/packages/testing/tests/fixtures/errors-app/vendor/autoload.php new file mode 100644 index 00000000..160aad87 --- /dev/null +++ b/packages/testing/tests/fixtures/errors-app/vendor/autoload.php @@ -0,0 +1,11 @@ + http-app/ -> fixtures/ -> tests/ -> testing/ -> packages/ -> root +require dirname(__DIR__, 6) . '/vendor/autoload.php'; diff --git a/packages/testing/tests/fixtures/errors-app/vendor/marko/clock/composer.json b/packages/testing/tests/fixtures/errors-app/vendor/marko/clock/composer.json new file mode 100644 index 00000000..9dc2e960 --- /dev/null +++ b/packages/testing/tests/fixtures/errors-app/vendor/marko/clock/composer.json @@ -0,0 +1,11 @@ +{ + "name": "marko/clock", + "description": "Fixture stub for marko/clock used by the marko/testing error handler fixture", + "type": "marko-module", + "license": "MIT", + "extra": { + "marko": { + "module": true + } + } +} diff --git a/packages/testing/tests/fixtures/errors-app/vendor/marko/clock/module.php b/packages/testing/tests/fixtures/errors-app/vendor/marko/clock/module.php new file mode 100644 index 00000000..03932092 --- /dev/null +++ b/packages/testing/tests/fixtures/errors-app/vendor/marko/clock/module.php @@ -0,0 +1,15 @@ + [ + ClockInterface::class => SystemClock::class, + ], + 'singletons' => [ + ClockInterface::class, + ], +]; diff --git a/packages/testing/tests/fixtures/errors-app/vendor/marko/errors-simple/composer.json b/packages/testing/tests/fixtures/errors-app/vendor/marko/errors-simple/composer.json new file mode 100644 index 00000000..4b586da4 --- /dev/null +++ b/packages/testing/tests/fixtures/errors-app/vendor/marko/errors-simple/composer.json @@ -0,0 +1,11 @@ +{ + "name": "marko/errors-simple", + "description": "Fixture stub for marko/errors-simple used by the marko/testing error handler fixture", + "type": "marko-module", + "license": "MIT", + "extra": { + "marko": { + "module": true + } + } +} diff --git a/packages/testing/tests/fixtures/errors-app/vendor/marko/errors-simple/module.php b/packages/testing/tests/fixtures/errors-app/vendor/marko/errors-simple/module.php new file mode 100644 index 00000000..bbc6f11c --- /dev/null +++ b/packages/testing/tests/fixtures/errors-app/vendor/marko/errors-simple/module.php @@ -0,0 +1,9 @@ + marko/ -> vendor/ -> errors-app/ -> fixtures/ -> tests/ -> testing/ -> packages/ +return require dirname(__DIR__, 7) . '/errors-simple/module.php'; diff --git a/packages/testing/tests/fixtures/pest-project/phpunit.xml b/packages/testing/tests/fixtures/pest-project/phpunit.xml index bcd26a5a..1337b8c3 100644 --- a/packages/testing/tests/fixtures/pest-project/phpunit.xml +++ b/packages/testing/tests/fixtures/pest-project/phpunit.xml @@ -9,6 +9,7 @@ diff --git a/packages/testing/tests/fixtures/pest-project/tests/BootsApplicationSpec.php b/packages/testing/tests/fixtures/pest-project/tests/BootsApplicationSpec.php new file mode 100644 index 00000000..a8384644 --- /dev/null +++ b/packages/testing/tests/fixtures/pest-project/tests/BootsApplicationSpec.php @@ -0,0 +1,14 @@ +application()->modules)->not->toBeEmpty(); +});