Merge pull request rails#43693 from esparta/fix_race_conditions_test_cache_iii

ActiveSupport::Cache - fix more race conditions on test/cache - part III

Author: rafaelfranca

activesupport/test/cache/behaviors/cache_instrumentation_behavior.rb

@@ -10,28 +10,35 @@ def test_fetch_multi_uses_write_multi_entries_store_provider_interface
   end
 
   def test_write_multi_instrumentation
-    writes = { "a" => "aa", "b" => "bb" }
+    key_1 = SecureRandom.uuid
+    key_2 = SecureRandom.uuid
+    value_1 = SecureRandom.alphanumeric
+    value_2 = SecureRandom.alphanumeric
+    writes = { key_1 => value_1, key_2 => value_2 }
 
     events = with_instrumentation "write_multi" do
       @cache.write_multi(writes)
     end
 
     assert_equal %w[ cache_write_multi.active_support ], events.map(&:name)
     assert_nil events[0].payload[:super_operation]
-    assert_equal({ "a" => "aa", "b" => "bb" }, events[0].payload[:key])
+    assert_equal({ key_1 => value_1, key_2 => value_2 }, events[0].payload[:key])
   end
 
   def test_instrumentation_with_fetch_multi_as_super_operation
-    @cache.write("b", "bb")
+    key_1 = SecureRandom.uuid
+    @cache.write(key_1, SecureRandom.alphanumeric)
+
+    key_2 = SecureRandom.uuid
 
     events = with_instrumentation "read_multi" do
-      @cache.fetch_multi("a", "b") { |key| key * 2 }
+      @cache.fetch_multi(key_2, key_1) { |key| key * 2 }
     end
 
     assert_equal %w[ cache_read_multi.active_support ], events.map(&:name)
     assert_equal :fetch_multi, events[0].payload[:super_operation]
-    assert_equal ["a", "b"], events[0].payload[:key]
-    assert_equal ["b"], events[0].payload[:hits]
+    assert_equal [key_2, key_1], events[0].payload[:key]
+    assert_equal [key_1], events[0].payload[:hits]
     assert_equal @cache.class.name, events[0].payload[:store]
   end
 
@@ -48,15 +55,18 @@ def test_instrumentation_empty_fetch_multi
   end
 
   def test_read_multi_instrumentation
-    @cache.write("b", "bb")
+    key_1 = SecureRandom.uuid
+    @cache.write(key_1, SecureRandom.alphanumeric)
+
+    key_2 = SecureRandom.uuid
 
     events = with_instrumentation "read_multi" do
-      @cache.read_multi("a", "b")
+      @cache.read_multi(key_2, key_1)
     end
 
     assert_equal %w[ cache_read_multi.active_support ], events.map(&:name)
-    assert_equal ["a", "b"], events[0].payload[:key]
-    assert_equal ["b"], events[0].payload[:hits]
+    assert_equal [key_2, key_1], events[0].payload[:key]
+    assert_equal [key_1], events[0].payload[:hits]
     assert_equal @cache.class.name, events[0].payload[:store]
   end
 

activesupport/test/cache/stores/redis_cache_store_test.rb

@@ -238,8 +238,10 @@ def test_forward_compatibility
       @old_store = lookup_store
       ActiveSupport::Cache.format_version = previous_format
 
-      @old_store.write("foo", "bar")
-      assert_equal "bar", @cache.read("foo")
+      key = SecureRandom.uuid
+      value = SecureRandom.alphanumeric
+      @old_store.write(key, value)
+      assert_equal value, @cache.read(key)
     end
 
     def test_backward_compatibility
@@ -248,8 +250,10 @@ def test_backward_compatibility
       @old_store = lookup_store
       ActiveSupport::Cache.format_version = previous_format
 
-      @cache.write("foo", "bar")
-      assert_equal "bar", @old_store.read("foo")
+      key = SecureRandom.uuid
+      value = SecureRandom.alphanumeric
+      @cache.write(key, value)
+      assert_equal value, @old_store.read(key)
     end
 
     def after_teardown
@@ -363,11 +367,15 @@ def emulating_unavailability
 
   class DeleteMatchedTest < StoreTest
     test "deletes keys matching glob" do
-      @cache.write("foo", "bar")
-      @cache.write("fu", "baz")
-      @cache.delete_matched("foo*")
-      assert_not @cache.exist?("foo")
-      assert @cache.exist?("fu")
+      prefix = SecureRandom.alphanumeric
+      key = "#{prefix}#{SecureRandom.uuid}"
+      @cache.write(key, "bar")
+
+      other_key = SecureRandom.uuid
+      @cache.write(other_key, SecureRandom.alphanumeric)
+      @cache.delete_matched("#{prefix}*")
+      assert_not @cache.exist?(key)
+      assert @cache.exist?(other_key)
     end
 
     test "fails with regexp matchers" do
@@ -379,19 +387,26 @@ class DeleteMatchedTest < StoreTest
 
   class ClearTest < StoreTest
     test "clear all cache key" do
-      @cache.write("foo", "bar")
-      @cache.write("fu", "baz")
+      key = SecureRandom.uuid
+      other_key = SecureRandom.uuid
+      @cache.write(key, SecureRandom.uuid)
+      @cache.write(other_key, SecureRandom.uuid)
       @cache.clear
-      assert_not @cache.exist?("foo")
-      assert_not @cache.exist?("fu")
+      assert_not @cache.exist?(key)
+      assert_not @cache.exist?(other_key)
     end
 
     test "only clear namespace cache key" do
-      @cache.write("foo", "bar")
-      @cache.redis.set("fu", "baz")
+      key = SecureRandom.uuid
+      other_key = SecureRandom.uuid
+
+      @cache.write(key, SecureRandom.alphanumeric)
+      @cache.redis.set(other_key, SecureRandom.alphanumeric)
       @cache.clear
-      assert_not @cache.exist?("foo")
-      assert @cache.redis.exists?("fu")
+
+      assert_not @cache.exist?(key)
+      assert @cache.redis.exists?(other_key)
+      @cache.redis.del(other_key)
     end
 
     test "clear all cache key with Redis::Distributed" do