Automattic / wp-cache-memcached

Memcached Object Cache for WordPress
6 stars 0 forks source link

Allow non-persistent groups to have data add()ed after a failed get() #8

Open mreishus opened 2 months ago

mreishus commented 2 months ago

This is to fix the bug as described in here: p55Cj4-3nK-p2

However, this change causes many other failures, because several tests are expecting "found" to be false even after setting data. Example: https://github.com/Automattic/wp-cache-memcached/blob/a104fc5c3bfeea2e35409a21627a1778b372fd16/tests/test-wp-object-cache.php#L164

I don't quite understand why this is. Any thoughts here?

24) Test_WP_Object_Cache::test_replace_for_non_persistent_groups with data set "string" ('string', '')
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     'value' => ''
-    'found' => true
+    'found' => false
 )

/home/circleci/project/tests/test-wp-object-cache.php:163

25) Test_WP_Object_Cache::test_replace_for_non_persistent_groups with data set "float" (1.234, 5.678)
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     'value' => 5.678
-    'found' => true
+    'found' => false
 )

/home/circleci/project/tests/test-wp-object-cache.php:163

26) Test_WP_Object_Cache::test_replace_for_non_persistent_groups with data set "object" (stdClass Object (...), stdClass Object ())
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     'value' => stdClass Object ()
-    'found' => true
+    'found' => false
 )

/home/circleci/project/tests/test-wp-object-cache.php:163

FAILURES!
Tests: 151, Assertions: 786, Failures: 26.
WPprodigy commented 2 months ago

However, this change causes many other failures, because several tests are expecting "found" to be false even after setting data.

Yeah, I don't really like it either. I recall keeping it for backwards-compat:

It's using "found" to only mean "found in remote memcached", though kinda unclear if that's consistently the approach. And I'd guess it also makes for some rather awkward scenarios if somebody were wanting to check the $found result from a wp_cache_get() on a non-persistent item.

mreishus commented 2 months ago

Ah, interesting. I was not aware. Apparently the WPCOM client is full of many 'quirks' that aren't standard. :) I will think on this some, any suggestions in the meantime are welcome.