28: Prove the notice was there before asserting it is gone

tests/unit/Notices/PresenterTest.php

@@ -132,6 +132,11 @@ public function test_render_keeps_a_link_but_not_an_event_handler(): void {
 	public function test_render_clears_the_queue(): void {
 		$this->queue_notice( 'queue_merge_notice' );
 
+		// The queue has to be there before rendering can be what took it away: a writer that never
+		// wrote, or an option name the two halves disagree about, would satisfy the assertion below
+		// without a presenter having cleared anything.
+		$this->assertTrue( $this->queue_exists(), 'The queue must exist before it is rendered.' );
+
 		$presenter = $this->make_presenter();
 		$this->render_to_string( $presenter );
 
@@ -202,16 +207,83 @@ public function render( array $queue ): void {
 			}
 		};
 
+		$presenter = $this->make_presenter( null, $renderer );
+
+		// The control belongs in this test rather than in a sibling: a presenter that reached no
+		// renderer at all, or a renderer this one never received, would satisfy the assertion at the
+		// end for a reason that has nothing to do with the capability.
+		$this->queue_notice( 'queue_merge_notice' );
+
+		$this->render_to_string( $presenter );
+
+		$this->assertTrue( $renderer->called, 'This renderer must be reachable for someone who may consume the queue.' );
+
+		$renderer->called = false;
+
 		$this->queue_notice( 'queue_merge_notice' );
 
 		wp_set_current_user( $this->create_user( 'subscriber' ) );
 
-		$this->render_to_string( $this->make_presenter( null, $renderer ) );
+		$this->render_to_string( $presenter );
 
 		$this->assertFalse( $renderer->called );
 		$this->assertTrue( $this->queue_exists() );
 	}
 
+	/**
+	 * The capability is asked before the queue is read, not after.
+	 *
+	 * The gate gets its authority from being in front of both halves: reading is what the clearing
+	 * follows from, so a presenter that read the queue and then decided who may see it would already
+	 * have taken the notice out of the store by the time it turned the subscriber away. Asserting on
+	 * the output alone cannot see that difference, so the store here counts the calls it receives.
+	 */
+	public function test_the_capability_is_checked_before_the_queue_is_read(): void {
+		$store = new class() extends Store {
+			/**
+			 * @var int
+			 */
+			public $reads = 0;
+
+			/**
+			 * @var int
+			 */
+			public $clears = 0;
+
+			/**
+			 * @return array<string,string>
+			 */
+			public function all(): array {
+				++$this->reads;
+
+				return [ 'give-recurring:merge' => 'Bundled now.' ];
+			}
+
+			/**
+			 * @return void
+			 */
+			public function clear(): void {
+				++$this->clears;
+			}
+		};
+
+		$presenter = $this->make_presenter( $store );
+
+		wp_set_current_user( $this->create_user( 'subscriber' ) );
+
+		$this->assertSame( '', $this->render_to_string( $presenter ) );
+		$this->assertSame( 0, $store->reads, 'The queue must not be read at all for a user who may not consume it.' );
+		$this->assertSame( 0, $store->clears, 'And it must certainly not be cleared.' );
+
+		// The recorder has to be shown working, or "never read" and "never wired to anything" are the
+		// same result: the same store, in the same presenter, for a user who does have the capability.
+		$this->become_plugin_administrator();
+
+		$this->assertStringContainsString( 'Bundled now.', $this->render_to_string( $presenter ) );
+		$this->assertSame( 1, $store->reads );
+		$this->assertSame( 1, $store->clears );
+	}
+
 	/**
 	 * Rendering consumes the queue, so a user who cannot act on the notice must neither see it
 	 * nor destroy it. The merge notice is raised once and never re-queued.

tests/unit/Notices/RendererTest.php

@@ -3,6 +3,8 @@
  * @package Nexcess\PluginAbsorber
  */
 
+declare( strict_types=1 );
+
 namespace Nexcess\PluginAbsorber\Tests\Unit\Notices;
 
 use Codeception\TestCase\WPTestCase;
@@ -163,6 +165,26 @@ public static function empty_messages(): Generator {
 		yield 'a message that is only disallowed markup' => [ '<script></script>' ];
 	}
 
+	/**
+	 * Skipping is per message, not for the rest of the queue: an entry a host left empty — or one
+	 * `wp_kses_post()` emptied for it — must not take the notices behind it off the screen with it.
+	 * That is the difference between a `continue` and a `return`, and every case above holds a single
+	 * message, so none of them can see it.
+	 */
+	public function test_an_empty_message_does_not_stop_the_ones_behind_it(): void {
+		$output = $this->render(
+			[
+				'a:merge'      => 'First.',
+				'b:merge'      => '<script></script>',
+				'c:dependency' => 'Second.',
+			]
+		);
+
+		$this->assertStringContainsString( 'First.', $output );
+		$this->assertStringContainsString( 'Second.', $output );
+		$this->assertSame( 2, substr_count( $output, '<div class="notice' ), 'Two messages in, two notices out.' );
+	}
+
 	public function test_an_empty_queue_prints_nothing(): void {
 		$this->assertSame( '', $this->render( [] ) );
 	}

tests/unit/Notices/StoreTest.php

@@ -3,6 +3,8 @@
  * @package Nexcess\PluginAbsorber
  */
 
+declare( strict_types=1 );
+
 namespace Nexcess\PluginAbsorber\Tests\Unit\Notices;
 
 use Codeception\TestCase\WPTestCase;
@@ -11,6 +13,9 @@
 use Nexcess\PluginAbsorber\Exceptions\Config_Exception;
 use Nexcess\PluginAbsorber\Notices\Store;
 use Nexcess\PluginAbsorber\Tests\Support\Config_State;
+use RuntimeException;
+use WP_Error;
+use WP_Network;
 
 /**
  * The storage half of the queue, exercised without going through Queue.
@@ -28,6 +33,13 @@ class StoreTest extends WPTestCase {
 
 	private const OPTION_NORMALISED = 'give_core_plugin_absorber_notices';
 
+	/**
+	 * The second site the network-scope test reads the queue from, once it has one.
+	 *
+	 * @var int|null
+	 */
+	private $second_site_id = null;
+
 	public function setUp(): void {
 		parent::setUp();
 
@@ -37,6 +49,7 @@ public function setUp(): void {
 	}
 
 	public function tearDown(): void {
+		$this->delete_second_site();
 		delete_site_option( self::OPTION );
 		delete_site_option( self::OPTION_WOO );
 		delete_site_option( self::OPTION_NORMALISED );
@@ -83,6 +96,10 @@ public function test_clear_removes_the_row_entirely(): void {
 		$store = new Store();
 		$store->put( 'give-recurring:merge', 'Bundled now.' );
 
+		// The row has to be there before it can be taken away: a store that wrote nothing at all
+		// would satisfy every assertion below without ever having cleared anything.
+		$this->assertNotFalse( get_site_option( self::OPTION, false ), 'The queue must exist before it is cleared.' );
+
 		$store->clear();
 
 		$this->assertFalse( get_site_option( self::OPTION, false ), 'The option must be gone, not emptied.' );
@@ -179,4 +196,110 @@ public function test_the_queue_is_not_autoloaded(): void {
 
 		$this->assertNotContains( self::OPTION, array_keys( wp_load_alloptions() ) );
 	}
+
+	/**
+	 * The queue is one option for the whole network, not one per site.
+	 *
+	 * `get_site_option()`/`update_site_option()` rather than the plain pair is the only thing making
+	 * that true, and outside multisite the two are the same function — so nothing on the singlesite
+	 * leg can tell them apart, and swapping them would leave every other test in this class green.
+	 * The scope has to match the act it reports: `deactivate_plugins()` takes the standalone out of
+	 * the *network's* active plugins, and the merge notice explaining that is raised exactly once, so
+	 * a per-site queue would file it against whichever site the request happened to land on and no
+	 * administrator elsewhere would ever be told.
+	 */
+	public function test_the_queue_is_one_option_for_the_whole_network(): void {
+		if ( ! is_multisite() ) {
+			$this->markTestSkipped( 'Outside multisite there is one site, so there is no scope to cross.' );
+		}
+
+		( new Store() )->put( 'give-recurring:merge', 'Bundled now.' );
+
+		$this->assertSame(
+			[ 'give-recurring:merge' => 'Bundled now.' ],
+			( new Store() )->all(),
+			'The queue must be readable where it was written, or reading it elsewhere proves nothing.'
+		);
+
+		$this->second_site_id = $this->create_second_site();
+
+		switch_to_blog( $this->second_site_id );
+
+		try {
+			$elsewhere = ( new Store() )->all();
+		} finally {
+			// In a finally block so a read that throws cannot leave the rest of the suite running
+			// against the second site.
+			restore_current_blog();
+		}
+
+		$this->assertSame(
+			[ 'give-recurring:merge' => 'Bundled now.' ],
+			$elsewhere,
+			'The queue must reach every site on the network, not only the one that wrote it.'
+		);
+	}
+
+	/**
+	 * A second site on the current network, created with the same domain and a path of its own so
+	 * that it is unique on a subdomain install and a subdirectory one alike.
+	 *
+	 * @throws RuntimeException When there is no network, or the site cannot be created.
+	 *
+	 * @return int
+	 */
+	private function create_second_site(): int {
+		$network = get_network();
+
+		if ( ! $network instanceof WP_Network ) {
+			throw new RuntimeException( 'The multisite env has no current network.' );
+		}
+
+		// 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 );
+
+		$site_id = wpmu_create_blog(
+			$network->domain,
+			$network->path . 'absorber-queue-scope/',
+			'Queue scope',
+			0,
+			[],
+			get_current_network_id()
+		);
+
+		$wpdb->suppress_errors( $suppressing );
+
+		// `wpmu_create_blog()` puts WordPress into installing mode and never takes it back out, so
+		// without this every later test in the process runs as though the site were mid-install.
+		// Core's own blog factory ends with the same line, for the same reason.
+		wp_installing( false );
+
+		if ( $site_id instanceof WP_Error ) {
+			throw new RuntimeException( 'Could not create a second site: ' . $site_id->get_error_message() );
+		}
+
+		return $site_id;
+	}
+
+	/**
+	 * Creating a site is DDL, which MySQL commits outside the transaction the suite rolls back, so
+	 * the tables it made have to be dropped by hand rather than left to the rollback.
+	 *
+	 * @return void
+	 */
+	private function delete_second_site(): void {
+		if ( $this->second_site_id === null ) {
+			return;
+		}
+
+		wp_delete_site( $this->second_site_id );
+
+		$this->second_site_id = null;
+	}
 }

tests/unit/Notices/WriterTest.php

@@ -207,6 +207,11 @@ public function test_different_slugs_do_not_collide(): void {
 	 * Where notices are kept is a constructor argument, so a host can move the queue somewhere else
 	 * without also taking on how notices are worded. It is a required argument and it is bound by
 	 * `Provider`, so a host rebinds the store rather than subclassing the writer.
+	 *
+	 * The double answers `option_name()` too, and the writer is asked for it here, because that is
+	 * the whole of what a host reading the queue itself has to go on. A writer composing the name
+	 * from the hook prefix instead of delegating would agree with the default store on every other
+	 * test in this class and send that host to an option nothing writes.
 	 */
 	public function test_a_replacement_store_is_used_instead_of_the_option(): void {
 		$store = new class() extends Store {
@@ -215,6 +220,20 @@ public function test_a_replacement_store_is_used_instead_of_the_option(): void {
 			 */
 			public $written = [];
 
+			/**
+			 * Deliberately nothing the hook prefix could produce.
+			 *
+			 * @var string
+			 */
+			public $name = 'host_managed_notice_queue';
+
+			/**
+			 * @return string
+			 */
+			public function option_name(): string {
+				return $this->name;
+			}
+
 			/**
 			 * @return array<string,string>
 			 */
@@ -240,12 +259,18 @@ public function clear(): void {
 			}
 		};
 
-		$this->make_writer( $store )->queue_merge_notice(
+		$writer = $this->make_writer( $store );
+		$writer->queue_merge_notice(
 			$this->make_sub_plugin( [ 'conflict_notice_message' => static fn() => 'Bundled now.' ] )
 		);
 
 		$this->assertSame( [ 'give-recurring:merge' => 'Bundled now.' ], $store->written );
 		$this->assertFalse( $this->queue_exists(), 'The default option must not have been written to.' );
+		$this->assertSame(
+			$store->name,
+			$writer->option_name(),
+			'The writer must report where its store keeps the queue, not compose a name of its own.'
+		);
 	}
 
 	/**
@@ -280,6 +305,15 @@ public function test_a_corrupted_queue_heals_on_the_next_write(): void {
 	}
 
 	public function test_the_option_is_keyed_by_the_hook_prefix(): void {
+		// Under the host's own prefix the queue lands in the option this class asserts against. Shown
+		// first, and then taken away again, because otherwise its absence at the end of the test says
+		// only that setUp deleted it — which is true of an option nothing ever writes.
+		$this->make_writer()->queue_merge_notice( $this->make_sub_plugin() );
+
+		$this->assertTrue( $this->queue_exists(), 'The give-prefixed option is where this writer writes.' );
+
+		$this->clear_queue();
+
 		Config_State::reset();
 		Config::set_hook_prefix( 'woo' );
 
@@ -288,7 +322,7 @@ public function test_the_option_is_keyed_by_the_hook_prefix(): void {
 		$this->make_writer()->queue_merge_notice( $this->make_sub_plugin() );
 
 		$this->assertIsArray( get_site_option( self::OPTION_FOR_OTHER_PREFIX, false ) );
-		$this->assertFalse( $this->queue_exists() );
+		$this->assertFalse( $this->queue_exists(), 'Nothing may be left under the previous prefix.' );
 	}
 
 	public function test_queueing_needs_a_hook_prefix(): void {