Commit 9102b9e9b19 for woocommerce
commit 9102b9e9b19a124098f13af52591e932f161445a
Author: Vlad Olaru <vlad.olaru@automattic.com>
Date: Thu Oct 15 09:36:06 2026 +0300
[tests] Drop leftover product attribute taxonomies between tests (#69618)
* test: Clear the cached attribute list in the product attributes fixture
WC_Admin_Tests_API_Product_Attributes commits three attributes in
setUpBeforeClass() and deletes their rows with raw SQL in
tearDownAfterClass(), but left the wc_attribute_taxonomies transient
that still lists them. Every later test then got colour, number and
size from wc_get_attribute_taxonomies() until another test cleared
transients, so code that counts terms per listed attribute, such as
ProductFilterAttribute, worked with attributes that no longer exist.
Delete the transient and invalidate the woocommerce-attributes cache
group along with the rows.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test: Drop leftover attribute taxonomies in WC_Unit_Test_Case setup
Creating a product attribute registers its pa_* taxonomy and adds it to
$wc_product_attributes. The rollback deletes the attribute row, but
WC_Post_Types::register_taxonomies() only runs once per process, so
the taxonomy and the global entry stay for every later test: 18 tests
in 15 classes leave one behind, and about 15k tests start with at least
one. A test that needs pa_size could pass only because an earlier test
left it behind, as the OrderItemSchemaTest "any" variation tests did
until #69117.
At the start of each test, read the attribute names from the table and
unregister every pa_* taxonomy, and drop every $wc_product_attributes
entry, that has no row. Reading the table directly, rather than calling
wc_get_attribute_taxonomies(), keeps the attribute cache untouched for
tests that count its filter.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test: Tighten the attribute taxonomy cleanup test and docs
Review simplification. The new UnitTestCaseTearDownTest test cleaned
up after itself, but the code under test already does that in the
next test's setUp(): its attribute row is rolled back and both
taxonomies and global entries go. Restoring the old global even put
stale entries back. Drop the cleanup, shorten the @testdox and the
method docblock, and update the class docblock, which only described
the teardown half.
Keep array_keys( get_taxonomies() ) and say why: a renamed attribute's
taxonomy stays under its new registry key while its object keeps the
old name (pa_testupdate holds 'pa_test' after
WC_Tests_Attributes_Functions), so iterating the names misses it and
WC_Attribute_Functions_Test errors after that class.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test: Unregister the attribute taxonomies the cleanup test registers
The setup cleanup test registers pa_kept_with_a_row through the product
helper and pa_leaked_without_a_row by hand. The rollback deletes the
kept attribute's row but leaves its taxonomy and $wc_product_attributes
entry behind, so the test leaked the very state the base setUp() now
cleans. The next WC_Unit_Test_Case setup would clear it, but a test on a
plain WP_UnitTestCase base running in between would still see it.
Unregister both taxonomies and drop their global entries in a finally
block, so the cleanup also runs when the helper or an assertion throws.
A follower test on WP_UnitTestCase saw the leaked taxonomy before this
change and passes after it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test: Cover the attribute taxonomy cleanup through WC_Unit_Test_Case setup
The cleanup test calls unregister_stale_attribute_taxonomies() through
reflection, so it keeps passing if setUp() stops calling the method.
Add UnitTestCaseSetUpTest, which registers a rowless pa_* taxonomy and
its $wc_product_attributes entry before parent::setUp(), the way an
earlier test leaves them, and asserts setup removed both. Its tearDown()
drops the staged state in case setup did not. With the setUp() call
deleted, this test fails while the reflection test still passes.
The kept-row case stays in the reflection test: an attribute row written
before parent::setUp() would land outside the per-test transaction and
persist.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test: Fold the setup lifecycle test into UnitTestCaseTearDownTest
UnitTestCaseSetUpTest held one two-assertion test in its own file, with
its own setUp() and tearDown(), while UnitTestCaseTearDownTest already
covers the WC_Unit_Test_Case state cleanup, including the attribute
taxonomy cleanup that runs during setup.
Move the staging into UnitTestCaseTearDownTest: its setUp() registers a
rowless pa_* taxonomy before parent::setUp() for every test in the
class, and the base setup removes it before the other tests run, so they
never see it. The new test asserts it is gone, and tearDown() drops it in
case setup did not. With the setUp() call deleted, only that test fails,
and a follower test on WP_UnitTestCase sees nothing leaked.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs: State the attribute taxonomy cleanup rule exactly in the unit-test skill
The fixture lifecycle table said WC_Unit_Test_Case unregisters pa_*
taxonomies "whose attribute row was rolled back". The setup cleanup
does not track rollbacks: it unregisters every pa_* taxonomy that has no
attribute row, whatever the reason. A reader trusting the old wording
would expect a rowless pa_* taxonomy registered in wpSetUpBeforeClass()
to survive, and would get a missing taxonomy instead.
Say what the cleanup does, so the consequence (register a rowless pa_*
taxonomy after parent::setUp()) follows from the table.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
diff --git a/.ai/skills/woocommerce-backend-dev/unit-tests.md b/.ai/skills/woocommerce-backend-dev/unit-tests.md
index 4e4c0ef1c86..f0940cc77ed 100644
--- a/.ai/skills/woocommerce-backend-dev/unit-tests.md
+++ b/.ai/skills/woocommerce-backend-dev/unit-tests.md
@@ -102,7 +102,7 @@ Choose cleanup from the test's base class; PHPUnit alone provides no database is
| Base class | Automatic isolation when parent setup/teardown runs |
| --- | --- |
| `WP_UnitTestCase` descendants, including `WP_HTTP_TestCase` and `WP_Test_REST_TestCase` | `$wpdb` transaction rollback, reset of the WordPress globals managed by the base, and hook snapshot restoration; the next setup flushes the object cache |
-| `WC_Unit_Test_Case` descendants, including `WC_REST_Unit_Test_Case` | All `WP_UnitTestCase` behavior plus WooCommerce cart/context, notices, and country-locale singleton cleanup |
+| `WC_Unit_Test_Case` descendants, including `WC_REST_Unit_Test_Case` | All `WP_UnitTestCase` behavior plus WooCommerce cart/context, notices, and country-locale singleton cleanup, and, at each setup, unregistration of every `pa_*` taxonomy without an attribute row |
| `PHPUnit\Framework\TestCase` | No WordPress transaction, hook restoration, or global cleanup |
| Other custom base | Inspect its implementation; do not infer cleanup from PHPUnit or its name |
diff --git a/plugins/woocommerce/changelog/test-base-case-drops-stale-attribute-taxonomies b/plugins/woocommerce/changelog/test-base-case-drops-stale-attribute-taxonomies
new file mode 100644
index 00000000000..2416d3fb506
--- /dev/null
+++ b/plugins/woocommerce/changelog/test-base-case-drops-stale-attribute-taxonomies
@@ -0,0 +1,3 @@
+Significance: patch
+Type: dev
+Comment: Unregister leftover product attribute taxonomies in the base PHPUnit test case setup; no production change.
diff --git a/plugins/woocommerce/tests/legacy/framework/class-wc-unit-test-case.php b/plugins/woocommerce/tests/legacy/framework/class-wc-unit-test-case.php
index edb13bcbd9f..31d4a91e22e 100644
--- a/plugins/woocommerce/tests/legacy/framework/class-wc-unit-test-case.php
+++ b/plugins/woocommerce/tests/legacy/framework/class-wc-unit-test-case.php
@@ -139,6 +139,7 @@ class WC_Unit_Test_Case extends WP_HTTP_TestCase {
// Register post types before each test.
WC_Post_types::register_post_types();
WC_Post_types::register_taxonomies();
+ $this->unregister_stale_attribute_taxonomies();
CodeHacker::reset_hacks();
@@ -147,6 +148,31 @@ class WC_Unit_Test_Case extends WP_HTTP_TestCase {
wc_get_container()->get( LegacyProxy::class )->reset();
}
+ /**
+ * Unregister pa_* taxonomies, and drop their $wc_product_attributes entries, once the attribute row is gone.
+ *
+ * The per-test rollback deletes the attribute rows a test created but leaves the taxonomies registered,
+ * and WC_Post_Types::register_taxonomies() does not rebuild them after bootstrap.
+ */
+ private function unregister_stale_attribute_taxonomies(): void {
+ global $wpdb, $wc_product_attributes;
+
+ // Read the table directly: wc_get_attribute_taxonomies() would fill the attribute cache before the test runs.
+ $attribute_taxonomies = array_map(
+ 'wc_attribute_taxonomy_name',
+ $wpdb->get_col( "SELECT attribute_name FROM {$wpdb->prefix}woocommerce_attribute_taxonomies" )
+ );
+
+ // Use the registry keys: a renamed attribute's taxonomy object can keep its old name.
+ foreach ( array_keys( get_taxonomies() ) as $taxonomy ) {
+ if ( 0 === strpos( $taxonomy, 'pa_' ) && ! in_array( $taxonomy, $attribute_taxonomies, true ) ) {
+ unregister_taxonomy( $taxonomy );
+ }
+ }
+
+ $wc_product_attributes = array_intersect_key( (array) $wc_product_attributes, array_flip( $attribute_taxonomies ) );
+ }
+
/**
* Tear down test case.
*
diff --git a/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/api/product-attributes.php b/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/api/product-attributes.php
index 7ba9f07feb4..0064124c1f1 100644
--- a/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/api/product-attributes.php
+++ b/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/api/product-attributes.php
@@ -43,6 +43,9 @@ class WC_Admin_Tests_API_Product_Attributes extends WC_REST_Unit_Test_Case {
parent::tearDownAfterClass();
global $wpdb;
$wpdb->query( "DELETE FROM {$wpdb->prefix}woocommerce_attribute_taxonomies" );
+ // wc_get_attribute_taxonomies() caches the list, so drop the copy that still names the deleted rows.
+ delete_transient( 'wc_attribute_taxonomies' );
+ WC_Cache_Helper::invalidate_cache_group( 'woocommerce-attributes' );
$wpdb->query( 'commit' );
} finally {
self::disable_direct_product_attribute_lookup_updates();
diff --git a/plugins/woocommerce/tests/php/src/UnitTestCaseTearDownTest.php b/plugins/woocommerce/tests/php/src/UnitTestCaseTearDownTest.php
index 7675e069a98..48bce12ed80 100644
--- a/plugins/woocommerce/tests/php/src/UnitTestCaseTearDownTest.php
+++ b/plugins/woocommerce/tests/php/src/UnitTestCaseTearDownTest.php
@@ -4,13 +4,15 @@ declare( strict_types = 1 );
namespace Automattic\WooCommerce\Tests;
/**
- * Tests that WC_Unit_Test_Case clears the WC() singleton state that neither the per-test
- * database rollback nor the hook restore covers.
+ * Tests the state cleanup in WC_Unit_Test_Case: the WC() singleton state that survives the
+ * parent teardown, and the attribute taxonomies the rollback leaves registered.
*
- * This drives clear_wc_singleton_state() directly rather than dirtying state in one test and
+ * This drives the cleanup methods directly rather than dirtying state in one test and
* asserting it is gone in the next. A pair like that only holds under declaration order: run
* the suite with --order-by=random or reverse and the assertion half runs first, passes
* against state nothing has dirtied yet, and stops covering anything without ever going red.
+ * The one exception stages a leftover taxonomy before parent::setUp() in the same test, so it
+ * covers the setup call itself.
*/
class UnitTestCaseTearDownTest extends \WC_Unit_Test_Case {
@@ -19,6 +21,38 @@ class UnitTestCaseTearDownTest extends \WC_Unit_Test_Case {
*/
private const LEAKED_LOCALE_LABEL = 'Leaked postcode label';
+ /**
+ * A pa_* taxonomy with no attribute row, staged before every setup in this class.
+ */
+ private const STALE_TAXONOMY = 'pa_stale_before_setup';
+
+ /**
+ * Stage the leftover an earlier test would leave, then run the base setup.
+ *
+ * Only a registration is staged: an attribute row written here would land outside the
+ * per-test transaction and persist.
+ */
+ public function setUp(): void {
+ global $wc_product_attributes;
+
+ register_taxonomy( self::STALE_TAXONOMY, 'product' );
+ $wc_product_attributes[ self::STALE_TAXONOMY ] = (object) array( 'attribute_name' => 'stale_before_setup' );
+
+ parent::setUp();
+ }
+
+ /**
+ * Drop the staged leftover in case setup did not.
+ */
+ public function tearDown(): void {
+ global $wc_product_attributes;
+
+ unregister_taxonomy( self::STALE_TAXONOMY );
+ unset( $wc_product_attributes[ self::STALE_TAXONOMY ] );
+
+ parent::tearDown();
+ }
+
/**
* Every piece of singleton state the teardown is responsible for is cleared.
*/
@@ -70,4 +104,45 @@ class UnitTestCaseTearDownTest extends \WC_Unit_Test_Case {
'The filtered locale should not still be cached.'
);
}
+
+ /**
+ * @testdox Setup unregisters a pa_* taxonomy an earlier test left without an attribute row.
+ */
+ public function test_setup_drops_a_leftover_attribute_taxonomy(): void {
+ global $wc_product_attributes;
+
+ $this->assertFalse( taxonomy_exists( self::STALE_TAXONOMY ), 'Setup should unregister a pa_* taxonomy with no attribute row.' );
+ $this->assertArrayNotHasKey( self::STALE_TAXONOMY, (array) $wc_product_attributes, 'Setup should drop its $wc_product_attributes entry.' );
+ }
+
+ /**
+ * @testdox Setup unregisters pa_* taxonomies that have no attribute row and keeps the ones that do.
+ */
+ public function test_setup_drops_attribute_taxonomies_without_an_attribute_row(): void {
+ global $wc_product_attributes;
+
+ $kept = 'pa_kept_with_a_row';
+ $leaked = 'pa_leaked_without_a_row';
+
+ try {
+ \WC_Helper_Product::create_attribute( 'kept_with_a_row', array() );
+ register_taxonomy( $leaked, 'product' );
+ $wc_product_attributes[ $leaked ] = (object) array( 'attribute_name' => 'leaked_without_a_row' );
+
+ $method = new \ReflectionMethod( \WC_Unit_Test_Case::class, 'unregister_stale_attribute_taxonomies' );
+ $method->setAccessible( true );
+ $method->invoke( $this );
+
+ $this->assertFalse( taxonomy_exists( $leaked ), 'A pa_* taxonomy with no attribute row should be unregistered.' );
+ $this->assertArrayNotHasKey( $leaked, $wc_product_attributes, 'An attribute with no row should leave $wc_product_attributes.' );
+ $this->assertTrue( taxonomy_exists( $kept ), 'A pa_* taxonomy with an attribute row should stay registered.' );
+ $this->assertArrayHasKey( $kept, $wc_product_attributes, 'An attribute with a row should stay in $wc_product_attributes.' );
+ } finally {
+ // The rollback drops the kept attribute's row but not its taxonomy, which a later test on another base class would still see.
+ foreach ( array( $kept, $leaked ) as $taxonomy ) {
+ unregister_taxonomy( $taxonomy );
+ unset( $wc_product_attributes[ $taxonomy ] );
+ }
+ }
+ }
}