From a4f176a22d83637f856725701f6b79d5ef614013 Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Mon, 3 Aug 2026 14:52:51 +0200 Subject: [PATCH 1/6] Add Registrar and its interface --- src/Contracts/Registrar_Interface.php | 42 +++++++++ src/Registrar.php | 53 ++++++++++++ tests/unit/RegistrarTest.php | 117 ++++++++++++++++++++++++++ 3 files changed, 212 insertions(+) create mode 100644 src/Contracts/Registrar_Interface.php create mode 100644 src/Registrar.php create mode 100644 tests/unit/RegistrarTest.php diff --git a/src/Contracts/Registrar_Interface.php b/src/Contracts/Registrar_Interface.php new file mode 100644 index 0000000..5b262ce --- /dev/null +++ b/src/Contracts/Registrar_Interface.php @@ -0,0 +1,42 @@ + Keyed by slug. + */ + public function all(): array; + + /** + * @since 1.0.0 + * + * @return void + */ + public function reset(): void; +} diff --git a/src/Registrar.php b/src/Registrar.php new file mode 100644 index 0000000..17a7eb6 --- /dev/null +++ b/src/Registrar.php @@ -0,0 +1,53 @@ + Sub_Plugin map. + * + * @since 1.0.0 + */ +class Registrar implements Registrar_Interface { + /** + * @var array + */ + private $sub_plugins = []; + + /** + * Assigning by key rather than appending is what makes a re-registration replace in place: + * the entry keeps its original position, so a host that registers conditionally in two code + * paths gets one entry and an unchanged load order. + * + * @since 1.0.0 + * + * @param Sub_Plugin $sub_plugin Sub-plugin to register. + * + * @return void + */ + public function register( Sub_Plugin $sub_plugin ): void { + $this->sub_plugins[ $sub_plugin->get_slug() ] = $sub_plugin; + } + + /** + * @since 1.0.0 + * + * @return array + */ + public function all(): array { + return $this->sub_plugins; + } + + /** + * @since 1.0.0 + * + * @return void + */ + public function reset(): void { + $this->sub_plugins = []; + } +} diff --git a/tests/unit/RegistrarTest.php b/tests/unit/RegistrarTest.php new file mode 100644 index 0000000..9bd0c4e --- /dev/null +++ b/tests/unit/RegistrarTest.php @@ -0,0 +1,117 @@ + $slug, + 'bundled_plugin_file' => "/tmp/{$slug}/{$slug}.php", + 'plugin_loaded_constant' => strtoupper( str_replace( '-', '_', $slug ) ) . '_VERSION', + ] + ); + } + + public function test_it_satisfies_the_contract(): void { + $this->assertInstanceOf( Registrar_Interface::class, new Registrar() ); + } + + public function test_it_starts_empty(): void { + $this->assertSame( [], ( new Registrar() )->all() ); + } + + public function test_it_keys_registrations_by_slug(): void { + $registrar = new Registrar(); + $sub_plugin = $this->make_sub_plugin( 'give-recurring' ); + + $registrar->register( $sub_plugin ); + + $this->assertSame( [ 'give-recurring' => $sub_plugin ], $registrar->all() ); + } + + public function test_it_keeps_multiple_registrations(): void { + $registrar = new Registrar(); + + $registrar->register( $this->make_sub_plugin( 'give-recurring' ) ); + $registrar->register( $this->make_sub_plugin( 'give-fee-recovery' ) ); + + $this->assertCount( 2, $registrar->all() ); + $this->assertArrayHasKey( 'give-recurring', $registrar->all() ); + $this->assertArrayHasKey( 'give-fee-recovery', $registrar->all() ); + } + + public function test_it_preserves_registration_order(): void { + $registrar = new Registrar(); + + $registrar->register( $this->make_sub_plugin( 'give-recurring' ) ); + $registrar->register( $this->make_sub_plugin( 'give-fee-recovery' ) ); + + $this->assertSame( + [ 'give-recurring', 'give-fee-recovery' ], + array_keys( $registrar->all() ), + 'The load path iterates this map, so a host that registers a dependency first must see it first.' + ); + } + + public function test_registering_the_same_slug_twice_lets_the_last_one_win(): void { + $registrar = new Registrar(); + $first = $this->make_sub_plugin( 'give-recurring' ); + $second = $this->make_sub_plugin( 'give-recurring' ); + + $registrar->register( $first ); + $registrar->register( $second ); + + $this->assertCount( 1, $registrar->all() ); + $this->assertSame( $second, $registrar->all()['give-recurring'] ); + } + + /** + * Re-registering must not move an entry to the end, or a host that conditionally re-registers + * would silently reorder the load. + */ + public function test_re_registering_keeps_the_original_position(): void { + $registrar = new Registrar(); + + $registrar->register( $this->make_sub_plugin( 'give-recurring' ) ); + $registrar->register( $this->make_sub_plugin( 'give-fee-recovery' ) ); + $registrar->register( $this->make_sub_plugin( 'give-recurring' ) ); + + $this->assertSame( + [ 'give-recurring', 'give-fee-recovery' ], + array_keys( $registrar->all() ) + ); + } + + public function test_reset_empties_the_registry(): void { + $registrar = new Registrar(); + $registrar->register( $this->make_sub_plugin( 'give-recurring' ) ); + + $registrar->reset(); + + $this->assertSame( [], $registrar->all() ); + } + + public function test_registrars_do_not_share_state(): void { + $first = new Registrar(); + $second = new Registrar(); + + $first->register( $this->make_sub_plugin( 'give-recurring' ) ); + + $this->assertSame( [], $second->all() ); + } +} From ddd9016b05ca577320afa90ae610fa4871877872 Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Mon, 3 Aug 2026 14:58:29 +0200 Subject: [PATCH 2/6] Address review: document the load-order guarantee hosts depend on Registration order decides load order, and nothing said so where a host would read it. A host registering an add-on before the plugin it extends gets a class-not-found fatal at plugins_loaded. Also explain on the contract why reset() is on it: a container-bound singleton registrar survives a Loader reset, so an implementation that no-ops reset() leaks registrations between boots. --- README.md | 4 ++++ src/Contracts/Registrar_Interface.php | 6 ++++++ src/Registrar.php | 6 ++++-- tests/unit/RegistrarTest.php | 13 +------------ 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index f32fe12..28172de 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,10 @@ When a sub-plugin's standalone counterpart is still active: The load guard and the standalone basename are deliberately **two separate keys**. No constant does double duty as both a guard and a path resolver. +Sub-plugins load in **registration order**, so register a dependency before anything that extends it +at include time. Registering the same slug twice replaces the entry in place rather than moving it, +which keeps that order stable for a host that registers conditionally from more than one code path. + A value that is a `string` is always used as a value, never called — even when a function of that name exists. `dependency_check` and `activation_callback` are the two keys that are only ever callables, and a value under either that cannot be called is rejected when the sub-plugin is diff --git a/src/Contracts/Registrar_Interface.php b/src/Contracts/Registrar_Interface.php index 5b262ce..cc3a6a9 100644 --- a/src/Contracts/Registrar_Interface.php +++ b/src/Contracts/Registrar_Interface.php @@ -34,6 +34,12 @@ public function register( Sub_Plugin $sub_plugin ): void; public function all(): array; /** + * Empty the registry. + * + * On the contract rather than on the implementation because a container-bound singleton + * registrar survives a Loader reset — the same populated instance comes back on the next + * resolve. An implementation that no-ops this leaks registrations between boots. + * * @since 1.0.0 * * @return void diff --git a/src/Registrar.php b/src/Registrar.php index 17a7eb6..1f69e63 100644 --- a/src/Registrar.php +++ b/src/Registrar.php @@ -19,8 +19,10 @@ class Registrar implements Registrar_Interface { private $sub_plugins = []; /** - * Assigning by key rather than appending is what makes a re-registration replace in place: - * the entry keeps its original position, so a host that registers conditionally in two code + * Register a sub-plugin, replacing any earlier one with the same slug. + * + * Assigning by key rather than appending is what makes the replacement happen in place: the + * entry keeps its original position, so a host that registers conditionally from two code * paths gets one entry and an unchanged load order. * * @since 1.0.0 diff --git a/tests/unit/RegistrarTest.php b/tests/unit/RegistrarTest.php index 9bd0c4e..4091877 100644 --- a/tests/unit/RegistrarTest.php +++ b/tests/unit/RegistrarTest.php @@ -44,18 +44,7 @@ public function test_it_keys_registrations_by_slug(): void { $this->assertSame( [ 'give-recurring' => $sub_plugin ], $registrar->all() ); } - public function test_it_keeps_multiple_registrations(): void { - $registrar = new Registrar(); - - $registrar->register( $this->make_sub_plugin( 'give-recurring' ) ); - $registrar->register( $this->make_sub_plugin( 'give-fee-recovery' ) ); - - $this->assertCount( 2, $registrar->all() ); - $this->assertArrayHasKey( 'give-recurring', $registrar->all() ); - $this->assertArrayHasKey( 'give-fee-recovery', $registrar->all() ); - } - - public function test_it_preserves_registration_order(): void { + public function test_it_keeps_multiple_registrations_in_order(): void { $registrar = new Registrar(); $registrar->register( $this->make_sub_plugin( 'give-recurring' ) ); From f845456b780b1fbb5eec5495a2853a73b91da328 Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Mon, 3 Aug 2026 14:58:30 +0200 Subject: [PATCH 3/6] Plan: guard load_all against a foreign registrar Registrar_Interface::all() only declares `array`, so a host binding its own registrar that returns anything else would fatal inside plugins_loaded on the first predicate call. The rename half of this commit is gone: Task 7 now writes the fixture helper once, as a trait. --- docs/superpowers/plans/2026-07-31-plugin-absorber.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/superpowers/plans/2026-07-31-plugin-absorber.md b/docs/superpowers/plans/2026-07-31-plugin-absorber.md index d164e61..d637a20 100644 --- a/docs/superpowers/plans/2026-07-31-plugin-absorber.md +++ b/docs/superpowers/plans/2026-07-31-plugin-absorber.md @@ -3701,6 +3701,13 @@ Append these methods, and extend `reset()` as shown at the end: */ public static function load_all(): void { foreach ( self::all() as $sub_plugin ) { + // Registrar_Interface::all() only declares `array`. A host binding its own registrar + // that returns anything else would otherwise fatal inside plugins_loaded on the first + // predicate call -- the exact failure this library exists to prevent. + if ( ! $sub_plugin instanceof Sub_Plugin ) { + continue; + } + self::load( $sub_plugin ); } } From ecd2d61f8f34e4b4a635c379710cd9eee3df1c64 Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Tue, 11 Aug 2026 11:43:07 +0200 Subject: [PATCH 4/6] Address review: take the shared fixture, and name the re-registration case The registry test built its own sub-plugins from a slug. It uses WithSubPlugins from PR 7 instead, so the whole suite has one answer to what a well-formed sub-plugin looks like. The re-registration test said a host might "conditionally re-register" without saying when one would. It now names the case: a host registers every bundled sub-plugin in one routine at load and runs that routine again for a single slug once a licence check or a saved setting resolves. That is why the second call has to update in place -- moving the slug to the end puts an add-on ahead of the class it extends. --- tests/unit/RegistrarTest.php | 44 ++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/tests/unit/RegistrarTest.php b/tests/unit/RegistrarTest.php index 4091877..d001efa 100644 --- a/tests/unit/RegistrarTest.php +++ b/tests/unit/RegistrarTest.php @@ -8,24 +8,13 @@ use Codeception\TestCase\WPTestCase; use Nexcess\PluginAbsorber\Contracts\Registrar_Interface; use Nexcess\PluginAbsorber\Registrar; -use Nexcess\PluginAbsorber\Sub_Plugin; +use Nexcess\PluginAbsorber\Tests\Support\Traits\WithSubPlugins; /** * @since 1.0.0 */ class RegistrarTest extends WPTestCase { - /** - * @param string $slug Sub-plugin slug. - */ - private function make_sub_plugin( string $slug ): Sub_Plugin { - return new Sub_Plugin( - [ - 'slug' => $slug, - 'bundled_plugin_file' => "/tmp/{$slug}/{$slug}.php", - 'plugin_loaded_constant' => strtoupper( str_replace( '-', '_', $slug ) ) . '_VERSION', - ] - ); - } + use WithSubPlugins; public function test_it_satisfies_the_contract(): void { $this->assertInstanceOf( Registrar_Interface::class, new Registrar() ); @@ -37,7 +26,7 @@ public function test_it_starts_empty(): void { public function test_it_keys_registrations_by_slug(): void { $registrar = new Registrar(); - $sub_plugin = $this->make_sub_plugin( 'give-recurring' ); + $sub_plugin = $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ); $registrar->register( $sub_plugin ); @@ -47,8 +36,8 @@ public function test_it_keys_registrations_by_slug(): void { public function test_it_keeps_multiple_registrations_in_order(): void { $registrar = new Registrar(); - $registrar->register( $this->make_sub_plugin( 'give-recurring' ) ); - $registrar->register( $this->make_sub_plugin( 'give-fee-recovery' ) ); + $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ) ); + $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-fee-recovery' ] ) ); $this->assertSame( [ 'give-recurring', 'give-fee-recovery' ], @@ -59,8 +48,8 @@ public function test_it_keeps_multiple_registrations_in_order(): void { public function test_registering_the_same_slug_twice_lets_the_last_one_win(): void { $registrar = new Registrar(); - $first = $this->make_sub_plugin( 'give-recurring' ); - $second = $this->make_sub_plugin( 'give-recurring' ); + $first = $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ); + $second = $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ); $registrar->register( $first ); $registrar->register( $second ); @@ -70,15 +59,20 @@ public function test_registering_the_same_slug_twice_lets_the_last_one_win(): vo } /** - * Re-registering must not move an entry to the end, or a host that conditionally re-registers - * would silently reorder the load. + * Re-registering must update in place rather than move the entry to the end. + * + * A host registers all of its bundled sub-plugins in one routine at load, then runs that + * routine again for a single slug once something it could not know up front resolves — a + * licence check that came back, a setting saved in the admin. Moving that slug to the end + * puts it behind sub-plugins registered after it, and an add-on extending a class the moved + * sub-plugin defines would then load first and fatal. */ public function test_re_registering_keeps_the_original_position(): void { $registrar = new Registrar(); - $registrar->register( $this->make_sub_plugin( 'give-recurring' ) ); - $registrar->register( $this->make_sub_plugin( 'give-fee-recovery' ) ); - $registrar->register( $this->make_sub_plugin( 'give-recurring' ) ); + $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ) ); + $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-fee-recovery' ] ) ); + $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ) ); $this->assertSame( [ 'give-recurring', 'give-fee-recovery' ], @@ -88,7 +82,7 @@ public function test_re_registering_keeps_the_original_position(): void { public function test_reset_empties_the_registry(): void { $registrar = new Registrar(); - $registrar->register( $this->make_sub_plugin( 'give-recurring' ) ); + $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ) ); $registrar->reset(); @@ -99,7 +93,7 @@ public function test_registrars_do_not_share_state(): void { $first = new Registrar(); $second = new Registrar(); - $first->register( $this->make_sub_plugin( 'give-recurring' ) ); + $first->register( $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ) ); $this->assertSame( [], $second->all() ); } From 3b53216ba9eb3cde47bd94289803a8af31381cfe Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Tue, 11 Aug 2026 12:13:32 +0200 Subject: [PATCH 5/6] Address review: reject a duplicate slug instead of letting the last one win MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A slug is not just this map's key — it also names the sub-plugin's notices and its once-ever activation record. Last-wins dropped the losing sub-plugin from the load silently and handed its activation record to the winner, which is the expensive half of what register() could not previously tell apart: - the same sub-plugin registered twice, benign - two different sub-plugins claiming one slug, silent data loss Refusing the collision gets the second case right and turns the first into a fatal on the first page load in development. The message names both bundled files, because the registrations routinely come from different host plugins and the stack trace only shows the one that lost. The registry is left untouched, so a caller that catches still boots with a coherent registry. Nothing legitimate is lost. The re-registration case this previously protected — a licence check that resolves late, a setting saved in the admin — is already served by `enabled`, which is re-evaluated on every load rather than cached, so a host defers that decision with a callable instead of a second register() call. The load-order guarantee is unaffected and still tested. reset() clears the guard with the registry, so a host that boots twice in one process does not fatal on the second pass. --- README.md | 9 +- .../2026-07-31-plugin-absorber-design.md | 3 +- engineering-plan.md | 2 +- src/Contracts/Registrar_Interface.php | 5 +- src/Registrar.php | 32 ++++++-- tests/unit/RegistrarTest.php | 82 ++++++++++++++----- 6 files changed, 103 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 28172de..4fbf058 100644 --- a/README.md +++ b/README.md @@ -61,8 +61,13 @@ The load guard and the standalone basename are deliberately **two separate keys* double duty as both a guard and a path resolver. Sub-plugins load in **registration order**, so register a dependency before anything that extends it -at include time. Registering the same slug twice replaces the entry in place rather than moving it, -which keeps that order stable for a host that registers conditionally from more than one code path. +at include time. + +Register each slug exactly once. A slug also names the sub-plugin's notices and its once-ever +activation record, so a second registration under the same slug is refused with a +`Config_Exception` naming both bundled files rather than quietly dropping one of the two from the +load. Register unconditionally and put anything you cannot decide up front — a licence that may not +be active, a setting the site owner can change — in `enabled`, which is re-evaluated on every load. A value that is a `string` is always used as a value, never called — even when a function of that name exists. `dependency_check` and `activation_callback` are the two keys that are only ever diff --git a/docs/superpowers/specs/2026-07-31-plugin-absorber-design.md b/docs/superpowers/specs/2026-07-31-plugin-absorber-design.md index 720226d..3e5954b 100644 --- a/docs/superpowers/specs/2026-07-31-plugin-absorber-design.md +++ b/docs/superpowers/specs/2026-07-31-plugin-absorber-design.md @@ -241,7 +241,8 @@ message. This is documented in `tests/README.md`. `is_plugin_active` / `is_plugin_active_for_network`; `are_dependencies_met()` with and without a callable; `get_conflict_policy()` for string, for callable, and with the `…/conflict_policy` filter overriding both; both message getters, string and callable. -- **8 — `Registrar`.** register; `all()`; last-wins dedupe by slug; `reset()`. +- **8 — `Registrar`.** register; `all()`; a duplicate slug throws `Config_Exception` naming both + bundled files and leaves the registry untouched; `reset()` clears the guard with the registry. - **9 — `Loader::resolve()`.** No container → default instance; di52 container binding a custom `Registrar_Interface` → bound instance returned; memoized (identical instance twice); `Loader::reset()` clears both the memo and the registry. diff --git a/engineering-plan.md b/engineering-plan.md index 98c2c98..ad4662f 100644 --- a/engineering-plan.md +++ b/engineering-plan.md @@ -500,7 +500,7 @@ class Loader { // resolve()/registrar()/resolver()/notices()/activation() — see the Container section above. public static function register( array $config ): void { - self::registrar()->register( new Sub_Plugin( $config ) ); // last-wins dedupe by slug + self::registrar()->register( new Sub_Plugin( $config ) ); // throws on a duplicate slug } /** @return array */ diff --git a/src/Contracts/Registrar_Interface.php b/src/Contracts/Registrar_Interface.php index cc3a6a9..342c3b2 100644 --- a/src/Contracts/Registrar_Interface.php +++ b/src/Contracts/Registrar_Interface.php @@ -5,6 +5,7 @@ namespace Nexcess\PluginAbsorber\Contracts; +use Nexcess\PluginAbsorber\Exceptions\Config_Exception; use Nexcess\PluginAbsorber\Sub_Plugin; /** @@ -14,12 +15,14 @@ */ interface Registrar_Interface { /** - * Register a sub-plugin. Registering an existing slug replaces it. + * Register a sub-plugin. A slug may only be registered once. * * @since 1.0.0 * * @param Sub_Plugin $sub_plugin Sub-plugin to register. * + * @throws Config_Exception When the slug is already registered. + * * @return void */ public function register( Sub_Plugin $sub_plugin ): void; diff --git a/src/Registrar.php b/src/Registrar.php index 1f69e63..f85b97b 100644 --- a/src/Registrar.php +++ b/src/Registrar.php @@ -6,6 +6,7 @@ namespace Nexcess\PluginAbsorber; use Nexcess\PluginAbsorber\Contracts\Registrar_Interface; +use Nexcess\PluginAbsorber\Exceptions\Config_Exception; /** * Default registry: a plain slug => Sub_Plugin map. @@ -19,20 +20,41 @@ class Registrar implements Registrar_Interface { private $sub_plugins = []; /** - * Register a sub-plugin, replacing any earlier one with the same slug. + * Register a sub-plugin. * - * Assigning by key rather than appending is what makes the replacement happen in place: the - * entry keeps its original position, so a host that registers conditionally from two code - * paths gets one entry and an unchanged load order. + * A slug is an identity, not a key this map happens to use: it also names the sub-plugin's + * notices and its once-ever activation record. Letting a second registration win would drop + * the first sub-plugin from the load silently and hand its activation record to the winner, so + * the collision is refused instead. There is no legitimate second registration to protect — + * a decision the host cannot make up front belongs in the `enabled` callable, which is + * re-evaluated on every load, not in a second call to this method. * * @since 1.0.0 * * @param Sub_Plugin $sub_plugin Sub-plugin to register. * + * @throws Config_Exception When the slug is already registered. + * * @return void */ public function register( Sub_Plugin $sub_plugin ): void { - $this->sub_plugins[ $sub_plugin->get_slug() ] = $sub_plugin; + $slug = $sub_plugin->get_slug(); + + if ( isset( $this->sub_plugins[ $slug ] ) ) { + // Both files, because the two registrations routinely come from different host plugins + // and the stack trace only shows the one that lost. + throw new Config_Exception( + sprintf( + 'Two sub-plugins are registered under the slug "%1$s": %2$s and %3$s.' + . ' A slug must identify exactly one sub-plugin.', + $slug, + $this->sub_plugins[ $slug ]->get_bundled_plugin_file(), + $sub_plugin->get_bundled_plugin_file() + ) + ); + } + + $this->sub_plugins[ $slug ] = $sub_plugin; } /** diff --git a/tests/unit/RegistrarTest.php b/tests/unit/RegistrarTest.php index d001efa..08fe9da 100644 --- a/tests/unit/RegistrarTest.php +++ b/tests/unit/RegistrarTest.php @@ -7,6 +7,7 @@ use Codeception\TestCase\WPTestCase; use Nexcess\PluginAbsorber\Contracts\Registrar_Interface; +use Nexcess\PluginAbsorber\Exceptions\Config_Exception; use Nexcess\PluginAbsorber\Registrar; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithSubPlugins; @@ -46,38 +47,79 @@ public function test_it_keeps_multiple_registrations_in_order(): void { ); } - public function test_registering_the_same_slug_twice_lets_the_last_one_win(): void { + public function test_it_rejects_a_slug_that_is_already_registered(): void { $registrar = new Registrar(); - $first = $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ); - $second = $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ); + $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ) ); - $registrar->register( $first ); - $registrar->register( $second ); + $this->expectException( Config_Exception::class ); - $this->assertCount( 1, $registrar->all() ); - $this->assertSame( $second, $registrar->all()['give-recurring'] ); + $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ) ); } /** - * Re-registering must update in place rather than move the entry to the end. - * - * A host registers all of its bundled sub-plugins in one routine at load, then runs that - * routine again for a single slug once something it could not know up front resolves — a - * licence check that came back, a setting saved in the admin. Moving that slug to the end - * puts it behind sub-plugins registered after it, and an add-on extending a class the moved - * sub-plugin defines would then load first and fatal. + * The collision is between two sub-plugins the reader cannot see from the stack trace: the + * registrations come from different code paths, and often from different host plugins. Naming + * both bundled files is what turns the fatal into a diagnosis. */ - public function test_re_registering_keeps_the_original_position(): void { + public function test_the_rejection_names_the_slug_and_both_bundled_files(): void { $registrar = new Registrar(); + $registrar->register( + $this->make_sub_plugin( + [ + 'slug' => 'give-recurring', + 'bundled_plugin_file' => '/give/vendor/bundled/give-recurring.php', + ] + ) + ); + + try { + $registrar->register( + $this->make_sub_plugin( + [ + 'slug' => 'give-recurring', + 'bundled_plugin_file' => '/other/vendor/bundled/recurring.php', + ] + ) + ); + } catch ( Config_Exception $exception ) { + $this->assertStringContainsString( 'give-recurring', $exception->getMessage() ); + $this->assertStringContainsString( '/give/vendor/bundled/give-recurring.php', $exception->getMessage() ); + $this->assertStringContainsString( '/other/vendor/bundled/recurring.php', $exception->getMessage() ); + + return; + } + + $this->fail( 'register() accepted a duplicate slug instead of throwing.' ); + } + + public function test_a_rejected_registration_leaves_the_registry_untouched(): void { + $registrar = new Registrar(); + $first = $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ); + $registrar->register( $first ); + + try { + $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ) ); + } catch ( Config_Exception $exception ) { + $this->assertSame( [ 'give-recurring' => $first ], $registrar->all() ); + return; + } + + $this->fail( 'register() accepted a duplicate slug instead of throwing.' ); + } + + /** + * A host that boots twice in one process re-runs its registration routine, so the guard has to + * live in the same state `reset()` clears or the second boot fatals. + */ + public function test_reset_lets_a_slug_be_registered_again(): void { + $registrar = new Registrar(); $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ) ); - $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-fee-recovery' ] ) ); + + $registrar->reset(); $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ) ); - $this->assertSame( - [ 'give-recurring', 'give-fee-recovery' ], - array_keys( $registrar->all() ) - ); + $this->assertCount( 1, $registrar->all() ); } public function test_reset_empties_the_registry(): void { From f10cf0eaa4fb5f3b7fd002abe21f42c8e4697a02 Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Tue, 11 Aug 2026 12:49:38 +0200 Subject: [PATCH 6/6] Refactor Registrar and its interface: remove reset() method and introduce Registrar_State for test isolation This commit removes the reset() method from the Registrar class and the Registrar_Interface, as it is no longer needed for production code. Instead, a new Registrar_State helper class is introduced to manage the state of the Registrar during tests, allowing for proper isolation without exposing a reset method in the public API. This change ensures that the library maintains a clean interface while still supporting necessary test functionality. Additionally, related tests have been updated to reflect these changes. --- .../plans/2026-07-31-plugin-absorber.md | 313 ++++++++++++------ src/Contracts/Registrar_Interface.php | 13 - src/Registrar.php | 9 - tests/unit/RegistrarTest.php | 23 -- 4 files changed, 217 insertions(+), 141 deletions(-) diff --git a/docs/superpowers/plans/2026-07-31-plugin-absorber.md b/docs/superpowers/plans/2026-07-31-plugin-absorber.md index d637a20..715540c 100644 --- a/docs/superpowers/plans/2026-07-31-plugin-absorber.md +++ b/docs/superpowers/plans/2026-07-31-plugin-absorber.md @@ -33,7 +33,7 @@ Every task's requirements implicitly include this section. - **Branching:** stacked. Each branch cuts from the previous branch, and merges to `main` in order. Never open PR N+1 before PR N's branch exists. - **Commits:** no co-author trailers, ever. - **Every source file** carries a file-level docblock with `@package Nexcess\PluginAbsorber` and every method a docblock with `@since 1.0.0`. This binds `src/` only. Test classes and test support classes keep the file-level docblock, but their methods do not need `@since` — the test code in this plan's own tasks is written that way deliberately (ruled 2026-07-31). -- **No test-only seams in `src/`** (ruled 2026-08-11, PR 4 review). Production classes do not carry a `reset()` for the suite's benefit — that is API the library then supports forever. Tests clear static state by reflection instead, through a helper under `tests/_support/`. `Config` is served by `Nexcess\PluginAbsorber\Tests\Support\Config_State::reset()`; **every `Config::reset()` in the task blocks below means `Config_State::reset()`.** The same applies to `Registrar` and `Loader` when tasks 8 and 9 land. +- **No test-only seams in `src/`** (ruled 2026-08-11, PR 4 review). Production classes do not carry a `reset()` for the suite's benefit — that is API the library then supports forever. Tests clear static state by reflection instead, through a helper under `tests/_support/`. `Config` is served by `Nexcess\PluginAbsorber\Tests\Support\Config_State::reset()`; **every `Config::reset()` in the task blocks below means `Config_State::reset()`.** `Registrar` is served by `Tests\Support\Registrar_State::reset( Registrar $registrar )` and `Loader` by `Tests\Support\Loader_State::reset()`, which empties the memoized registrar through `Registrar_State` before discarding the memo; both are spelled out in full in the task blocks below. ## File Structure @@ -2321,7 +2321,15 @@ filter, last wins) and both network-activation branches.' **Interfaces:** - Consumes: `Sub_Plugin` (Task 7), and the `WithSubPlugins` trait (Task 7) for its fixtures — the test builds no sub-plugin of its own. -- Produces: `Registrar_Interface` with `register( Sub_Plugin $sub_plugin ): void`, `all(): array` returning `array` keyed by slug, and `reset(): void`. Task 9 resolves this interface; Tasks 11 and 12 iterate `all()`. +- Produces: `Registrar_Interface` with `register( Sub_Plugin $sub_plugin ): void` and `all(): array` returning `array` keyed by slug. Task 9 resolves this interface; Tasks 11 and 12 iterate `all()`. + +> The contract is those two methods and nothing else. A registry that can be emptied is only ever +> wanted by the suite, and a method on the contract is a promise to every host that implements it. +> The tests here need no such method — each one builds its own `Registrar`, so nothing leaks +> between them. Emptying an already-populated registrar first becomes necessary in Task 9, where a +> container-bound singleton survives a `Loader` reset, and that task adds +> `Tests\Support\Registrar_State::reset()` to do it by reflection. See the Global Constraint on +> test-only seams. - [ ] **Step 1: Cut the branch** @@ -2405,15 +2413,6 @@ class RegistrarTest extends WPTestCase { array_keys( $registrar->all() ) ); } - - public function test_reset_empties_the_registry(): void { - $registrar = new Registrar(); - $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ) ); - - $registrar->reset(); - - $this->assertSame( [], $registrar->all() ); - } } ``` @@ -2457,13 +2456,6 @@ interface Registrar_Interface { * @return array Keyed by slug. */ public function all(): array; - - /** - * @since 1.0.0 - * - * @return void - */ - public function reset(): void; } ``` @@ -2509,22 +2501,13 @@ class Registrar implements Registrar_Interface { public function all(): array { return $this->sub_plugins; } - - /** - * @since 1.0.0 - * - * @return void - */ - public function reset(): void { - $this->sub_plugins = []; - } } ``` - [ ] **Step 6: Run the tests to verify they pass** Run: `slic run unit` -Expected: PASS — 6 tests. +Expected: PASS — 5 tests. - [ ] **Step 7: Commit, push, open the PR** @@ -2542,9 +2525,11 @@ Usage: Why this way: keyed by slug so re-registering the same slug replaces rather than duplicates — a host that conditionally registers in two code paths gets one entry, not two loads. The interface ships in -this PR rather than in a contracts-only PR so it arrives with an implementation and tests. +this PR rather than in a contracts-only PR so it arrives with an implementation and tests. It +carries no way to empty the registry: that is wanted only by the suite, which clears the map by +reflection from `tests/_support/` rather than making every host implementation carry the method. -Verify: `slic run unit` — 6 tests.' +Verify: `slic run unit` — 5 tests.' ``` --- @@ -2554,7 +2539,7 @@ Verify: `slic run unit` — 6 tests.' **PR 9** · branch `09-loader-resolve` from `08-registrar` · 2 source files **Files:** -- Create: `src/Loader.php`, `tests/unit/LoaderResolveTest.php` +- Create: `src/Loader.php`, `tests/_support/Registrar_State.php`, `tests/_support/Loader_State.php`, `tests/unit/LoaderResolveTest.php` - Modify: `README.md` **Interfaces:** @@ -2564,7 +2549,8 @@ Verify: `slic run unit` — 6 tests.' - `Loader::registrar(): Registrar_Interface` - `Loader::register( array $config ): void` - `Loader::all(): array` - - `Loader::reset(): void` — clears the memo **and** the registry + - `Tests\Support\Loader_State::reset(): void` — clears the memo **and** the registry, for the suite only + - `Tests\Support\Registrar_State::reset( Registrar $registrar ): void` Tasks 10, 12 and 13 each add one accessor alongside their own interface. @@ -2574,7 +2560,146 @@ Verify: `slic run unit` — 6 tests.' git checkout 08-registrar && git checkout -b 09-loader-resolve ``` -- [ ] **Step 2: Write the failing test** +- [ ] **Step 2: Write `tests/_support/Registrar_State.php` and `tests/_support/Loader_State.php`** + +`Loader` is the second static facade in the library, and like `Config` it needs clearing between +tests without carrying a public `reset()` for the suite's benefit. Both helpers land here, in the +task that first needs them, modelled on `Config_State`: the properties are walked by reflection and +an unknown one is a `LogicException` rather than a silent leak, so state added to `Loader` later +fails loudly instead of surviving into the next test. + +`Registrar_State` is separate because emptying the registry is a different job from resetting the +facade — it acts on an instance, not on static state — and because the `Loader` reset needs it +before it drops the memo. + +```php +setAccessible( true ); + $property->setValue( $registrar, [] ); + } +} +``` + +```php + + */ + protected const DEFAULTS = [ + 'resolved' => [], + ]; + + /** + * Empty the registry, then return every static property of `Loader` to its default. + * + * The registrar is emptied before the memo is dropped, and not the other way round: when the + * registrar came from a container binding as a singleton, the container hands back the same + * populated instance on the next resolve, so discarding the memo alone leaves every + * registration in place. + * + * @throws LogicException When `Loader` has grown a static property this helper does not know + * about, rather than leaving it to leak between tests. + * + * @return void + */ + public static function reset(): void { + self::empty_registrar(); + + $reflection = new ReflectionClass( Loader::class ); + + foreach ( $reflection->getProperties( ReflectionProperty::IS_STATIC ) as $property ) { + $name = $property->getName(); + + if ( ! array_key_exists( $name, self::DEFAULTS ) ) { + throw new LogicException( + sprintf( 'Loader::$%s has no default in %s. Add one.', $name, self::class ) + ); + } + + $property->setAccessible( true ); + $property->setValue( null, self::DEFAULTS[ $name ] ); + } + } + + /** + * Empty the memoized registrar, when one was resolved and it is the shipped `Registrar`. + * + * A test that binds a registrar of its own owns that instance: it should build a fresh one per + * test, or give it its own way to clear itself. + * + * @return void + */ + private static function empty_registrar(): void { + $resolved = new ReflectionProperty( Loader::class, 'resolved' ); + + $resolved->setAccessible( true ); + + /** @var array $memo */ + $memo = $resolved->getValue(); + $registrar = $memo[ Registrar_Interface::class ] ?? null; + + if ( $registrar instanceof Registrar ) { + Registrar_State::reset( $registrar ); + } + } +} +``` + +- [ ] **Step 3: Write the failing test** ```php sub_plugins; } - - public function reset(): void { - $this->sub_plugins = []; - } }; $container = new Container(); @@ -2682,10 +2804,6 @@ class LoaderResolveTest extends WPTestCase { public function all(): array { return $this->sub_plugins; } - - public function reset(): void { - $this->sub_plugins = []; - } }; $container = new Container(); @@ -2697,11 +2815,11 @@ class LoaderResolveTest extends WPTestCase { $this->assertArrayHasKey( 'give-recurring', $bound->sub_plugins ); } - public function test_reset_clears_both_the_memo_and_the_registry(): void { + public function test_the_state_helper_clears_both_the_memo_and_the_registry(): void { Loader::register( $this->config( 'give-recurring' ) ); $first = Loader::registrar(); - Loader::reset(); + Loader_State::reset(); $this->assertSame( [], Loader::all(), 'The registry must be empty after reset.' ); $this->assertNotSame( $first, Loader::registrar(), 'The memo must be discarded after reset.' ); @@ -2709,12 +2827,12 @@ class LoaderResolveTest extends WPTestCase { } ``` -- [ ] **Step 3: Run it to verify it fails** +- [ ] **Step 4: Run it to verify it fails** Run: `slic run unit` Expected: FAIL — `Class "Nexcess\PluginAbsorber\Loader" not found`. -- [ ] **Step 4: Write `src/Loader.php`** +- [ ] **Step 5: Write `src/Loader.php`** Only resolution and registration in this PR. `boot()` and the load loop land in Task 11. @@ -2804,39 +2922,32 @@ class Loader { public static function all(): array { return self::registrar()->all(); } - - /** - * Discard every resolved collaborator and the registry. Test seam. - * - * @since 1.0.0 - * - * @return void - */ - public static function reset(): void { - if ( isset( self::$resolved[ Registrar_Interface::class ] ) ) { - self::registrar()->reset(); - } - - self::$resolved = []; - } } ``` -> `reset()` empties the registrar before discarding the memo. Dropping the memo alone is not -> enough: when the registrar came from a container binding as a singleton, the container hands back -> the same populated instance on the next resolve. +> The facade has no `reset()`. Nothing in production ever needs to un-resolve a collaborator — the +> memo is built once per request and dies with it — so the only caller would be the suite, and a +> public static method is a promise to every host that reads the class. `Loader_State::reset()` +> does the job from `tests/_support/` instead, emptying the registrar through `Registrar_State` +> before it discards the memo: dropping the memo alone is not enough, because when the registrar +> came from a container binding as a singleton the container hands back the same populated instance +> on the next resolve. +> +> That reflection reaches into the shipped `Registrar` only. A test that binds a registrar of its +> own — the two below do — owns that fake, and should build a fresh one per test or give it its own +> way to clear itself. That is test-side code, which is exactly where this seam belongs. -- [ ] **Step 5: Run the tests to verify they pass** +- [ ] **Step 6: Run the tests to verify they pass** Run: `slic run unit` Expected: PASS — 7 tests. -- [ ] **Step 6: Confirm static analysis is still clean** +- [ ] **Step 7: Confirm static analysis is still clean** Run: `composer test:analysis` Expected: `[OK] No errors`. -- [ ] **Step 7: Append to the README** +- [ ] **Step 8: Append to the README** ```markdown ### Rebinding a collaborator @@ -2860,13 +2971,13 @@ The container is **not** used to wire hooks — those stay plain static callback stays genuinely optional. ``` -- [ ] **Step 8: Commit, push, open the PR** +- [ ] **Step 9: Commit, push, open the PR** ```bash -git add src/Loader.php tests/unit/LoaderResolveTest.php README.md +git add src/Loader.php tests/_support/Registrar_State.php tests/_support/Loader_State.php tests/unit/LoaderResolveTest.php README.md git commit -m "Add Loader resolution and registration" git push -u origin 09-loader-resolve -gh pr create --base 08-registrar --title "Loader resolution and registration" --body 'What: `Loader::resolve()`, the `registrar()` accessor, `register()`, `all()`, and `reset()`. +gh pr create --base 08-registrar --title "Loader resolution and registration" --body 'What: `Loader::resolve()`, the `registrar()` accessor, `register()`, and `all()`. Usage: @@ -2882,9 +2993,11 @@ accessors with their own fallback logic, so adding a collaborator is one line. T checked with `has()` before `get()`, which is what keeps it optional — a host with no container, or with one that binds nothing, gets plain `new` instances. -`reset()` empties the registrar before dropping the memo. Discarding the memo alone is not enough: -a container-bound singleton hands back the same populated instance on the next resolve, so the -registry would survive a reset and leak between tests. +The facade carries no `reset()`: nothing in production un-resolves a collaborator, so it would be a +public promise made for the test suite alone. The suite clears the state by reflection from +`tests/_support/` instead, and `Loader_State::reset()` empties the registrar before dropping the +memo — discarding the memo alone is not enough, since a container-bound singleton hands back the +same populated instance on the next resolve and the registry would survive the reset. Verify: `slic run unit` — 7 tests, covering both the bound and unbound paths with a real di52 container.' @@ -2935,6 +3048,7 @@ use Codeception\TestCase\WPTestCase; use Nexcess\PluginAbsorber\Config; use Nexcess\PluginAbsorber\Loader; use Nexcess\PluginAbsorber\Notices; +use Nexcess\PluginAbsorber\Tests\Support\Loader_State; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithSubPlugins; /** @@ -2954,7 +3068,7 @@ class NoticesTest extends WPTestCase { public function tearDown(): void { delete_transient( self::TRANSIENT ); - Loader::reset(); + Loader_State::reset(); Config::reset(); parent::tearDown(); } @@ -3371,7 +3485,7 @@ redirect (queue with one instance, render with another).' **PR 11** · branch `11-loader-load-path` from `10-notices-queue` · 2 source files **Files:** -- Modify: `src/Loader.php`, `README.md` +- Modify: `src/Loader.php`, `tests/_support/Loader_State.php` (the new `$booted` property needs a default), `README.md` - Create: `tests/unit/LoaderLoadTest.php`, `tests/unit/LoaderBootTest.php` **Interfaces:** @@ -3408,6 +3522,7 @@ namespace Nexcess\PluginAbsorber\Tests\Unit; use Codeception\TestCase\WPTestCase; use Nexcess\PluginAbsorber\Config; use Nexcess\PluginAbsorber\Loader; +use Nexcess\PluginAbsorber\Tests\Support\Loader_State; /** * @since 1.0.0 @@ -3435,7 +3550,7 @@ class LoaderLoadTest extends WPTestCase { unset( $GLOBALS['absorber_loads'] ); delete_transient( 'give_plugin_absorber_notices' ); - Loader::reset(); + Loader_State::reset(); Config::reset(); parent::tearDown(); } @@ -3601,6 +3716,7 @@ namespace Nexcess\PluginAbsorber\Tests\Unit; use Codeception\TestCase\WPTestCase; use Nexcess\PluginAbsorber\Config; use Nexcess\PluginAbsorber\Loader; +use Nexcess\PluginAbsorber\Tests\Support\Loader_State; /** * @since 1.0.0 @@ -3615,7 +3731,7 @@ class LoaderBootTest extends WPTestCase { public function tearDown(): void { remove_all_actions( 'plugins_loaded' ); remove_all_actions( 'admin_notices' ); - Loader::reset(); + Loader_State::reset(); Config::reset(); parent::tearDown(); } @@ -3666,7 +3782,7 @@ Add the `$booted` property beside `$resolved`: private static $booted = false; ``` -Append these methods, and extend `reset()` as shown at the end: +Append these methods: ```php /** @@ -3765,29 +3881,30 @@ Append these methods, and extend `reset()` as shown at the end: } ``` -Then extend `reset()` so the boot flag clears too: +- [ ] **Step 6: Teach `tests/_support/Loader_State.php` about the boot flag** -```php - public static function reset(): void { - if ( isset( self::$resolved[ Registrar_Interface::class ] ) ) { - self::registrar()->reset(); - } +`Loader_State::reset()` walks `Loader`'s static properties and refuses one it has no default for, +so until `$booted` is listed every test that resets throws a `LogicException` naming it. That is +the helper doing its job: a boot flag left standing would wire the hooks once and then let every +later test's `boot()` no-op. - self::$resolved = []; - self::$booted = false; - } +```php + protected const DEFAULTS = [ + 'resolved' => [], + 'booted' => false, + ]; ``` -- [ ] **Step 6: Run the tests to verify they pass** +- [ ] **Step 7: Run the tests to verify they pass** Run: `slic run unit` Expected: PASS — 9 load tests + 3 boot tests. -- [ ] **Step 7: Confirm static analysis is still clean** +- [ ] **Step 8: Confirm static analysis is still clean** Run: `composer test:analysis` -- [ ] **Step 8: Append to the README** +- [ ] **Step 9: Append to the README** ```markdown ### Bootstrap @@ -3831,10 +3948,10 @@ A sub-plugin is skipped when it is disabled, its dependencies are unmet, its gua already defined, its bundled file is missing, or this filter returns false. ``` -- [ ] **Step 9: Commit, push, open the PR** +- [ ] **Step 10: Commit, push, open the PR** ```bash -git add src/Loader.php tests/unit/LoaderLoadTest.php tests/unit/LoaderBootTest.php README.md +git add src/Loader.php tests/_support/Loader_State.php tests/unit/LoaderLoadTest.php tests/unit/LoaderBootTest.php README.md git commit -m "Add Loader boot and the load path" git push -u origin 11-loader-load-path gh pr create --base 10-notices-queue --title "Loader boot and load path" --body 'What: `boot()`, `load_all()`, the five-gate load path, and the `should_load` filter. @@ -3901,6 +4018,7 @@ use Nexcess\PluginAbsorber\Conflict\Resolver; use Nexcess\PluginAbsorber\Conflict_Policy; use Nexcess\PluginAbsorber\Loader; use Nexcess\PluginAbsorber\Sub_Plugin; +use Nexcess\PluginAbsorber\Tests\Support\Loader_State; use Nexcess\PluginAbsorber\Tests\Support\TestException; use lucatume\WPBrowser\Traits\UopzFunctions; @@ -3964,7 +4082,7 @@ class ResolverTest extends WPTestCase { public function tearDown(): void { delete_transient( 'give_plugin_absorber_notices' ); - Loader::reset(); + Loader_State::reset(); Config::reset(); parent::tearDown(); } @@ -4511,6 +4629,7 @@ use Codeception\TestCase\WPTestCase; use Nexcess\PluginAbsorber\Activation; use Nexcess\PluginAbsorber\Config; use Nexcess\PluginAbsorber\Loader; +use Nexcess\PluginAbsorber\Tests\Support\Loader_State; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithSubPlugins; /** @@ -4530,7 +4649,7 @@ class ActivationTest extends WPTestCase { public function tearDown(): void { delete_option( self::OPTION ); - Loader::reset(); + Loader_State::reset(); Config::reset(); parent::tearDown(); } @@ -4882,6 +5001,7 @@ use Codeception\TestCase\WPTestCase; use Nexcess\PluginAbsorber\Config; use Nexcess\PluginAbsorber\Loader; use Nexcess\PluginAbsorber\Notices; +use Nexcess\PluginAbsorber\Tests\Support\Loader_State; /** * @since 1.0.0 @@ -4914,7 +5034,7 @@ class NoticesActivationErrorTest extends WPTestCase { public function tearDown(): void { unset( $_GET['plugin'], $_GET['_error_nonce'] ); set_current_screen( 'front' ); - Loader::reset(); + Loader_State::reset(); Config::reset(); parent::tearDown(); } @@ -5391,6 +5511,7 @@ use Codeception\TestCase\WPTestCase; use Nexcess\PluginAbsorber\Config; use Nexcess\PluginAbsorber\Conflict_Policy; use Nexcess\PluginAbsorber\Loader; +use Nexcess\PluginAbsorber\Tests\Support\Loader_State; use Nexcess\PluginAbsorber\Tests\Support\TestException; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithBundledPlugins; use lucatume\WPBrowser\Traits\UopzFunctions; @@ -5432,7 +5553,7 @@ class EndToEndTest extends WPTestCase { delete_option( self::OPTION ); update_option( 'active_plugins', [] ); unset( $GLOBALS['absorber_loads'] ); - Loader::reset(); + Loader_State::reset(); Config::reset(); parent::tearDown(); } diff --git a/src/Contracts/Registrar_Interface.php b/src/Contracts/Registrar_Interface.php index 342c3b2..27f54c1 100644 --- a/src/Contracts/Registrar_Interface.php +++ b/src/Contracts/Registrar_Interface.php @@ -35,17 +35,4 @@ public function register( Sub_Plugin $sub_plugin ): void; * @return array Keyed by slug. */ public function all(): array; - - /** - * Empty the registry. - * - * On the contract rather than on the implementation because a container-bound singleton - * registrar survives a Loader reset — the same populated instance comes back on the next - * resolve. An implementation that no-ops this leaks registrations between boots. - * - * @since 1.0.0 - * - * @return void - */ - public function reset(): void; } diff --git a/src/Registrar.php b/src/Registrar.php index f85b97b..3c0ca7d 100644 --- a/src/Registrar.php +++ b/src/Registrar.php @@ -65,13 +65,4 @@ public function register( Sub_Plugin $sub_plugin ): void { public function all(): array { return $this->sub_plugins; } - - /** - * @since 1.0.0 - * - * @return void - */ - public function reset(): void { - $this->sub_plugins = []; - } } diff --git a/tests/unit/RegistrarTest.php b/tests/unit/RegistrarTest.php index 08fe9da..cf4ffae 100644 --- a/tests/unit/RegistrarTest.php +++ b/tests/unit/RegistrarTest.php @@ -108,29 +108,6 @@ public function test_a_rejected_registration_leaves_the_registry_untouched(): vo $this->fail( 'register() accepted a duplicate slug instead of throwing.' ); } - /** - * A host that boots twice in one process re-runs its registration routine, so the guard has to - * live in the same state `reset()` clears or the second boot fatals. - */ - public function test_reset_lets_a_slug_be_registered_again(): void { - $registrar = new Registrar(); - $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ) ); - - $registrar->reset(); - $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ) ); - - $this->assertCount( 1, $registrar->all() ); - } - - public function test_reset_empties_the_registry(): void { - $registrar = new Registrar(); - $registrar->register( $this->make_sub_plugin( [ 'slug' => 'give-recurring' ] ) ); - - $registrar->reset(); - - $this->assertSame( [], $registrar->all() ); - } - public function test_registrars_do_not_share_state(): void { $first = new Registrar(); $second = new Registrar();