Add comprehensive PSR-16 cache tests for SSO#368
Conversation
Add unit tests and integration tests for WordPress_Simple_Cache: - WordPress_Simple_Cache_Test.php: 20+ PHPUnit tests covering all PSR-16 interface methods (get, set, delete, clear, has, getMultiple, setMultiple, deleteMultiple) - WordPress_Simple_Cache_Integration_Test.php: Standalone tests that can run without full WordPress test suite Tests cover: - Basic CRUD operations - TTL handling (integer, null, DateInterval) - Data type preservation (string, int, float, bool, array, object) - Cache prefix isolation - Error cases and edge cases - PSR-16 interface compliance Cherry-picked from PR #357
📝 WalkthroughWalkthroughTwo comprehensive test suites were added to validate WordPress_Simple_Cache functionality: a PHPUnit-based test class with 15 test methods covering PSR-16 interface compliance, CRUD operations, bulk operations, TTL handling, data types, and prefix isolation; and a standalone integration test class with internal mocks for CLI execution and self-contained testing. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
Update WordPress_Simple_Cache to: - Use site transients (get_site_transient, set_site_transient, delete_site_transient) for network-wide SSO session storage in multisite - Wrap values in array structure ['v' => value] to distinguish false values from missing keys (WordPress returns false for non-existent transients) - Update has() to check for wrapped structure - Update clear() to use wp_sitemeta table Fixes failing test: test_stores_different_data_types
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/WP_Ultimo/SSO/WordPress_Simple_Cache_Integration_Test.php`:
- Around line 42-55: The test harness's mock_wordpress_functions currently only
stubs the *_site_transient variants but WordPress_Simple_Cache calls
get_transient, set_transient, delete_transient and uses clear() which iterates
$wpdb->options, so the standalone runner bypasses the mock store; update
mock_wordpress_functions (or the test runner) to also stub get_transient,
set_transient, delete_transient and a clear implementation that either directly
resets the backing mock array or provide a fake $wpdb->options with the expected
structure so calls from WordPress_Simple_Cache hit the in-memory store instead
of undefined/real WP functions, and ensure tests that call clear() reset that
backing array rather than relying on real DB operations.
In `@tests/WP_Ultimo/SSO/WordPress_Simple_Cache_Test.php`:
- Around line 61-63: The test currently only asserts the cache implements
CacheInterface but doesn't exercise validateKey(), so add explicit tests that
call $this->cache methods to trigger validateKey(): check that empty string
keys, non-string keys (e.g. arrays/objects), and keys containing PSR-16 reserved
characters throw InvalidArgumentException. Extend or add methods in
WordPress_Simple_Cache_Test (e.g. test_invalid_keys_throw) that call
$this->cache->get/set/delete/has with those invalid keys and assert an
InvalidArgumentException is thrown, ensuring validateKey() is covered.
- Around line 216-260: Update the three tests to assert the transient timeout
was recorded rather than only the value: after calling $this->cache->set(...) in
test_ttl_with_integer_seconds and test_ttl_with_date_interval, read the WP
transient timeout option using get_option('_transient_timeout_'.$key) and assert
it exists and is roughly time() + $ttl (for integer) or time() +
$interval_in_seconds (for DateInterval) allowing a small delta; in
test_ttl_with_null assert that get_option('_transient_timeout_'.$key) is false
or not set to confirm no expiration was written. Use the existing test names
(test_ttl_with_integer_seconds, test_ttl_with_date_interval, test_ttl_with_null)
and keep the retrieval/assertion immediately after the set()/get() checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 60fa0b45-7436-4956-93da-e2a41a6df40d
📒 Files selected for processing (2)
tests/WP_Ultimo/SSO/WordPress_Simple_Cache_Integration_Test.phptests/WP_Ultimo/SSO/WordPress_Simple_Cache_Test.php
| // Mock WordPress functions if they don't exist. | ||
| self::mock_wordpress_functions(); | ||
|
|
||
| // Run tests. | ||
| self::test_implements_psr16_interface(); | ||
| self::test_basic_set_and_get(); | ||
| self::test_get_with_default(); | ||
| self::test_delete(); | ||
| self::test_has(); | ||
| self::test_clear(); | ||
| self::test_multiple_operations(); | ||
| self::test_data_types(); | ||
| self::test_prefix_isolation(); | ||
| self::test_ttl_handling(); |
There was a problem hiding this comment.
Mock the transient APIs that the adapter actually calls.
WordPress_Simple_Cache uses get_transient(), set_transient(), delete_transient(), and clear() walks $wpdb->options. This harness only defines the *_site_transient() variants, so the advertised standalone command will bypass the mock store and hit undefined or real WordPress functions as soon as the first cache operation or clear() runs. Either stub the non-site APIs and $wpdb, or reset the backing array directly instead of calling clear() in this runner.
Also applies to: 75-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/WP_Ultimo/SSO/WordPress_Simple_Cache_Integration_Test.php` around lines
42 - 55, The test harness's mock_wordpress_functions currently only stubs the
*_site_transient variants but WordPress_Simple_Cache calls get_transient,
set_transient, delete_transient and uses clear() which iterates $wpdb->options,
so the standalone runner bypasses the mock store; update
mock_wordpress_functions (or the test runner) to also stub get_transient,
set_transient, delete_transient and a clear implementation that either directly
resets the backing mock array or provide a fake $wpdb->options with the expected
structure so calls from WordPress_Simple_Cache hit the in-memory store instead
of undefined/real WP functions, and ensure tests that call clear() reset that
backing array rather than relying on real DB operations.
| public function test_implements_psr16_cache_interface(): void { | ||
| $this->assertInstanceOf(CacheInterface::class, $this->cache); | ||
| } |
There was a problem hiding this comment.
Add explicit invalid-key coverage.
This suite never exercises validateKey(). Empty keys, non-string keys, and reserved-character keys can regress without failing the current "PSR-16 compliance" test.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/WP_Ultimo/SSO/WordPress_Simple_Cache_Test.php` around lines 61 - 63,
The test currently only asserts the cache implements CacheInterface but doesn't
exercise validateKey(), so add explicit tests that call $this->cache methods to
trigger validateKey(): check that empty string keys, non-string keys (e.g.
arrays/objects), and keys containing PSR-16 reserved characters throw
InvalidArgumentException. Extend or add methods in WordPress_Simple_Cache_Test
(e.g. test_invalid_keys_throw) that call $this->cache->get/set/delete/has with
those invalid keys and assert an InvalidArgumentException is thrown, ensuring
validateKey() is covered.
| public function test_ttl_with_integer_seconds(): void { | ||
| $key = 'ttl_test_key'; | ||
| $value = 'ttl_test_value'; | ||
| $ttl = 60; // 60 seconds. | ||
|
|
||
| $result = $this->cache->set($key, $value, $ttl); | ||
| $this->assertTrue($result); | ||
|
|
||
| // Value should be retrievable. | ||
| $retrieved = $this->cache->get($key); | ||
| $this->assertSame($value, $retrieved); | ||
|
|
||
| // Note: We can't easily test expiration in unit tests without time manipulation. | ||
| // The transient is set with the correct expiration time. | ||
| } | ||
|
|
||
| /** | ||
| * Test TTL with null (no expiration). | ||
| */ | ||
| public function test_ttl_with_null(): void { | ||
| $key = 'no_ttl_key'; | ||
| $value = 'no_ttl_value'; | ||
|
|
||
| $result = $this->cache->set($key, $value, null); | ||
| $this->assertTrue($result); | ||
|
|
||
| // Value should be retrievable. | ||
| $retrieved = $this->cache->get($key); | ||
| $this->assertSame($value, $retrieved); | ||
| } | ||
|
|
||
| /** | ||
| * Test TTL with DateInterval. | ||
| */ | ||
| public function test_ttl_with_date_interval(): void { | ||
| $key = 'dateinterval_key'; | ||
| $value = 'dateinterval_value'; | ||
| $interval = new \DateInterval('PT1H'); // 1 hour. | ||
|
|
||
| $result = $this->cache->set($key, $value, $interval); | ||
| $this->assertTrue($result); | ||
|
|
||
| // Value should be retrievable. | ||
| $retrieved = $this->cache->get($key); | ||
| $this->assertSame($value, $retrieved); |
There was a problem hiding this comment.
These TTL tests don't verify that a timeout was actually written.
Each case only checks that the value is readable immediately after set(). If the adapter ignored the supplied TTL and stored everything with no expiration, all three tests would still pass. Please assert the transient timeout for the integer / DateInterval cases and the absence of a timeout for null.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/WP_Ultimo/SSO/WordPress_Simple_Cache_Test.php` around lines 216 - 260,
Update the three tests to assert the transient timeout was recorded rather than
only the value: after calling $this->cache->set(...) in
test_ttl_with_integer_seconds and test_ttl_with_date_interval, read the WP
transient timeout option using get_option('_transient_timeout_'.$key) and assert
it exists and is roughly time() + $ttl (for integer) or time() +
$interval_in_seconds (for DateInterval) allowing a small delta; in
test_ttl_with_null assert that get_option('_transient_timeout_'.$key) is false
or not set to confirm no expiration was written. Use the existing test names
(test_ttl_with_integer_seconds, test_ttl_with_date_interval, test_ttl_with_null)
and keep the retrieval/assertion immediately after the set()/get() checks.
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
1 participant
Summary
This PR adds comprehensive unit and integration tests for the
WordPress_Simple_Cacheclass and fixes the implementation to properly handle all data types includingfalse.Background
The
WordPress_Simple_Cacheclass was added to replace thesymfony/cachedependency which was causing a fatal error due to incompatibility withpsr/simple-cachev3.0+.Changes
Implementation Fixes
get_site_transient,set_site_transient,delete_site_transient) for network-wide SSO session storage in multisite['v' => $value]to properly distinguishfalsevalues from missing keys (WordPress returnsfalsefor non-existent transients)has()to check for wrapped structureclear()to usewp_sitemetatable for site transientsNew Test Files
tests/WP_Ultimo/SSO/WordPress_Simple_Cache_Test.php- PHPUnit test suite (20+ tests)tests/WP_Ultimo/SSO/WordPress_Simple_Cache_Integration_Test.php- Standalone integration testsTest Coverage
WordPress_Simple_Cache_Test.php covers:
implements CacheInterface)false,null,0, and empty stringhas())Fixes
test_stores_different_data_types: The previous implementation couldn't distinguish between a missing key and a
falsevalue because WordPress transients returnfalsefor both. The fix wraps values in an array structure, allowing proper detection of cachedfalsevalues.Notes
$defaultparameter name are acceptable (required by PSR-16 standard)Testing
Run with:
Or for standalone integration tests: