11. Give every assertion something it can fail on

AGENTS.md

@@ -49,7 +49,7 @@ slic run unit --env multisite     # multisite
 slic run unit tests/unit/ConfigTest.php
 slic run unit "tests/unit/ConfigTest.php:test_it_rejects_invalid_hook_prefixes"
 
-composer test:unit                # wraps `slic run unit`
+composer test:unit                # both legs: `slic run unit` under singlesite, then multisite
 composer test:analysis            # PHPStan level 9 — must stay green on every PR
 ```
 

composer.json

@@ -51,7 +51,8 @@
             "phpstan analyse -c phpstan.neon.dist --memory-limit=1G"
         ],
         "test:unit": [
-            "slic run unit"
+            "slic run unit --env singlesite",
+            "slic run unit --env multisite"
         ]
     },
     "config": {

tests/README.md

@@ -11,6 +11,8 @@ slic cc build
 
 slic run unit                     # singlesite
 slic run unit --env multisite     # multisite
+
+composer test:unit                # both legs, singlesite then multisite
 ```
 
 CI runs both envs on PHP 7.4 and 8.5 — the ends of the supported range — against WordPress
@@ -267,6 +269,26 @@ The trait needs `UopzFunctions` on the same class, for the stub.
 a stub really can stop a code path before it reaches `exit`, and that the shared
 helper reports it when one does not.
 
+## An absence needs a control
+
+An assertion that something is *not* there passes just as happily when the place it looked was
+empty, and it goes on passing after the behaviour it was written for stops running. So the same
+read has to be shown holding something first:
+
+```php
+add_option( self::AUTOLOADED_CONTROL, 'Carried on every request.', '', 'yes' );
+
+$autoloaded = array_keys( wp_load_alloptions() );
+
+$this->assertContains( self::AUTOLOADED_CONTROL, $autoloaded );
+$this->assertNotContains( self::OPTION, $autoloaded );
+```
+
+That is the same rule as the recorder one — record calls, assert the list is empty, then invoke the
+stub once and assert it caught it — and it is why `Spy_Writer` records only the notice branches
+something reads back. A recorder no test reads is an invitation to assert it is empty and be told
+nothing, so a counter lands in the same change as the assertion that reads it.
+
 ## Expecting `_doing_it_wrong()`
 
 `setExpectedIncorrectUsage()` matches the first argument exactly, which for a
@@ -286,9 +308,13 @@ $loader->load_all();
 $this->assert_the_library_reported_incorrect_usage();
 ```
 
-An unexpected report still fails the test, because everything the listener sees
-is recorded and asserted to belong to this library. Call
-`stop_expecting_incorrect_usage()` from tearDown.
+An unexpected report still fails the test. `expect_incorrect_usage()` declares
+that *this library* may report a developer mistake and nothing else, so that is
+all the listener registers with WPTestCase: a report named for anything else —
+WordPress's own, another plugin's, a translation loaded before `init` — stays
+unexpected and fails the test as it always did. Registering whatever arrived
+would have given that protection away on the first report a test did expect,
+which is most of them. Call `stop_expecting_incorrect_usage()` from tearDown.
 
 ## The scenario suite
 

tests/_support/Spy_Writer.php

@@ -40,27 +40,13 @@ class Spy_Writer implements Writer_Interface {
 	 */
 	public $merge_notices = [];
 
-	/**
-	 * Slugs handed to queue_conflict_notice(), in order.
-	 *
-	 * @var string[]
-	 */
-	public $conflict_notices = [];
-
 	/**
 	 * Slugs handed to queue_dependency_notice(), in order.
 	 *
 	 * @var string[]
 	 */
 	public $dependency_notices = [];
 
-	/**
-	 * Slugs handed to queue_stranding_notice(), in order.
-	 *
-	 * @var string[]
-	 */
-	public $stranding_notices = [];
-
 	/**
 	 * @param Sub_Plugin $sub_plugin Sub-plugin concerned.
 	 *
@@ -71,12 +57,16 @@ public function queue_merge_notice( Sub_Plugin $sub_plugin ): void {
 	}
 
 	/**
+	 * Recorded nowhere, because nothing reads it: the conflict branch is asserted against the real
+	 * queue's own `slug:conflict` key, in `Conflict\ResolverTest`. A recorder with no reader is a
+	 * standing invitation to assert it is empty and be told nothing at all — so if a test here ever
+	 * needs this call counted, the counter lands in the same change as the assertion that reads it.
+	 *
 	 * @param Sub_Plugin $sub_plugin Sub-plugin concerned.
 	 *
 	 * @return void
 	 */
 	public function queue_conflict_notice( Sub_Plugin $sub_plugin ): void {
-		$this->conflict_notices[] = $sub_plugin->get_slug();
 	}
 
 	/**
@@ -89,12 +79,14 @@ public function queue_dependency_notice( Sub_Plugin $sub_plugin ): void {
 	}
 
 	/**
+	 * Recorded nowhere, for the reason given on queue_conflict_notice(): the stranding branch is
+	 * asserted against the real queue's own `slug:stranding` key.
+	 *
 	 * @param Sub_Plugin $sub_plugin Sub-plugin concerned.
 	 *
 	 * @return void
 	 */
 	public function queue_stranding_notice( Sub_Plugin $sub_plugin ): void {
-		$this->stranding_notices[] = $sub_plugin->get_slug();
 	}
 
 	/**

tests/_support/Traits/WithIncorrectUsage.php

@@ -19,8 +19,11 @@
  * observe — that the mistake is reported at all, and reported against this library rather than
  * swallowed or blamed on WordPress — is what these assertions pin.
  *
- * The expectation is registered from the report itself, so an unexpected report still fails the test:
- * anything the library reports is recorded here and asserted over.
+ * The expectation is registered from the report itself, but only for a report this library made.
+ * `expect_incorrect_usage()` declares that this library may report a developer mistake and nothing
+ * more, so that is the only thing registered with WPTestCase — a report from anywhere else is left
+ * unexpected and still fails the test. Registering whatever arrived would have handed WordPress's own
+ * protection away on the first report a test did expect, which is most of them.
  *
  * The *message* is recorded alongside the name, because "something was reported" is a weak thing to
  * assert on its own: a failed registry read, a missing hook prefix and the gate a test is actually
@@ -73,15 +76,24 @@ protected function expect_incorrect_usage(): void {
 
 		$reports  = &$this->incorrect_usage_reports;
 		$messages = &$this->incorrect_usage_messages;
+		$declared = self::declared_report_prefix();
 
-		$listener = function ( $function_name, $message = '' ) use ( &$reports, &$messages ): void {
+		$listener = function ( $function_name, $message = '' ) use ( &$reports, &$messages, $declared ): void {
 			if ( ! is_string( $function_name ) ) {
 				return;
 			}
 
 			$reports[]  = $function_name;
 			$messages[] = is_string( $message ) ? $message : '';
 
+			// Only what this library reports is what a caller of this trait declared it expects, so
+			// only that is registered with WPTestCase. A report from anywhere else — WordPress's
+			// own, another plugin's, a translation loaded before `init` — stays unexpected and
+			// still fails the test, which is the protection a blanket registration gave away.
+			if ( strpos( $function_name, $declared ) !== 0 ) {
+				return;
+			}
+
 			$this->setExpectedIncorrectUsage( $function_name );
 		};
 
@@ -104,7 +116,7 @@ protected function assert_the_library_reported_incorrect_usage(): void {
 
 		foreach ( $this->incorrect_usage_reports as $report ) {
 			$this->assertStringStartsWith(
-				'Nexcess\\PluginAbsorber\\',
+				self::declared_report_prefix(),
 				$report,
 				'The report has to name this library, or the host goes looking in WordPress.'
 			);
@@ -153,6 +165,21 @@ protected function stop_expecting_incorrect_usage(): void {
 		$this->incorrect_usage_messages = [];
 	}
 
+	/**
+	 * What a report has to be named for this trait to count it as the one a caller expects.
+	 *
+	 * Every report this library makes carries `__METHOD__`, so its namespace is the whole of the
+	 * declaration `expect_incorrect_usage()` stands for. A method rather than a constant: PHP 7.4
+	 * has no trait constants.
+	 *
+	 * @since 1.0.0
+	 *
+	 * @return string
+	 */
+	private static function declared_report_prefix(): string {
+		return 'Nexcess\\PluginAbsorber\\';
+	}
+
 	/**
 	 * Unhook whatever this trait last installed, if anything.
 	 *

tests/unit/Conflict/RedirectorTest.php

@@ -84,6 +84,15 @@ public static function request_uris(): Generator {
 		yield 'a query value carrying a line break' => [ '/wp-admin/edit.php?s=a%0D%0Ab', admin_url( 'edit.php?s=ab' ) ];
 		yield 'a query value carrying a null byte'  => [ '/wp-admin/edit.php?s=a%00b', admin_url( 'edit.php?s=ab' ) ];
 
+		// A query value is not always a scalar: `s[]=` is the shape a list of checked boxes comes
+		// back in, and wp_parse_str() hands it over as a nested array. The strip has to walk into
+		// one, because a pass that only looked at strings would step over the array whole and put
+		// the line break in the Location header inside a parameter of its own.
+		yield 'a line break inside an array query value' => [
+			'/wp-admin/edit.php?s[]=a%0D%0Ab&s[]=c',
+			admin_url( 'edit.php?s%5B0%5D=ab&s%5B1%5D=c' ),
+		];
+
 		// An admin root names the dashboard by leaving it out, exactly as core's own /wp-admin/ link
 		// does. Sending an admin who asked for the dashboard to the plugins list instead is the
 		// failure this whole class is about, in its most common form.
@@ -108,6 +117,49 @@ public static function request_uris(): Generator {
 		yield 'a uri naming another host' => [ '//evil.test/wp-admin/plugins.php', admin_url( 'plugins.php' ) ];
 	}
 
+	/**
+	 * The screen-name pattern is anchored with `\z` and not with `$`, and the whole of the
+	 * difference is a trailing newline: in PCRE `$` also matches immediately before one, so
+	 * "edit.php\n" would satisfy it and a line break would leave this class inside the one value it
+	 * promises is validated — on its way into a Location header.
+	 *
+	 * No case in the provider above can pin that, and not for want of trying: PHP's own parse_url()
+	 * rewrites every non-printable byte of a path to an underscore, so a request URI carrying a real
+	 * line break reaches the pattern as "edit.php_" and is refused for a different reason entirely.
+	 * That rewrite is an implementation detail of the parser rather than a promise this class is
+	 * entitled to lean on, and the anchor is what holds if it ever changes — so the parse is stubbed
+	 * to hand the path over verbatim, which is the only way to put the question to the pattern.
+	 *
+	 * The clean path is asserted first, under the same stub. Without it the fallback below would be
+	 * satisfied just as well by a stub that broke every destination, which is the wrong reason to
+	 * pass.
+	 */
+	public function test_it_refuses_a_screen_name_with_a_line_break_after_it(): void {
+		$this->setFunctionReturn( 'is_network_admin', false );
+		$this->setFunctionReturn( 'is_user_admin', false );
+
+		$this->setFunctionReturn(
+			'wp_parse_url',
+			static function ( $url, $component = -1 ) {
+				return $component === PHP_URL_QUERY ? null : $url;
+			},
+			true
+		);
+
+		$redirector = new Redirector();
+
+		$this->assertSame(
+			admin_url( 'edit.php' ),
+			$redirector->after_deactivation( '/wp-admin/edit.php' ),
+			'A path naming a screen still resolves to it, so the parse is not what refuses below.'
+		);
+
+		$this->assertSame(
+			admin_url( 'plugins.php' ),
+			$redirector->after_deactivation( "/wp-admin/edit.php\n" )
+		);
+	}
+
 	/**
 	 * $_SERVER carries whatever the SAPI put there, and a host may filter it besides, so the guard
 	 * is a runtime one rather than a promise the signature can keep.

tests/unit/Notices/StoreTest.php

@@ -33,6 +33,12 @@ class StoreTest extends WPTestCase {
 
 	private const OPTION_NORMALISED = 'give_core_plugin_absorber_notices';
 
+	/**
+	 * An option deliberately written with autoload on, so that "the queue is not in the bundle" is
+	 * read off a bundle that demonstrably holds something.
+	 */
+	private const AUTOLOADED_CONTROL = 'plugin_absorber_autoload_control';
+
 	/**
 	 * The second site the network-scope test reads the queue from, once it has one.
 	 *
@@ -53,6 +59,7 @@ public function tearDown(): void {
 		delete_site_option( self::OPTION );
 		delete_site_option( self::OPTION_WOO );
 		delete_site_option( self::OPTION_NORMALISED );
+		delete_option( self::AUTOLOADED_CONTROL );
 		Config_State::reset();
 		parent::tearDown();
 	}
@@ -186,6 +193,11 @@ public function test_it_needs_a_hook_prefix(): void {
 	/**
 	 * The queue is empty on nearly every request and only ever read in the admin, so it must not
 	 * ride along in the autoloaded bundle on every front-end request.
+	 *
+	 * The control option is what makes the absence mean anything. An `assertNotContains()` over an
+	 * empty bundle passes without the store having done a thing, and goes on passing for ever — so
+	 * an option written with autoload on is looked for first, out of a bundle re-read from the
+	 * database rather than off the cache entry the write above just touched.
 	 */
 	public function test_the_queue_is_not_autoloaded(): void {
 		if ( is_multisite() ) {
@@ -194,7 +206,18 @@ public function test_the_queue_is_not_autoloaded(): void {
 
 		( new Store() )->put( 'give-recurring:merge', 'Bundled now.' );
 
-		$this->assertNotContains( self::OPTION, array_keys( wp_load_alloptions() ) );
+		add_option( self::AUTOLOADED_CONTROL, 'Carried on every request.', '', 'yes' );
+
+		wp_cache_delete( 'alloptions', 'options' );
+
+		$autoloaded = array_keys( wp_load_alloptions() );
+
+		$this->assertContains(
+			self::AUTOLOADED_CONTROL,
+			$autoloaded,
+			'The bundle has to carry an autoloaded option, or the queue not being in it says nothing.'
+		);
+		$this->assertNotContains( self::OPTION, $autoloaded );
 	}
 
 	/**

tests/unit/Notices/WriterTest.php

@@ -35,6 +35,12 @@ class WriterTest extends WPTestCase {
 	// derived from the prefix rather than fixed.
 	private const OPTION_FOR_OTHER_PREFIX = 'woo_plugin_absorber_notices';
 
+	/**
+	 * An option deliberately written with autoload on, so that "the queue is not in the bundle" is
+	 * read off a bundle that demonstrably holds something.
+	 */
+	private const AUTOLOADED_CONTROL = 'plugin_absorber_writer_autoload_control';
+
 	public function setUp(): void {
 		parent::setUp();
 
@@ -46,6 +52,7 @@ public function setUp(): void {
 	public function tearDown(): void {
 		$this->clear_queue();
 		delete_site_option( self::OPTION_FOR_OTHER_PREFIX );
+		delete_option( self::AUTOLOADED_CONTROL );
 		Config_State::reset();
 		parent::tearDown();
 	}
@@ -361,6 +368,11 @@ public function test_queueing_needs_a_hook_prefix(): void {
 	/**
 	 * The queue is empty on nearly every request and only ever read in the admin, so it must not
 	 * ride along in the autoloaded bundle on every front-end request.
+	 *
+	 * The control option is what makes the absence mean anything. An `assertNotContains()` over an
+	 * empty bundle passes without the writer having done a thing, and goes on passing for ever — so
+	 * an option written with autoload on is looked for first, out of a bundle re-read from the
+	 * database rather than off the cache entry the write above just touched.
 	 */
 	public function test_the_queue_is_not_autoloaded(): void {
 		if ( is_multisite() ) {
@@ -369,7 +381,18 @@ public function test_the_queue_is_not_autoloaded(): void {
 
 		$this->make_writer()->queue_merge_notice( $this->make_sub_plugin() );
 
-		$this->assertNotContains( self::OPTION, array_keys( wp_load_alloptions() ) );
+		add_option( self::AUTOLOADED_CONTROL, 'Carried on every request.', '', 'yes' );
+
+		wp_cache_delete( 'alloptions', 'options' );
+
+		$autoloaded = array_keys( wp_load_alloptions() );
+
+		$this->assertContains(
+			self::AUTOLOADED_CONTROL,
+			$autoloaded,
+			'The bundle has to carry an autoloaded option, or the queue not being in it says nothing.'
+		);
+		$this->assertNotContains( self::OPTION, $autoloaded );
 	}
 
 	/**