From 0f65580122edfde4f646ea42cff3e808ae0284e7 Mon Sep 17 00:00:00 2001
From: masklinn <github.com@masklinn.net>
Date: Wed, 27 Mar 2024 22:11:29 +0100
Subject: [PATCH] Add test to ensure backfilling results does not lead to
 evictions

Partial results are back-filled (new domains added) by re-setting them
in the cache. With a sufficiently incorrect implementation, the cache
can evict entries on that occasion even though it does not need
to (because we're replacing an existing entry).

Exactly that should have been fixed by #204, but was not tested at the
time.

Fixes #199
---
 tests/test_caches.py | 73 +++++++++++++++++++++-----------------------
 1 file changed, 35 insertions(+), 38 deletions(-)

diff --git a/tests/test_caches.py b/tests/test_caches.py
index d612520..70934ee 100644
--- a/tests/test_caches.py
+++ b/tests/test_caches.py
@@ -1,17 +1,14 @@
 from collections import OrderedDict
 
+import pytest  # type: ignore
+
 from ua_parser import (
-    BasicResolver,
     CachingResolver,
-    Device,
     Domain,
-    OS,
     Parser,
     PartialResult,
-    UserAgent,
 )
-from ua_parser.caching import Lru
-from ua_parser.matchers import DeviceMatcher, OSMatcher, UserAgentMatcher
+from ua_parser.caching import Lru, S3Fifo, Sieve
 
 
 def test_lru():
@@ -19,7 +16,9 @@ def test_lru():
     popped LRU-first.
     """
     cache = Lru(2)
-    p = Parser(CachingResolver(BasicResolver(([], [], [])), cache))
+    p = Parser(
+        CachingResolver(lambda s, d: PartialResult(d, None, None, None, s), cache)
+    )
 
     p.parse("a")
     p.parse("b")
@@ -41,37 +40,35 @@ def test_lru():
     )
 
 
-def test_backfill():
-    """Tests that caches handle partial parsing correctly, by updating the
-    existing entry when new parts get parsed.
+@pytest.mark.parametrize("cache", [Lru, S3Fifo, Sieve])
+def test_backfill(cache):
+    """Tests that caches handle partial parsing correctly, by updating
+    the existing entry when new parts get parsed, without evicting
+    still-fitting entries.
     """
-    cache = Lru(2)
-    p = Parser(
-        CachingResolver(
-            BasicResolver(
-                (
-                    [UserAgentMatcher("(a)")],
-                    [OSMatcher("(a)")],
-                    [DeviceMatcher("(a)")],
-                )
-            ),
-            cache,
-        )
-    )
+    misses = 0
+
+    def resolver(ua: str, domains: Domain, /) -> PartialResult:
+        nonlocal misses
+        misses += 1
+        return PartialResult(domains, None, None, None, ua)
+
+    p = Parser(CachingResolver(resolver, cache(10)))
 
+    # fill the cache first, hit every entry twice to ensure QD is defeated
+    for s in map(str, range(9)):
+        p.parse(s)
+        p.parse(s)
+    assert misses == 9
+    # add a partial entry
     p.parse_user_agent("a")
-    assert cache.cache == {
-        "a": PartialResult(Domain.USER_AGENT, UserAgent("a"), None, None, "a"),
-    }
-    p("a", Domain.OS)
-    assert cache.cache == {
-        "a": PartialResult(
-            Domain.USER_AGENT | Domain.OS, UserAgent("a"), OS("a"), None, "a"
-        ),
-    }
-    p.parse("a")
-    assert cache.cache == {
-        "a": PartialResult(
-            Domain.ALL, UserAgent("a"), OS("a"), Device("a", None, "a"), "a"
-        ),
-    }
+    # fill the partial entry, counts as a miss since it needs to
+    # resolve the new bit
+    p.parse_os("a")
+    assert misses == 11
+
+    misses = 0
+    # check that the original entries are all hits
+    for s in map(str, range(9)):
+        p.parse(s)
+    assert misses == 0