Skip to content

Commit 0f65580

Browse files
committed
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
1 parent bb74478 commit 0f65580

File tree

1 file changed

+35
-38
lines changed

1 file changed

+35
-38
lines changed

tests/test_caches.py

Lines changed: 35 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,24 @@
11
from collections import OrderedDict
22

3+
import pytest # type: ignore
4+
35
from ua_parser import (
4-
BasicResolver,
56
CachingResolver,
6-
Device,
77
Domain,
8-
OS,
98
Parser,
109
PartialResult,
11-
UserAgent,
1210
)
13-
from ua_parser.caching import Lru
14-
from ua_parser.matchers import DeviceMatcher, OSMatcher, UserAgentMatcher
11+
from ua_parser.caching import Lru, S3Fifo, Sieve
1512

1613

1714
def test_lru():
1815
"""Tests that the cache entries do get moved when accessed, and are
1916
popped LRU-first.
2017
"""
2118
cache = Lru(2)
22-
p = Parser(CachingResolver(BasicResolver(([], [], [])), cache))
19+
p = Parser(
20+
CachingResolver(lambda s, d: PartialResult(d, None, None, None, s), cache)
21+
)
2322

2423
p.parse("a")
2524
p.parse("b")
@@ -41,37 +40,35 @@ def test_lru():
4140
)
4241

4342

44-
def test_backfill():
45-
"""Tests that caches handle partial parsing correctly, by updating the
46-
existing entry when new parts get parsed.
43+
@pytest.mark.parametrize("cache", [Lru, S3Fifo, Sieve])
44+
def test_backfill(cache):
45+
"""Tests that caches handle partial parsing correctly, by updating
46+
the existing entry when new parts get parsed, without evicting
47+
still-fitting entries.
4748
"""
48-
cache = Lru(2)
49-
p = Parser(
50-
CachingResolver(
51-
BasicResolver(
52-
(
53-
[UserAgentMatcher("(a)")],
54-
[OSMatcher("(a)")],
55-
[DeviceMatcher("(a)")],
56-
)
57-
),
58-
cache,
59-
)
60-
)
49+
misses = 0
50+
51+
def resolver(ua: str, domains: Domain, /) -> PartialResult:
52+
nonlocal misses
53+
misses += 1
54+
return PartialResult(domains, None, None, None, ua)
55+
56+
p = Parser(CachingResolver(resolver, cache(10)))
6157

58+
# fill the cache first, hit every entry twice to ensure QD is defeated
59+
for s in map(str, range(9)):
60+
p.parse(s)
61+
p.parse(s)
62+
assert misses == 9
63+
# add a partial entry
6264
p.parse_user_agent("a")
63-
assert cache.cache == {
64-
"a": PartialResult(Domain.USER_AGENT, UserAgent("a"), None, None, "a"),
65-
}
66-
p("a", Domain.OS)
67-
assert cache.cache == {
68-
"a": PartialResult(
69-
Domain.USER_AGENT | Domain.OS, UserAgent("a"), OS("a"), None, "a"
70-
),
71-
}
72-
p.parse("a")
73-
assert cache.cache == {
74-
"a": PartialResult(
75-
Domain.ALL, UserAgent("a"), OS("a"), Device("a", None, "a"), "a"
76-
),
77-
}
65+
# fill the partial entry, counts as a miss since it needs to
66+
# resolve the new bit
67+
p.parse_os("a")
68+
assert misses == 11
69+
70+
misses = 0
71+
# check that the original entries are all hits
72+
for s in map(str, range(9)):
73+
p.parse(s)
74+
assert misses == 0

0 commit comments

Comments
 (0)