diff --git a/tests/unit/AbsorberTest.php b/tests/unit/AbsorberTest.php index db179bc..f54f68f 100644 --- a/tests/unit/AbsorberTest.php +++ b/tests/unit/AbsorberTest.php @@ -3,6 +3,8 @@ * @package Nexcess\PluginAbsorber */ +declare( strict_types=1 ); + namespace Nexcess\PluginAbsorber\Tests\Unit; use Codeception\TestCase\WPTestCase; @@ -11,9 +13,11 @@ use Nexcess\PluginAbsorber\Config; use Nexcess\PluginAbsorber\Conflict\Resolver; use Nexcess\PluginAbsorber\Conflict\Rewriter; +use Nexcess\PluginAbsorber\Contracts\Provider_Interface; use Nexcess\PluginAbsorber\Exceptions\Config_Exception; use Nexcess\PluginAbsorber\Notices\Presenter; use Nexcess\PluginAbsorber\Notices\Writer; +use Nexcess\PluginAbsorber\Provider; use Nexcess\PluginAbsorber\Registry\Contracts\Registrar_Interface; use Nexcess\PluginAbsorber\Registry\Registrar; use Nexcess\PluginAbsorber\Sub_Plugin; @@ -23,34 +27,55 @@ use Nexcess\PluginAbsorber\Tests\Support\Spy_Registrar; use Nexcess\PluginAbsorber\Tests\Support\Spy_Rewriter; use Nexcess\PluginAbsorber\Tests\Support\Test_Container; +use Nexcess\PluginAbsorber\Tests\Support\Traits\WithBundledPlugins; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithContainer; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithIncorrectUsage; use RuntimeException; +use StellarWP\ContainerContract\ContainerInterface; use stdClass; use Throwable; /** * The public surface: the accessors, registration, and the two notice trampolines. * - * Boot timing lives in `Boot\SchedulerTest` and the load loop in `LoaderTest`, which is where - * those behaviours moved. What is left here is what a host actually calls. + * Boot timing lives in `Boot\SchedulerTest` and the load loop in `LoaderTest`, which is where those + * behaviours moved. What is left here is what a host actually calls — `boot()` included, for the two + * decisions the method makes itself rather than delegates: which provider runs, and when the flag + * that makes it idempotent is set. * * @since 1.0.0 */ class AbsorberTest extends WPTestCase { + use WithBundledPlugins; use WithContainer; use WithIncorrectUsage; + /** + * What `did_action( 'plugins_loaded' )` reported before a test rewound it, or null. + * + * @var int|null + */ + private $plugins_loaded_count = null; + public function setUp(): void { parent::setUp(); Absorber_State::reset(); Config_State::reset(); Config::set_hook_prefix( 'give' ); + $this->reset_bundled_plugin_loads(); } public function tearDown(): void { + // The counter is process-global, so a test that left it rewound would tell the next one it is + // still early enough to wire a plugins_loaded callback. + if ( $this->plugins_loaded_count !== null ) { + $GLOBALS['wp_actions']['plugins_loaded'] = $this->plugins_loaded_count; + $this->plugins_loaded_count = null; + } + $this->stop_expecting_incorrect_usage(); + $this->remove_bundled_plugin_files(); Absorber_State::reset(); Config_State::reset(); $this->tear_down_container(); @@ -100,6 +125,52 @@ public function test_a_binding_that_does_not_implement_its_interface_is_reported } } + /** + * The same report for a binding that is not an object at all — a configuration array a host meant + * to pass somewhere else, or a class name left as the string it was written as. `get_class()` + * fatals on every one of those, so the type is what the sentence names, and it has to be this + * sentence that arrives rather than a TypeError from inside this library. + * + * @dataProvider non_object_bindings + * + * @param mixed $bound What the host's binding resolves to. + * @param string $reported How the report has to name it. + */ + public function test_a_binding_that_is_not_an_object_is_reported_by_type( $bound, string $reported ): void { + $container = new Test_Container(); + $container->singleton( + Registrar_Interface::class, + static function () use ( $bound ) { + return $bound; + } + ); + $this->set_up_container( $container ); + + try { + Absorber::registrar(); + $this->fail( 'Expected a Config_Exception.' ); + } catch ( Config_Exception $exception ) { + $this->assertStringContainsString( Registrar_Interface::class, $exception->getMessage() ); + $this->assertStringContainsString( + sprintf( 'returned %s', $reported ), + $exception->getMessage(), + 'A host reading this has to be told what it bound, and a type is all there is to tell.' + ); + } + } + + /** + * A class name is the likeliest of these by far: it is what the host meant to bind, one `::class` + * short of having bound it. + * + * @return Generator + */ + public static function non_object_bindings(): Generator { + yield 'a class name left as a string' => [ Registrar::class, 'string' ]; + yield 'a configuration array' => [ [ 'registrar' => true ], 'array' ]; + yield 'a count' => [ 3, 'integer' ]; + } + /** * A bound factory may throw, and a container asked for something it cannot build throws its own * exception type; the contract is explicit that has() true does not promise get() succeeds. @@ -381,6 +452,107 @@ public function test_the_state_helper_clears_buffered_registrations(): void { $this->assertSame( [], Absorber::all() ); } + /** + * The boot flag is set last, after the wiring rather than in front of it, and this is the failure + * that ordering exists to prevent. A boot that threw on the way through — no container, a binding + * the container cannot build — has wired nothing at all, so a host that fixes its bootstrap and + * calls again has to get a working library. Set first, the second call returns early: nothing + * loads, nothing is reported, and the site looks entirely healthy. + * + * Asserted by loading a sub-plugin rather than by counting hooks, because "boot() ran again" is + * not the promise — the promise is that the library works afterwards. + */ + public function test_a_boot_that_failed_can_be_booted_again(): void { + $this->rewind_plugins_loaded(); + + $failed = false; + + try { + Absorber::boot(); + } catch ( Config_Exception $exception ) { + $failed = true; + } + + $this->assertTrue( $failed, 'Booting with no container has to fail, or this test is about nothing.' ); + + $this->set_up_container(); + $this->register_bundled_sub_plugin(); + + Absorber::boot(); + + $this->assertSame( 0, $this->bundled_plugin_loads(), 'The second boot must wire the load rather than run it.' ); + + do_action( 'plugins_loaded' ); + + $this->assertSame( 1, $this->bundled_plugin_loads(), 'The boot after the failure has to leave a working library.' ); + } + + /** + * boot() binds a `Provider_Interface` of its own only when nothing answers to one already, so a + * host may replace the whole set of bindings rather than rebinding them one at a time. + * + * What says the library's own provider stood down is an interface id: a container reports it can + * answer for any class that exists, bound or not, so a concrete binding would look present either + * way. Nothing can build an interface unprompted, so there `has()` means what it says. + */ + public function test_a_host_provider_replaces_the_default_bindings(): void { + $this->rewind_plugins_loaded(); + + $calls = []; + + $record = static function () use ( &$calls ): void { + $calls[] = 'register'; + }; + + $provider = new class( $record ) implements Provider_Interface { + /** + * @var callable + */ + private $record; + + /** + * @param callable $record Logs the call. + */ + public function __construct( callable $record ) { + $this->record = $record; + } + + /** + * @return void + */ + public function register(): void { + ( $this->record )(); + } + }; + + $container = $this->bare_container(); + $container->singleton( + Provider_Interface::class, + static function () use ( $provider ): Provider_Interface { + return $provider; + } + ); + + Config::set_container( $container ); + + Absorber::boot(); + + $this->assertSame( [ 'register' ], $calls, 'The host\'s provider is the one boot() has to run.' ); + $this->assertFalse( + $container->has( Registrar_Interface::class ), + 'The library\'s own provider must not have run beside it.' + ); + + // The probe has to be shown to work: an interface this library never binds and one it binds + // through a provider that never ran look exactly alike from here. + ( new Provider( $container ) )->register(); + + $this->assertTrue( + $container->has( Registrar_Interface::class ), + 'The default provider really does bind what the assertion above looked for.' + ); + } + public function test_render_notices_delegates_to_the_bound_presenter(): void { $presenter = $this->bind_presenter(); @@ -588,6 +760,60 @@ static function () use ( $rewriter ): Rewriter { return $rewriter; } + /** + * A container with nothing in it but a way to reach itself. + * + * `WithContainer::set_up_container()` runs this library's own provider over the container on the + * way past, which is the very thing the provider test is about not happening. What is kept is the + * container's binding to its own contract: `Boot\Scheduler` takes one, and boot() resolves the + * scheduler whichever provider ran. + * + * @return Test_Container + */ + private function bare_container(): Test_Container { + $container = new Test_Container(); + $container->singleton( + ContainerInterface::class, + static function () use ( $container ): ContainerInterface { + return $container; + } + ); + + return $container; + } + + /** + * Register a sub-plugin whose bundled file records that it was loaded. + * + * @return void + */ + private function register_bundled_sub_plugin(): void { + $constant = $this->make_guard_constant(); + + Absorber::register( + [ + 'slug' => 'give-recurring', + 'bundled_plugin_file' => $this->make_bundled_plugin_file( $constant ), + 'plugin_loaded_constant' => $constant, + ] + ); + } + + /** + * Put plugins_loaded back where a host bootstrap finds it. + * + * The harness has to dispatch the hook before it can run anything, so a boot in a test would + * rightly report that it is too late to wire and run the sequence inline instead. tearDown puts + * the counter back. + * + * @return void + */ + private function rewind_plugins_loaded(): void { + $this->plugins_loaded_count = did_action( 'plugins_loaded' ); + + unset( $GLOBALS['wp_actions']['plugins_loaded'] ); + } + /** * Bind a recording registrar, in the order a host binds one: before the provider fills in what is * missing. diff --git a/tests/unit/ActivatorTest.php b/tests/unit/ActivatorTest.php index c3eac7a..a6136b6 100644 --- a/tests/unit/ActivatorTest.php +++ b/tests/unit/ActivatorTest.php @@ -14,7 +14,10 @@ use Nexcess\PluginAbsorber\Sub_Plugin; use Nexcess\PluginAbsorber\Tests\Support\Config_State; use Nexcess\PluginAbsorber\Tests\Support\Traits\WithSubPlugins; +use Nexcess\PluginAbsorber\Tests\Support\Traits\WithUsers; use RuntimeException; +use WP_Error; +use WP_Network; /** * "Once, ever", and where that is recorded. @@ -28,6 +31,7 @@ */ class ActivatorTest extends WPTestCase { use WithSubPlugins; + use WithUsers; private const OPTION = 'give_plugin_absorber_activations'; @@ -210,6 +214,104 @@ public function test_a_throwing_callback_leaves_the_slug_unrecorded(): void { $this->assertCount( 1, $calls, 'The slug has to be retried, not skipped forever.' ); } + /** + * The record is a *site* option, so where there is a network it is the network's and every site + * reads the same one. + * + * That matches the deactivation it follows. `Plugin\Deactivator` leaves `deactivate_plugins()`'s + * `$network_wide` at core's null default, which takes the standalone out of the *network's* active + * plugins — so a per-site record would have the host's migration run again on every other site in + * the network, each of them creating tables for a plugin that was already merged once. + * + * Both halves are asserted because either alone is satisfiable by the wrong implementation: a + * record written per-site is still readable from the site that wrote it, and an option nothing + * wrote is absent from every site there is. + */ + public function test_the_record_is_network_wide(): void { + if ( ! is_multisite() ) { + $this->markTestSkipped( 'There is no second site to read the record from on singlesite.' ); + } + + $calls = []; + $sub_plugin = $this->recording_sub_plugin( $calls ); + + ( new Activator() )->maybe_run( $sub_plugin ); + + $this->assertFalse( + get_option( self::OPTION, false ), + 'A per-site option would be invisible to every other site in the network.' + ); + + $blog_id = $this->create_site(); + + switch_to_blog( $blog_id ); + + try { + $this->assertSame( + [ 'give-recurring' => true ], + $this->recorded(), + 'The record has to be readable from another site in the network.' + ); + + ( new Activator() )->maybe_run( $sub_plugin ); + } finally { + // In a finally so a failed assertion cannot leave the rest of the process running against + // the second site, or leave its tables behind. + restore_current_blog(); + wp_delete_site( $blog_id ); + } + + $this->assertCount( 1, $calls, 'A callback recorded on one site must not run again on another.' ); + } + + /** + * A second site on the same network, for the one assertion that needs somewhere else to read from. + * + * @throws RuntimeException When multisite has no network, or the site cannot be created — rather + * than switching to a blog id that was never made and reading options out + * of tables that do not exist. + * + * @return int + */ + private function create_site(): int { + $network = get_network(); + + if ( ! $network instanceof WP_Network ) { + throw new RuntimeException( 'Multisite with no network to create a site on.' ); + } + + $name = 'absorber-' . uniqid(); + $domain = $network->domain; + $path = $network->path . $name . '/'; + + // A subdomain network puts the new site in front of the network's domain instead of below its + // path. WPLoader installs a subdirectory network, but a fixture that only works on one of the + // two would fail as a broken test rather than as a broken library. + if ( is_subdomain_install() ) { + $domain = $name . '.' . $network->domain; + $path = $network->path; + } + + // Creating a site runs core's populate_options(), which calls delete_expired_transients(), whose + // DELETE self-joins the options table under two aliases. The suite runs inside a transaction on + // TEMPORARY tables, and MySQL cannot open one of those twice in a statement -- so the query + // fails, harmlessly, on a site that has no transients to expire. Suppressed for the one call + // rather than left to print a WordPress database error into every CI log. + global $wpdb; + + $suppressing = $wpdb->suppress_errors( true ); + + $blog_id = wpmu_create_blog( $domain, $path, 'Plugin Absorber', $this->create_user( 'administrator' ) ); + + $wpdb->suppress_errors( $suppressing ); + + if ( $blog_id instanceof WP_Error ) { + throw new RuntimeException( 'Could not create a second site: ' . $blog_id->get_error_message() ); + } + + return $blog_id; + } + /** * A sub-plugin whose activation callback appends itself to the given array. * diff --git a/tests/unit/Conflict/DetectorTest.php b/tests/unit/Conflict/DetectorTest.php index dad81ad..8d8fc45 100644 --- a/tests/unit/Conflict/DetectorTest.php +++ b/tests/unit/Conflict/DetectorTest.php @@ -288,6 +288,26 @@ public function test_it_stops_at_the_first_conflict_it_finds(): void { ); } + /** + * The short-circuit is a `return true`, not a `return`. A host that bundles two plugins and has a + * standalone still installed for the *second* of them has to have that conflict found: a probe + * answering from the first registration alone would leave the standalone active, nothing + * deactivated and nothing said — and every other multi-registration case in this file puts the + * conflicting sub-plugin first, so it would go on passing. + */ + public function test_it_walks_past_a_sub_plugin_that_is_not_in_conflict(): void { + $this->bind_checker_active_for( [ 'give-fee-recovery/give-fee-recovery.php' ] ); + $this->register(); + $this->register_fee_recovery(); + + $this->assertTrue( $this->detector()->has_conflict() ); + $this->assertSame( + [ 'give-recurring/give-recurring.php', 'give-fee-recovery/give-fee-recovery.php' ], + $this->asked, + 'Both standalones have to be asked about, in registration order.' + ); + } + /** * The probe reads the registry and nothing else, which is what keeps it cheap enough to ask of * every admin GET — and a duplicate slug is the one bootstrap mistake that read can still raise, @@ -465,16 +485,87 @@ public function is_active( string $basename ): bool { } /** - * Bind the recording checker in place of the default one. + * A checker that logs like the one above but answers per basename. * - * Bound before the provider runs, which is the only order that leaves it bound. + * One fixed answer cannot express the site this library is written for: several sub-plugins + * bundled, and a standalone still installed for only one of them. + * + * @param string[] $active Basenames reported active; every other one is reported inactive. + * + * @return Checker_Interface + */ + private function checker_active_for( array $active ): Checker_Interface { + $asked = &$this->asked; + + $record = static function ( string $basename ) use ( &$asked ): void { + $asked[] = $basename; + }; + + return new class( $record, $active ) implements Checker_Interface { + /** + * @var callable + */ + private $record; + + /** + * @var string[] + */ + private $active; + + /** + * @param callable $record Logs the basename asked about. + * @param string[] $active Basenames reported active. + */ + public function __construct( callable $record, array $active ) { + $this->record = $record; + $this->active = $active; + } + + /** + * @param string $basename Plugin basename. + * + * @return bool + */ + public function is_active( string $basename ): bool { + ( $this->record )( $basename ); + + return in_array( $basename, $this->active, true ); + } + }; + } + + /** + * Bind the recording checker in place of the default one. * * @param bool $active Whether every standalone is reported active. * * @return void */ private function bind_checker( bool $active ): void { - $checker = $this->recording_checker( $active ); + $this->install_checker( $this->recording_checker( $active ) ); + } + + /** + * Bind a checker that reports only the named basenames active. + * + * @param string[] $active Basenames reported active. + * + * @return void + */ + private function bind_checker_active_for( array $active ): void { + $this->install_checker( $this->checker_active_for( $active ) ); + } + + /** + * Put a checker into a container of its own, and configure the library with it. + * + * Bound before the provider runs, which is the only order that leaves it bound. + * + * @param Checker_Interface $checker Checker the detector is to be built with. + * + * @return void + */ + private function install_checker( Checker_Interface $checker ): void { $container = new Test_Container(); $container->singleton( Checker_Interface::class, diff --git a/tests/unit/Plugin/CheckerTest.php b/tests/unit/Plugin/CheckerTest.php index 3392c2d..815131a 100644 --- a/tests/unit/Plugin/CheckerTest.php +++ b/tests/unit/Plugin/CheckerTest.php @@ -3,6 +3,8 @@ * @package Nexcess\PluginAbsorber */ +declare( strict_types=1 ); + namespace Nexcess\PluginAbsorber\Tests\Unit\Plugin; use Codeception\TestCase\WPTestCase; @@ -10,6 +12,7 @@ use lucatume\WPBrowser\Traits\UopzFunctions; use Nexcess\PluginAbsorber\Plugin\Checker; use Nexcess\PluginAbsorber\Plugin\Contracts\Checker_Interface; +use RuntimeException; /** * Asking WordPress whether a plugin is active. @@ -18,6 +21,10 @@ * — which `learndash-core` has to, since it filters `option_active_plugins` so `is_plugin_active()` * does not report what is in the database — should not have to reimplement a deactivation to do it. * + * `Plugin\Loads_Plugin_Functions` is exercised here too, through the caller that reaches it on every + * request. The trait is two lines and no state, so it earns no file of its own — but which function + * name its guard tests is an invariant, and this is where a caller asks it the question. + * * @since 1.0.0 */ class CheckerTest extends WPTestCase { @@ -28,6 +35,13 @@ class CheckerTest extends WPTestCase { */ private $checker; + /** + * Throwaway WordPress roots written by this test, removed in tearDown. + * + * @var string[] + */ + private $wordpress_roots = []; + public function setUp(): void { parent::setUp(); @@ -35,6 +49,16 @@ public function setUp(): void { require_once ABSPATH . 'wp-admin/includes/plugin.php'; $this->checker = new Checker(); + + unset( $GLOBALS['absorber_plugin_functions_loads'] ); + } + + public function tearDown(): void { + $this->remove_wordpress_roots(); + + unset( $GLOBALS['absorber_plugin_functions_loads'] ); + + parent::tearDown(); } public function test_it_implements_the_contract(): void { @@ -103,4 +127,147 @@ static function () { $this->assertSame( 1, $calls ); } + + /** + * `Plugin\Loads_Plugin_Functions` guards on `deactivate_plugins()`, and it has to keep doing so. + * `is_plugin_active()` is a common third-party shim: guarded on that name, something else defining + * it stands the require down, the rest of `wp-admin/includes/plugin.php` never loads, and the + * first call to a function nobody shimmed is a fatal — on a site whose only symptom is having + * installed one more plugin. Nothing about the swap is visible until then. + * + * The missing-functions state is built rather than found. Every test in this file requires the + * real file in setUp so uopz has something to stub, a `require_once` cannot be undone for the rest + * of the process, and no test may depend on having run before whichever other one loads it — so + * ABSPATH is pointed at a fixture root and `function_exists()` answers about that one name the way + * it does on a front-end request, where WordPress has loaded none of this. + */ + public function test_it_loads_the_plugin_functions_when_they_are_missing(): void { + $asked = []; + $root = $this->make_wordpress_root(); + + $restore_root = $this->setConstant( 'ABSPATH', $root ); + $restore_probe = $this->setFunctionReturn( + 'function_exists', + static function ( $name ) use ( &$asked ) { + $asked[] = $name; + + // Every other question is answered as it really stands. is_callable() rather than a + // recursive function_exists(), which this closure has replaced. + return $name === 'deactivate_plugins' ? false : is_callable( $name ); + }, + true + ); + + try { + $this->checker->is_active( 'give-recurring/give-recurring.php' ); + } finally { + // Undone the moment the call under test returns rather than in tearDown, and in a finally + // so a throw above cannot strand either one. Both are process-global -- everything in the + // process reads ABSPATH and asks function_exists(), this test's own assertions included -- + // so the window they are wrong in has to be the call and nothing else. The trait's `@after` + // is the backstop. + $restore_probe(); + $restore_root(); + } + + $this->assertSame( + 'deactivate_plugins', + $asked[0] ?? '', + 'The guard has to ask about the one function no third party shims.' + ); + $this->assertSame( + 1, + $this->plugin_functions_loads(), + 'A missing deactivate_plugins() has to pull ABSPATH . wp-admin/includes/plugin.php in.' + ); + } + + /** + * The other half. With the functions already in memory — every admin request, and every request + * at all once anything else has loaded them — the guard stands down, so the check does not stat a + * file per sub-plugin per request. + */ + public function test_it_does_not_reload_the_plugin_functions_when_they_are_there(): void { + $root = $this->make_wordpress_root(); + $restore = $this->setConstant( 'ABSPATH', $root ); + + try { + $this->checker->is_active( 'give-recurring/give-recurring.php' ); + } finally { + $restore(); + } + + $this->assertSame( 0, $this->plugin_functions_loads() ); + + // The recorder has to be shown to work: a fixture that records nothing at all would leave the + // same zero however the guard had behaved. + require_once $root . 'wp-admin/includes/plugin.php'; + + $this->assertSame( 1, $this->plugin_functions_loads(), 'The fixture really does record its own load.' ); + } + + /** + * Write a WordPress root whose `wp-admin/includes/plugin.php` records that it was included. + * + * A new root every call. `require_once` dedupes by resolved path for the lifetime of the PHP + * process, so a fixture shared between two tests lets the second one pass without including + * anything at all. + * + * @throws RuntimeException When the fixture cannot be written, rather than reporting a load that + * never had anywhere to happen. + * + * @return string Root with a trailing slash, as ABSPATH carries. + */ + private function make_wordpress_root(): string { + $root = sys_get_temp_dir() . '/absorber-abspath-' . uniqid( '', true ) . '/'; + + if ( ! mkdir( $root . 'wp-admin/includes', 0777, true ) ) { + throw new RuntimeException( 'Could not write a WordPress root fixture at ' . $root ); + } + + $this->wordpress_roots[] = $root; + + file_put_contents( + $root . 'wp-admin/includes/plugin.php', + 'wordpress_roots as $root ) { + $file = $root . 'wp-admin/includes/plugin.php'; + + if ( file_exists( $file ) ) { + unlink( $file ); + } + + foreach ( [ 'wp-admin/includes', 'wp-admin', '' ] as $directory ) { + $path = $root . $directory; + + if ( is_dir( $path ) ) { + rmdir( $path ); + } + } + } + + $this->wordpress_roots = []; + } }