Commit a173eb853b1 for woocommerce
commit a173eb853b16136e0bdd190b9c540b1b83b42f27
Author: Vlad Olaru <vlad.olaru@automattic.com>
Date: Wed Aug 26 15:56:45 2026 +0300
[docs] Clarify PHPUnit fixture cleanup guidance (#68033)
* docs: Clarify PHPUnit fixture cleanup lifecycle
[Context]
WooCommerce tests use several PHPUnit base classes with different
isolation guarantees.
[Problem]
The agent guidance implied that all tests shared transaction rollback
while nearby examples encouraged redundant teardown for database
fixtures and filters.
[Solution]
Document cleanup by base-class lifecycle, bound WordPress transaction
coverage, remove conflicting examples, and add a review guard against
duplicating automatic cleanup.
* docs: Scope the rollback guarantee to the transaction boundary
The fixture cleanup guidance told readers that a WP_UnitTestCase
descendant runs each test in a $wpdb transaction whenever the parent
lifecycle runs, then enumerated what rollback does not cover. That
enumeration left out ordering within setup.
WP_UnitTestCase_Base::set_up() calls start_transaction() as its last
statement, and start_transaction() only issues SET autocommit = 0 and
START TRANSACTION. A child set_up() that writes before calling
parent::set_up() therefore lands outside the transaction: on the first
test in the process autocommit is still on, and on every later test the
START TRANSACTION implicitly commits the pending one. Either way the
rows outlive the test. Verified on wp-env with a throwaway probe: a user
created before parent::set_up() survived the rollback, one created after
it did not.
Add that case to both carve-out lists and state the requirement
directly, so an overriding set_up() calls parent::set_up() before any
write the rollback must cover.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs: Name WC_REST_Unit_Test_Case in the base class table
The base class table listed WC_Unit_Test_Case alone, so a reader
looking up WC_REST_Unit_Test_Case had to first discover that it
extends WC_Unit_Test_Case before the row applied to them. The
WP_UnitTestCase row above it already names its descendants inline.
Name the descendant in the same style, so the row can be matched
against the class a test actually extends without a lookup.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs: Cover both lifecycle method spellings in the parent setup rule
The rule about calling the parent setup before rollback-dependent
writes named only parent::set_up(). WooCommerce tests overwhelmingly
override the camelCase form instead: 654 setUp() declarations against
11 set_up(), and 430 tearDown() against 8 tear_down(), because both
WC_Unit_Test_Case and WP_HTTP_TestCase define the camelCase methods.
The rule as written therefore addressed the rare case and missed the
common one, even though the hazard is identical. Verified on wp-env
that a camelCase override leaks the same way: a user created before
parent::setUp() survived the rollback, one created after it did not.
State the rule in terms of the parent setup and tell the reader to
match the call to the method they override, so it applies whichever
spelling the class uses.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs: Show a working teardown in the test file template
Dropping the no-op tearDown() from the template was right on its own
terms: PHP calls the parent anyway when nothing overrides it, and
Generic.CodeAnalysis.UselessOverridingMethod flags the empty override,
which the Lint job fails on because it treats warnings as errors.
Verified by running the ruleset over the removed method.
But removing it left the template with no teardown at all, so it
stopped demonstrating the case that actually needs care, and nothing
discouraged a reader from adding the empty override back.
Show a teardown that does real work instead: reset state no base class
owns, wrapped so the parent still runs if that cleanup throws, which is
the idiom WC_Unit_Test_Case::tearDown() already uses. Name the sniff in
the cleanup guidance, and record the rule in the key elements table.
The template was materialised as a real test file and passes phpcs with
no errors and no warnings.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs: Move scoped PHP test instructions to AGENTS.md
The repository already keeps agent instructions in AGENTS.md and leaves
CLAUDE.md as a one-line "@AGENTS.md" import, at the root and in
packages/action-scheduler. The scoped PHP test instructions were the
odd one out, reachable only by tools that look for CLAUDE.md.
Rename the file and add the import shim so the content is available
under the neutral name every agent reads, with no change for readers
who follow CLAUDE.md.
Retitle it accordingly, since the file is no longer Claude-specific,
and repoint its Parent line at the root AGENTS.md: it named
plugins/woocommerce/CLAUDE.md, which does not exist.
The shim is a single import rather than prose, so MD041 fires on it;
markdownlint only sees files a change touches, which is why the two
existing shims never tripped it. Add the new one to the ignore list.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs: Exempt the existing agent instruction shims from markdownlint
The root CLAUDE.md and packages/action-scheduler/CLAUDE.md are the same
single-line "@AGENTS.md" import as the shim just added, and MD041 fires
on all three. They pass today only because CI lints the files a change
touches and nothing has touched them; the next edit to either would
turn the markdown job red for a file that is correct as written.
Add both alongside the new shim. The root entry is anchored as
/CLAUDE.md so it matches only the root file: an unanchored CLAUDE.md
would match at every depth and silently stop linting the CLAUDE.md
files that do carry prose, such as packages/js and client/admin.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
diff --git a/.ai/skills/woocommerce-backend-dev/unit-tests.md b/.ai/skills/woocommerce-backend-dev/unit-tests.md
index fb025e21fa7..4e4c0ef1c86 100644
--- a/.ai/skills/woocommerce-backend-dev/unit-tests.md
+++ b/.ai/skills/woocommerce-backend-dev/unit-tests.md
@@ -3,6 +3,7 @@
## Table of Contents
- [Complete Test File Template](#complete-test-file-template)
+- [Fixture Lifecycle and Cleanup](#fixture-lifecycle-and-cleanup)
- [Test File Naming and Location](#test-file-naming-and-location)
- [System Under Test Variable](#system-under-test-variable)
- [Test Method Documentation](#test-method-documentation)
@@ -51,7 +52,12 @@ class OrderProcessorTest extends WC_Unit_Test_Case {
* Tear down test fixtures.
*/
public function tearDown(): void {
- parent::tearDown();
+ try {
+ // A static cache on the SUT, which no base class resets.
+ OrderProcessor::reset_cache();
+ } finally {
+ parent::tearDown();
+ }
}
/**
@@ -87,6 +93,28 @@ class OrderProcessorTest extends WC_Unit_Test_Case {
| Test docblock | Use `@testdox` with sentence ending in `.` |
| Return type | Use `void` for test methods |
| Assertion messages | Include helpful context for failures |
+| Teardown | Only for state the base does not own; guarantee `parent::tearDown()` runs |
+
+## Fixture Lifecycle and Cleanup
+
+Choose cleanup from the test's base class; PHPUnit alone provides no database isolation.
+
+| 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 |
+| `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 |
+
+For a `WP_UnitTestCase` descendant, do not manually delete per-test products, coupons, orders, users, options, metadata, or other rows as post-assertion cleanup when its transaction covers them. Rollback does not cover writes made before the parent setup starts the transaction, class fixtures, explicit commits or DDL, non-transactional tables, other database connections, files, external services, or process state that the selected base does not reset.
+
+- Call the parent setup before any write the rollback must cover, matching the method you override: `parent::setUp()` from a camelCase override, `parent::set_up()` from a snake_case one. Both reach `WP_UnitTestCase_Base::set_up()`, which starts the transaction as its last step, so writes made earlier in the override are committed and outlive the test.
+- Create persistent class fixtures in `wpSetUpBeforeClass()` and delete them in `wpTearDownAfterClass()`.
+- Remove a hook before the test ends only when later work in that test must not run it; WordPress parent teardown restores the hook snapshot.
+- An arrangement-time reset is valid when `setUp()` or the base class preloads state. Do not repeat cleanup already performed by the base.
+- If custom cleanup is required, guarantee `parent::tearDown()` runs. In a `WP_UnitTestCase` descendant, perform cleanup that can write through `$wpdb` before the parent rollback. Do not add an override that only calls the parent: `Generic.CodeAnalysis.UselessOverridingMethod` flags it, and the Lint job treats that warning as a failure.
+
+See [Performance and isolation principles](../../../plugins/woocommerce/tests/README.md#performance-and-isolation-principles) for fixture sizing and database constraints.
## Test File Naming and Location
@@ -373,32 +401,30 @@ The fake logger must implement `WC_Logger_Interface`. Create an anonymous class
```php
public function test_logs_warning_for_invalid_input(): void {
- $fake_logger = $this->create_fake_logger();
-
- // Inject via filter - passing object bypasses cache.
- add_filter(
- 'woocommerce_logging_class',
- function () use ( $fake_logger ) {
- return $fake_logger;
- }
- );
+ $fake_logger = $this->create_fake_logger();
- $this->sut->process_input( 'invalid-value' );
+ // Inject via filter - passing object bypasses cache.
+ add_filter(
+ 'woocommerce_logging_class',
+ static function () use ( $fake_logger ) {
+ return $fake_logger;
+ }
+ );
- $this->assertCount( 1, $fake_logger->warning_calls );
+ $this->sut->process_input( 'invalid-value' );
- remove_all_filters( 'woocommerce_logging_class' ); // Always clean up.
+ $this->assertCount( 1, $fake_logger->warning_calls );
}
```
### Key Points
-| Aspect | Detail |
-| ------------ | ----------------------------------------------- |
-| Filter name | `woocommerce_logging_class` |
-| Return value | Object instance (not class name string) |
-| Interface | Must implement `WC_Logger_Interface` |
-| Cleanup | Always call `remove_all_filters()` after test |
+| Aspect | Detail |
+| ------------ | ------------------------------------------------------------- |
+| Filter name | `woocommerce_logging_class` |
+| Return value | Object instance (not class name string) |
+| Interface | Must implement `WC_Logger_Interface` |
+| Isolation | `WP_UnitTestCase` parent teardown restores the filter snapshot |
### Reference
diff --git a/.ai/skills/woocommerce-code-review/SKILL.md b/.ai/skills/woocommerce-code-review/SKILL.md
index 68fbdbec3d4..91bac49d093 100644
--- a/.ai/skills/woocommerce-code-review/SKILL.md
+++ b/.ai/skills/woocommerce-code-review/SKILL.md
@@ -40,6 +40,7 @@ Consult the `woocommerce-backend-dev` skill for detailed standards. Using these
- ❌ **Using `$instance` in tests** - Must use `$sut` variable name ([unit-tests.md](../woocommerce-backend-dev/unit-tests.md))
- ❌ **Missing `@testdox`** - Required in test method docblocks ([unit-tests.md](../woocommerce-backend-dev/unit-tests.md))
- ❌ **Test file naming** - Must follow convention for `includes/` vs `src/` ([unit-tests.md](../woocommerce-backend-dev/unit-tests.md))
+- ❌ **Cleanup that duplicates the base lifecycle** - Identify the test's base class before requesting post-assertion fixture deletion or state restoration; flag cleanup already covered by its transaction or teardown ([unit-tests.md](../woocommerce-backend-dev/unit-tests.md#fixture-lifecycle-and-cleanup))
### Frontend JS/TS Code
diff --git a/.markdownlintignore b/.markdownlintignore
index 7e1afe04e8c..2232847196b 100644
--- a/.markdownlintignore
+++ b/.markdownlintignore
@@ -4,3 +4,7 @@ plugins/woocommerce/lib/packages/*
packages/php/email-editor/vendor-prefixed/*
plugins/woocommerce/src/Internal/StockNotifications/CODERABBIT-TRIAGE-RAW.md
packages/php/woocommerce-analytics/CHANGELOG.md
+# Agent instruction shims: a single "@AGENTS.md" import, not prose (MD041).
+/CLAUDE.md
+plugins/woocommerce/packages/action-scheduler/CLAUDE.md
+plugins/woocommerce/tests/php/src/CLAUDE.md
diff --git a/plugins/woocommerce/tests/README.md b/plugins/woocommerce/tests/README.md
index 087cc1515aa..c37b5d6433a 100644
--- a/plugins/woocommerce/tests/README.md
+++ b/plugins/woocommerce/tests/README.md
@@ -175,7 +175,7 @@ General guidelines for all the unit tests:
- In addition to covering each line of a method/function, make sure to test common input and edge cases.
- Prefer `assertSame()` where possible as it tests both type and value
- Remember that only methods prefixed with `test` will be run so use helper methods liberally to keep test methods small and reduce code duplication. If there is a common helper method used in multiple test files, consider adding it to the `WC_Unit_Test_Case` class so it can be shared by all test cases
-- Filters persist between test cases so be sure to remove them in your test method or in the `tearDown()` method.
+- For `WP_UnitTestCase` descendants, parent teardown restores the hook snapshot. Remove a filter earlier only when later work in the same test must not run it; other base classes must manage their own hooks.
- Use data providers where possible. Be sure that their name is like `data_provider_function_to_test` (i.e. the data provider for `test_is_postcode` would be `data_provider_test_is_postcode`). Read more about data providers in the [PHPUnit manual](https://phpunit.de/manual/current/en/writing-tests-for-phpunit.html#writing-tests-for-phpunit.data-providers).
### Performance and isolation principles
@@ -189,8 +189,9 @@ The full suite runs in a few minutes because every test pays only for what it as
3. Direct row inserts when the code under test only reads persisted rows (see "Seed at the boundary the code reads" below).
The same ladder applies to products and other entities: full helper → minimal production API call → boundary seeding.
-- **Respect the per-test transaction.** Every test runs inside a transaction that is rolled back, so most cleanup is free. Never use `TRUNCATE TABLE` in tests or helpers: it is DDL and implicitly commits the transaction, silently breaking isolation for everything after it. Use `DELETE FROM` instead.
-- **Share immutable fixtures at class level.** Catalogs that no test mutates belong in `wpSetUpBeforeClass()`, reloaded per test with fresh object instances (`wc_get_product()` etc.). Per-test mutations are contained by the transaction rollback and the per-test object-cache flush. Clean up class-created data in `wpTearDownAfterClass()`.
+- **Respect the selected base class.** Tests that extend `WP_UnitTestCase` directly or indirectly and call the parent lifecycle run each test in a `$wpdb` transaction. Rollback covers writes through that connection to transactional tables; it does not cover writes made before the parent setup starts the transaction, class fixtures, explicit commits or DDL, non-transactional tables, other connections, files, external services, or process state. An override must call the parent setup — `parent::setUp()` or `parent::set_up()`, matching the method it overrides — before any write the rollback must cover. Plain PHPUnit does not provide this rollback; inspect other custom bases for their own lifecycle. Never use `TRUNCATE TABLE`: its implicit commit breaks isolation; use `DELETE FROM` when deletion is required.
+- **Share immutable fixtures at class level.** For these WordPress base classes, catalogs that no test mutates belong in `wpSetUpBeforeClass()`, reloaded per test with fresh object instances (`wc_get_product()` etc.). Per-test mutations are contained by the transaction rollback and the per-test object-cache flush. Clean up class-created data in `wpTearDownAfterClass()`.
+- **Clean only state the base class does not own.** WordPress parent teardown restores its hook snapshot and the globals it manages; `WC_Unit_Test_Case` also resets its documented WooCommerce singleton state. Inspect other bases before relying on or duplicating cleanup. Resetting preloaded state during test arrangement is not teardown cleanup.
- **Seed at the boundary the code reads.** If the code under test only reads persisted rows (aggregate SQL, lookup tables, migration sources), fixtures may insert those rows directly — with exactly the columns production writes and collision-safe IDs. Keep at least one real write-path (CRUD) test per area so the sync path stays covered.
- **Register only the REST surface you exercise.** `WC_REST_Unit_Test_Case` registers routes lazily per namespace; scoped tests can use `WC_Unit_Test_Case::create_rest_server_with_routes()`. Avoid re-firing the full `rest_api_init` per test.
- **Never sleep, never fetch.** Control timestamps through the object setters instead of `sleep()`, and mock HTTP through the framework's interception layer (`$this->http_responder`, local fixture files) so the suite is deterministic and passes without network access.
diff --git a/plugins/woocommerce/tests/php/src/AGENTS.md b/plugins/woocommerce/tests/php/src/AGENTS.md
new file mode 100644
index 00000000000..de7b2fec51b
--- /dev/null
+++ b/plugins/woocommerce/tests/php/src/AGENTS.md
@@ -0,0 +1,274 @@
+# PHP Testing - AI Agents Documentation
+
+**Scope**: PHPUnit test patterns for WooCommerce plugin tests
+**Parent**: `AGENTS.md` (repository root)
+
+## Quick Reference: Resilient Test Patterns
+
+### Rule: Use targeted assertions, not full equality checks
+
+| Pattern | Brittle (Wrong) | Resilient (Correct) |
+| ------- | --------------- | ------------------- |
+| Array key + value | `assertSame(['key' => null], $arr)` | `assertArrayHasKey('key', $arr)` + `assertNull($arr['key'])` |
+| Single value | `assertSame(['a' => 1, 'b' => 2], $r)` | `assertArrayHasKey('a', $r)` + `assertSame(1, $r['a'])` |
+| Nested | `assertSame(['m' => ['e' => null]], $r)` | `assertArrayHasKey('e', $r['m'])` + `assertNull($r['m']['e'])` |
+
+Why: Full equality breaks when new keys are added.
+
+**Example**: `WooPaymentsServiceTest.php:510-511`
+
+```php
+// WRONG - Breaks if new keys added to messages array
+$this->assertSame( array( 'not_supported' => null ), $result['messages'] );
+
+// CORRECT - Tests only what matters
+$this->assertArrayHasKey( 'not_supported', $result['messages'] );
+$this->assertNull( $result['messages']['not_supported'] );
+```
+
+## PHPUnit Assertions
+
+**Priority order**: Most specific → Structure → General
+
+| Assertion | Use | Example |
+| --------- | --- | ------- |
+| `assertSame()` | Strict === | `assertSame(5, $count)` |
+| `assertEquals()` | Loose == | `assertEquals('5', $count)` |
+| `assertNull()` | Null check | `assertNull($error)` |
+| `assertTrue()` | Boolean | `assertTrue($flag)` |
+| `assertArrayHasKey()` | Key exists | `assertArrayHasKey('id', $data)` |
+| `assertIsArray()` | Type check | `assertIsArray($result)` |
+| `assertCount()` | Array size | `assertCount(3, $items)` |
+
+## Mock External Classes
+
+**Pattern for external classes (WooPayments, etc.):**
+
+```php
+if ( ! class_exists( 'WC_Payments_Utils' ) ) {
+ /**
+ * Mock for testing.
+ *
+ * phpcs:disable Squiz.Classes.ClassFileName.NoMatch
+ * phpcs:disable SlevomatCodingStandard.Files.TypeNameMatchesFileName.NoMatchBetweenTypeNameAndFileName
+ * phpcs:disable Squiz.Classes.ValidClassName.NotCamelCaps
+ */
+ class WC_Payments_Utils {
+ public static function supported_countries(): array {
+ return array( 'US', 'GB' );
+ }
+ }
+ // phpcs:enable
+}
+```
+
+**Why ignores needed:**
+
+- `ClassFileName.NoMatch` - Mock doesn't match file name
+- `TypeNameMatchesFileName.NoMatchBetweenTypeNameAndFileName` - External class not PSR-4
+- `ValidClassName.NotCamelCaps` - Uses underscores
+
+## Unused Closure Parameters
+
+**PHPCS requires**: Use `unset()` for required but unused parameters
+
+```php
+// WRONG - PHPCS error
+'callback' => function ( string $url ) {
+ return array( 'success' => true );
+},
+
+// CORRECT
+'callback' => function ( string $url ) {
+ unset( $url ); // Avoid parameter not used PHPCS errors.
+ return array( 'success' => true );
+},
+
+// Multiple unused
+'callback' => function ( $a, $b, $c ) {
+ unset( $a, $b ); // Avoid parameter not used PHPCS errors.
+ return process( $c );
+},
+```
+
+**Scenarios**: Mock callbacks, array_map/filter, interface implementations
+
+## Test Structure
+
+**Arrange-Act-Assert pattern:**
+
+```php
+public function test_feature_name(): void {
+ // Arrange - Set up test data and mocks.
+ $mock = $this->createMock( SomeClass::class );
+ $mock->method( 'get_data' )->willReturn( 'value' );
+
+ // Act - Execute the code being tested.
+ $result = $this->service->process( $mock );
+
+ // Assert - Verify expected behavior.
+ $this->assertIsArray( $result );
+ $this->assertArrayHasKey( 'status', $result );
+ $this->assertSame( 'success', $result['status'] );
+}
+```
+
+## Data Providers
+
+**For testing multiple scenarios:**
+
+```php
+/**
+ * @return array<string, array<mixed>>
+ */
+public function provider_scenarios(): array {
+ return array(
+ 'US merchant' => array( 'US', 'expected' ),
+ 'UK merchant' => array( 'GB', 'expected' ),
+ 'unsupported' => array( 'XX', null ),
+ );
+}
+
+/**
+ * @dataProvider provider_scenarios
+ */
+public function test_behavior( string $country, $expected ): void {
+ $result = $this->service->get_data( $country );
+ $this->assertSame( $expected, $result );
+}
+```
+
+## Setup and Teardown
+
+Cleanup depends on the selected base class. Follow the [fixture lifecycle rules](../../README.md#performance-and-isolation-principles).
+
+- When overriding setup or teardown, always call the parent implementation.
+- Do not delete per-test database fixtures merely as teardown cleanup when an inherited `WP_UnitTestCase` transaction covers their writes.
+- Explicitly restore only class-level, external, or process state that the base class does not reset.
+
+## Integration Tests
+
+**REST API endpoints:**
+
+```php
+public function test_endpoint_returns_data(): void {
+ wp_set_current_user( $this->admin_id );
+ $request = new \WP_REST_Request( 'GET', '/wc/v3/settings/payments' );
+ $response = rest_do_request( $request );
+
+ $this->assertSame( 200, $response->get_status() );
+ $this->assertIsArray( $response->get_data() );
+}
+```
+
+**WordPress hooks:**
+
+```php
+public function test_hook_fires(): void {
+ $fired = false;
+ add_filter( 'woocommerce_payment_gateways',
+ function ( $gateways ) use ( &$fired ) {
+ $fired = true;
+ return $gateways;
+ }
+ );
+
+ $result = $this->service->get_gateways();
+
+ $this->assertTrue( $fired );
+}
+```
+
+## Testing Private Methods
+
+**Use sparingly - prefer testing public interfaces:**
+
+```php
+public function test_private_method(): void {
+ $reflection = new \ReflectionClass( $this->service );
+ $method = $reflection->getMethod( 'private_method' );
+ $method->setAccessible( true );
+
+ $result = $method->invoke( $this->service, 'arg' );
+
+ $this->assertSame( 'expected', $result );
+}
+```
+
+## Running Tests
+
+```bash
+# Class
+pnpm test:php:env -- --filter WooPaymentsServiceTest
+
+# Method
+pnpm test:php:env -- --filter ClassName::test_method_name
+
+# Pattern
+pnpm test:php:env -- --filter "test_.*_not_supported"
+
+# Verbose (use --testdox for readable output)
+pnpm test:php:env -- --testdox --filter WooPaymentsServiceTest
+```
+
+## Debugging Failures
+
+**Common issues:**
+
+| Error | Cause | Fix |
+| ----- | ----- | --- |
+| `Undefined array key` | Missing key | Check code returns key |
+| `Arrays not identical` | Extra/missing keys | Use targeted assertions |
+| `Mock not called` | Code path skipped | Check test setup |
+| `Unexpected call` | Mock too strict | Use `$this->any()` |
+
+**Read diff output:**
+
+```text
+Failed asserting that two arrays are identical.
+--- Expected
++++ Actual
+@@ @@
+ Array (
+- 'key' => 'expected'
++ 'key' => 'actual'
+ )
+```
+
+## Critical Rules
+
+1. **One purpose per test** - Multiple assertions OK if testing same behavior
+2. **Test behavior, not implementation** - Avoid testing internals
+3. **Resilient assertions** - Won't break when adjacent code changes
+4. **Mock externals** - No real API calls or external plugins
+5. **Match cleanup to the base lifecycle** - Do not duplicate rollback or base teardown; restore state the base does not own
+
+## File Organization
+
+```text
+tests/php/src/
+├── Internal/
+│ └── Admin/
+│ ├── Settings/
+│ │ ├── PaymentsProviders/WooPayments/
+│ │ │ ├── WooPaymentsServiceTest.php
+│ │ │ └── WooPaymentsRestControllerIntegrationTest.php
+│ │ └── PaymentsRestControllerIntegrationTest.php
+│ └── Suggestions/
+│ └── PaymentsExtensionSuggestionsTest.php
+└── CLAUDE.md
+```
+
+**Naming:**
+
+- Unit tests: `{ClassName}Test.php`
+- Integration: `{ClassName}IntegrationTest.php`
+- Methods: `test_{feature}_{scenario}()` or
+ `test_{feature}_{scenario}_{outcome}()`
+
+## Related Docs
+
+- [`plugins/woocommerce/tests/README.md`](../../README.md) - Test environment, fixture lifecycle, and performance
+- [`woocommerce-backend-dev/unit-tests.md`](../../../../../.ai/skills/woocommerce-backend-dev/unit-tests.md) - Unit-test conventions
+- `src/Internal/Admin/Settings/CLAUDE.md` - Settings backend patterns
+- PHPUnit: <https://phpunit.de/manual/9.6/en/index.html>
diff --git a/plugins/woocommerce/tests/php/src/CLAUDE.md b/plugins/woocommerce/tests/php/src/CLAUDE.md
index 54ecaab0333..43c994c2d36 100644
--- a/plugins/woocommerce/tests/php/src/CLAUDE.md
+++ b/plugins/woocommerce/tests/php/src/CLAUDE.md
@@ -1,283 +1 @@
-# PHP Testing - Claude Code Documentation
-
-**Scope**: PHPUnit test patterns for WooCommerce plugin tests
-**Parent**: `plugins/woocommerce/CLAUDE.md`
-
-## Quick Reference: Resilient Test Patterns
-
-### Rule: Use targeted assertions, not full equality checks
-
-| Pattern | Brittle (Wrong) | Resilient (Correct) |
-| ------- | --------------- | ------------------- |
-| Array key + value | `assertSame(['key' => null], $arr)` | `assertArrayHasKey('key', $arr)` + `assertNull($arr['key'])` |
-| Single value | `assertSame(['a' => 1, 'b' => 2], $r)` | `assertArrayHasKey('a', $r)` + `assertSame(1, $r['a'])` |
-| Nested | `assertSame(['m' => ['e' => null]], $r)` | `assertArrayHasKey('e', $r['m'])` + `assertNull($r['m']['e'])` |
-
-Why: Full equality breaks when new keys are added.
-
-**Example**: `WooPaymentsServiceTest.php:510-511`
-
-```php
-// WRONG - Breaks if new keys added to messages array
-$this->assertSame( array( 'not_supported' => null ), $result['messages'] );
-
-// CORRECT - Tests only what matters
-$this->assertArrayHasKey( 'not_supported', $result['messages'] );
-$this->assertNull( $result['messages']['not_supported'] );
-```
-
-## PHPUnit Assertions
-
-**Priority order**: Most specific → Structure → General
-
-| Assertion | Use | Example |
-| --------- | --- | ------- |
-| `assertSame()` | Strict === | `assertSame(5, $count)` |
-| `assertEquals()` | Loose == | `assertEquals('5', $count)` |
-| `assertNull()` | Null check | `assertNull($error)` |
-| `assertTrue()` | Boolean | `assertTrue($flag)` |
-| `assertArrayHasKey()` | Key exists | `assertArrayHasKey('id', $data)` |
-| `assertIsArray()` | Type check | `assertIsArray($result)` |
-| `assertCount()` | Array size | `assertCount(3, $items)` |
-
-## Mock External Classes
-
-**Pattern for external classes (WooPayments, etc.):**
-
-```php
-if ( ! class_exists( 'WC_Payments_Utils' ) ) {
- /**
- * Mock for testing.
- *
- * phpcs:disable Squiz.Classes.ClassFileName.NoMatch
- * phpcs:disable SlevomatCodingStandard.Files.TypeNameMatchesFileName.NoMatchBetweenTypeNameAndFileName
- * phpcs:disable Squiz.Classes.ValidClassName.NotCamelCaps
- */
- class WC_Payments_Utils {
- public static function supported_countries(): array {
- return array( 'US', 'GB' );
- }
- }
- // phpcs:enable
-}
-```
-
-**Why ignores needed:**
-
-- `ClassFileName.NoMatch` - Mock doesn't match file name
-- `TypeNameMatchesFileName.NoMatchBetweenTypeNameAndFileName` - External class not PSR-4
-- `ValidClassName.NotCamelCaps` - Uses underscores
-
-## Unused Closure Parameters
-
-**PHPCS requires**: Use `unset()` for required but unused parameters
-
-```php
-// WRONG - PHPCS error
-'callback' => function ( string $url ) {
- return array( 'success' => true );
-},
-
-// CORRECT
-'callback' => function ( string $url ) {
- unset( $url ); // Avoid parameter not used PHPCS errors.
- return array( 'success' => true );
-},
-
-// Multiple unused
-'callback' => function ( $a, $b, $c ) {
- unset( $a, $b ); // Avoid parameter not used PHPCS errors.
- return process( $c );
-},
-```
-
-**Scenarios**: Mock callbacks, array_map/filter, interface implementations
-
-## Test Structure
-
-**Arrange-Act-Assert pattern:**
-
-```php
-public function test_feature_name(): void {
- // Arrange - Set up test data and mocks.
- $mock = $this->createMock( SomeClass::class );
- $mock->method( 'get_data' )->willReturn( 'value' );
-
- // Act - Execute the code being tested.
- $result = $this->service->process( $mock );
-
- // Assert - Verify expected behavior.
- $this->assertIsArray( $result );
- $this->assertArrayHasKey( 'status', $result );
- $this->assertSame( 'success', $result['status'] );
-}
-```
-
-## Data Providers
-
-**For testing multiple scenarios:**
-
-```php
-/**
- * @return array<string, array<mixed>>
- */
-public function provider_scenarios(): array {
- return array(
- 'US merchant' => array( 'US', 'expected' ),
- 'UK merchant' => array( 'GB', 'expected' ),
- 'unsupported' => array( 'XX', null ),
- );
-}
-
-/**
- * @dataProvider provider_scenarios
- */
-public function test_behavior( string $country, $expected ): void {
- $result = $this->service->get_data( $country );
- $this->assertSame( $expected, $result );
-}
-```
-
-## Setup and Teardown
-
-```php
-public function setUp(): void {
- parent::setUp();
- $this->admin_id = $this->factory->user->create(
- array( 'role' => 'administrator' )
- );
- $this->service = new ServiceClass();
-}
-
-public function tearDown(): void {
- wp_delete_user( $this->admin_id );
- unset( $GLOBALS['some_global'] );
- parent::tearDown();
-}
-```
-
-## Integration Tests
-
-**REST API endpoints:**
-
-```php
-public function test_endpoint_returns_data(): void {
- wp_set_current_user( $this->admin_id );
- $request = new \WP_REST_Request( 'GET', '/wc/v3/settings/payments' );
- $response = rest_do_request( $request );
-
- $this->assertSame( 200, $response->get_status() );
- $this->assertIsArray( $response->get_data() );
-}
-```
-
-**WordPress hooks:**
-
-```php
-public function test_hook_fires(): void {
- $fired = false;
- add_filter( 'woocommerce_payment_gateways',
- function ( $gateways ) use ( &$fired ) {
- $fired = true;
- return $gateways;
- }
- );
-
- $result = $this->service->get_gateways();
-
- $this->assertTrue( $fired );
-}
-```
-
-## Testing Private Methods
-
-**Use sparingly - prefer testing public interfaces:**
-
-```php
-public function test_private_method(): void {
- $reflection = new \ReflectionClass( $this->service );
- $method = $reflection->getMethod( 'private_method' );
- $method->setAccessible( true );
-
- $result = $method->invoke( $this->service, 'arg' );
-
- $this->assertSame( 'expected', $result );
-}
-```
-
-## Running Tests
-
-```bash
-# Class
-pnpm test:php:env -- --filter WooPaymentsServiceTest
-
-# Method
-pnpm test:php:env -- --filter ClassName::test_method_name
-
-# Pattern
-pnpm test:php:env -- --filter "test_.*_not_supported"
-
-# Verbose (use --testdox for readable output)
-pnpm test:php:env -- --testdox --filter WooPaymentsServiceTest
-```
-
-## Debugging Failures
-
-**Common issues:**
-
-| Error | Cause | Fix |
-| ----- | ----- | --- |
-| `Undefined array key` | Missing key | Check code returns key |
-| `Arrays not identical` | Extra/missing keys | Use targeted assertions |
-| `Mock not called` | Code path skipped | Check test setup |
-| `Unexpected call` | Mock too strict | Use `$this->any()` |
-
-**Read diff output:**
-
-```text
-Failed asserting that two arrays are identical.
---- Expected
-+++ Actual
-@@ @@
- Array (
-- 'key' => 'expected'
-+ 'key' => 'actual'
- )
-```
-
-## Critical Rules
-
-1. **One purpose per test** - Multiple assertions OK if testing same behavior
-2. **Test behavior, not implementation** - Avoid testing internals
-3. **Resilient assertions** - Won't break when adjacent code changes
-4. **Mock externals** - No real API calls or external plugins
-5. **Clean up** - Use tearDown() to reset state
-
-## File Organization
-
-```text
-tests/php/src/
-├── Internal/
-│ └── Admin/
-│ ├── Settings/
-│ │ ├── PaymentsProviders/WooPayments/
-│ │ │ ├── WooPaymentsServiceTest.php
-│ │ │ └── WooPaymentsRestControllerIntegrationTest.php
-│ │ └── PaymentsRestControllerIntegrationTest.php
-│ └── Suggestions/
-│ └── PaymentsExtensionSuggestionsTest.php
-└── CLAUDE.md
-```
-
-**Naming:**
-
-- Unit tests: `{ClassName}Test.php`
-- Integration: `{ClassName}IntegrationTest.php`
-- Methods: `test_{feature}_{scenario}()` or
- `test_{feature}_{scenario}_{outcome}()`
-
-## Related Docs
-
-- `plugins/woocommerce/CLAUDE.md` - Test commands, linting, workflow
-- `src/Internal/Admin/Settings/CLAUDE.md` - Settings backend patterns
-- PHPUnit: <https://phpunit.de/manual/9.6/en/index.html>
+@AGENTS.md