Commit 396b5e4688f for woocommerce
commit 396b5e4688f2190d431cb346d9400b313453c631
Author: Vlad Olaru <vlad.olaru@automattic.com>
Date: Fri Nov 13 17:10:04 2026 +0200
[tests] Stop the logger spy leaking into later tests and make ProvidersTest check real providers (#70849)
* test: Stop the logger spy filter leaking into later tests
LoggerSpyTrait added its woocommerce_logging_class filter in a @before
method, which PHPUnit runs before setUp(). WordPress's test case
snapshots all hooks once per process, in the first test's set_up(), and
restores that snapshot after every test. When a class using the trait
ran first, the spy filter joined the snapshot and came back after every
later test, so wc_get_logger() returned the first test's mock for the
rest of the run. With a random order that puts AnalyticsTest first,
WC_Order_Step_Logger_Functions_Test's debug-level test then finds no
log file and fails.
Override set_up() in the trait instead and add the spy after
parent::set_up(). The filter stays out of the snapshot, the normal
restore removes it, and the spy is still in place for the rest of the
test class's own setUp(), which NotificationsProcessorTests relies on.
The @before/@after methods and the explicit removal are no longer
needed.
Add a test that pins the timing: no spy filter before the base set-up,
spy in place right after it. It holds in any test order.
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
* test: Register fulfillment hooks per test in ProvidersTest
ProvidersTest registered the fulfillments controller's hooks only in
setUpBeforeClass(). WordPress's test case restores its hook snapshot
after every test, so those hooks were gone after the first test unless
the class happened to run first in the process. In a normal suite run
the providers endpoint then returned an empty list, and the loop in
test_response_format_validation checked nothing: the class made 11
assertions in the suite against 473 when run alone.
Register the hooks in setUp() as well, and assert the providers list is
not empty before checking each provider's shape. The class-level setup
stays because it creates the fulfillments tables outside a test
transaction.
Add the changelog entry for this branch.
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
* test: Check the logger spy filter where the base set-up starts
LoggerSpyTraitTest recorded whether the spy filter existed before its
own setUp() called parent::setUp(). That runs before the trait's
set_up() is entered, so a trait that added the filter inside set_up()
but ahead of parent::set_up() would still pass, while the filter would
again join WordPress's hook snapshot and leak.
Record it in an intermediate base class's set_up() instead, which runs
when the trait calls parent::set_up(): the last point before the
snapshot can be taken. With the filter moved ahead of parent::set_up()
in the trait, the previous test passed and this one fails.
Raised in review on #70849.
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
* test: Fail logger spy tests that run without the spy
LoggerSpyTrait adds its spy from set_up(). A test class that defines
its own set_up() replaces the trait's, so no spy is installed: the
assert helpers then report a misleading "no entries" failure, and
assertNoErrorLogged() or a direct assertEmpty( $this->captured_logs )
pass whatever is logged. The trait now fails such tests with a message
that names the cause, from the helpers and from
assert_post_conditions(), which also covers direct reads of
captured_logs. No class overrides set_up() today; a throwaway class
that did passed both kinds of test before and fails both now.
LoggerSpyTraitTest only checked for the trait's own callback before
the base set-up, so a filter added there through a closure would leak
and still pass. It now checks that woocommerce_logging_class has no
callback at all at that point, which nothing in src/ or includes/
adds; the closure version now fails it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
diff --git a/plugins/woocommerce/changelog/test-fix-logger-spy-hook-leak b/plugins/woocommerce/changelog/test-fix-logger-spy-hook-leak
new file mode 100644
index 00000000000..116f9156391
--- /dev/null
+++ b/plugins/woocommerce/changelog/test-fix-logger-spy-hook-leak
@@ -0,0 +1,3 @@
+Significance: patch
+Type: dev
+Comment: Fix a test logger spy leak and a fulfillment providers test that checked nothing; no production change.
diff --git a/plugins/woocommerce/tests/php/helpers/LoggerSpyTrait.php b/plugins/woocommerce/tests/php/helpers/LoggerSpyTrait.php
index 1aaf7a7201b..9e1b7df85d3 100644
--- a/plugins/woocommerce/tests/php/helpers/LoggerSpyTrait.php
+++ b/plugins/woocommerce/tests/php/helpers/LoggerSpyTrait.php
@@ -30,14 +30,18 @@ trait LoggerSpyTrait {
private $spy_logger;
/**
- * Set up the spy logger.
+ * Set up the spy logger once the base set-up has run.
*
- * This method is called automatically before each test via the @before annotation.
+ * WordPress snapshots the hooks in its first set_up() of the process and restores that snapshot
+ * after every test. Adding the filter after it keeps the spy out of the snapshot, so the
+ * restore removes it, while it is still in place for the rest of the test's own setUp().
+ * A set_up() in the using class replaces this one, so override setUp() there instead.
*
- * @before
* @return void
*/
- protected function set_up_spy_logger(): void {
+ public function set_up() {
+ parent::set_up();
+
$this->captured_logs = array();
$this->spy_logger = $this->getMockBuilder( \WC_Logger_Interface::class )->getMock();
@@ -69,16 +73,26 @@ trait LoggerSpyTrait {
}
/**
- * Tear down the spy logger.
+ * Fail a test that passed without the spy in place.
*
- * This method is called automatically after each test via the @after annotation.
+ * Tests that read $this->captured_logs directly would otherwise pass with nothing captured.
*
- * @after
* @return void
*/
- protected function tear_down_spy_logger(): void {
- remove_filter( 'woocommerce_logging_class', array( $this, 'get_spy_logger' ) );
- $this->captured_logs = array();
+ protected function assert_post_conditions() {
+ parent::assert_post_conditions();
+ $this->assert_spy_logger_installed();
+ }
+
+ /**
+ * Fail when the trait's set_up() did not run, so no log was captured.
+ *
+ * @return void
+ */
+ private function assert_spy_logger_installed(): void {
+ if ( null === $this->spy_logger ) {
+ $this->fail( 'LoggerSpyTrait installed no spy logger: a set_up() in the test class replaces the trait\'s. Override setUp() instead.' );
+ }
}
/**
@@ -147,6 +161,8 @@ trait LoggerSpyTrait {
* @return array<array{level: string, message: string, context: array}>
*/
private function get_logs_by_level( string $level ): array {
+ $this->assert_spy_logger_installed();
+
return array_filter(
$this->captured_logs,
fn( $log ) => $log['level'] === $level
diff --git a/plugins/woocommerce/tests/php/src/Internal/RestApi/Routes/V4/Fulfillments/ProvidersTest.php b/plugins/woocommerce/tests/php/src/Internal/RestApi/Routes/V4/Fulfillments/ProvidersTest.php
index 33198ca57e6..29bc0d9c3e0 100644
--- a/plugins/woocommerce/tests/php/src/Internal/RestApi/Routes/V4/Fulfillments/ProvidersTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/RestApi/Routes/V4/Fulfillments/ProvidersTest.php
@@ -104,6 +104,11 @@ class ProvidersTest extends WC_REST_Unit_Test_Case {
public function setUp(): void {
parent::setUp();
+ // Hooks added in setUpBeforeClass() are removed after the first test, so add them per test.
+ $fulfillments = wc_get_container()->get( \Automattic\WooCommerce\Admin\Features\Fulfillments\FulfillmentsController::class );
+ $fulfillments->register();
+ $fulfillments->initialize_fulfillments();
+
$this->controller = new FulfillmentsController();
$this->controller->init( new FulfillmentSchema(), new OrderFulfillmentsRestController() );
$this->controller->register_routes();
@@ -237,8 +242,8 @@ class ProvidersTest extends WC_REST_Unit_Test_Case {
$this->assertEquals( 200, $response->get_status() );
$this->assertIsArray( $data );
+ $this->assertNotEmpty( $data, 'The built-in shipping providers should be listed' );
- // Each provider should have the expected structure if any exist.
foreach ( $data as $key => $provider ) {
$this->assertIsString( $key );
$this->assertIsArray( $provider );
diff --git a/plugins/woocommerce/tests/php/src/LoggerSpyTraitTest.php b/plugins/woocommerce/tests/php/src/LoggerSpyTraitTest.php
new file mode 100644
index 00000000000..0d263cc1bac
--- /dev/null
+++ b/plugins/woocommerce/tests/php/src/LoggerSpyTraitTest.php
@@ -0,0 +1,66 @@
+<?php
+declare( strict_types = 1 );
+
+namespace Automattic\WooCommerce\Tests;
+
+use Automattic\WooCommerce\RestApi\UnitTests\LoggerSpyTrait;
+
+/**
+ * Sits between the test class and WC_Unit_Test_Case, so its set_up() runs when the trait calls
+ * parent::set_up(): the last point before WordPress may snapshot the hooks.
+ *
+ * phpcs:disable Squiz.Classes.ClassFileName.NoMatch
+ * phpcs:disable SlevomatCodingStandard.Files.TypeNameMatchesFileName.NoMatchBetweenTypeNameAndFileName
+ * phpcs:disable Generic.Files.OneObjectStructurePerFile.MultipleFound
+ */
+abstract class LoggerSpyTraitTestBase extends \WC_Unit_Test_Case {
+
+ /**
+ * Whether woocommerce_logging_class had any callback when the base set-up started.
+ *
+ * @var bool
+ */
+ protected $logging_class_filtered_before_base_set_up;
+
+ /**
+ * Record whether the logging class is filtered, then run the base set-up.
+ */
+ public function set_up() {
+ $this->logging_class_filtered_before_base_set_up = has_filter( 'woocommerce_logging_class' );
+ parent::set_up();
+ }
+}
+
+/**
+ * Tests when LoggerSpyTrait puts its spy in place.
+ *
+ * WordPress snapshots the hooks in the first set-up of the process and restores that snapshot
+ * after every test, so a filter added before the base set-up would come back in every later
+ * test. Checking the timing here holds in any test order.
+ */
+class LoggerSpyTraitTest extends LoggerSpyTraitTestBase {
+ use LoggerSpyTrait;
+
+ /**
+ * Whether the spy was in place right after the base set-up, for the rest of setUp().
+ *
+ * @var bool
+ */
+ private $spy_in_place_after_base_set_up;
+
+ /**
+ * Record whether the spy is in place once the base set-up has run.
+ */
+ public function setUp(): void {
+ parent::setUp();
+ $this->spy_in_place_after_base_set_up = wc_get_logger() === $this->get_spy_logger();
+ }
+
+ /**
+ * @testdox The spy logger is added after the base set-up, and is in place for the rest of setUp().
+ */
+ public function test_spy_is_added_after_the_base_set_up(): void {
+ $this->assertFalse( $this->logging_class_filtered_before_base_set_up, 'A filter added before the base set-up joins the hook snapshot and leaks into later tests' );
+ $this->assertTrue( $this->spy_in_place_after_base_set_up, 'Classes that build their subject in setUp() need the spy by then' );
+ }
+}