From e3862b7c53734247f06bf61c94ebd92edf2fef8f Mon Sep 17 00:00:00 2001 From: Tao Peng Date: Mon, 3 Aug 2026 15:21:56 -0700 Subject: [PATCH 1/3] Fix flaky test_expiration on loaded CI runners The test set a cache entry with expires_in=1 and immediately read it back. On heavily loaded runners (e.g. Windows under SQLite lock contention) the set->get latency could exceed 1s, so the entry expired before the immediate read and get() returned None, failing the assert. Widen the short-expiration window to 5s so the immediate read cannot lose the set->get race, while still validating real expiry. --- tests/unit/test_persistent_cache.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_persistent_cache.py b/tests/unit/test_persistent_cache.py index bc7067d4..a221263f 100644 --- a/tests/unit/test_persistent_cache.py +++ b/tests/unit/test_persistent_cache.py @@ -46,12 +46,15 @@ def test_expiration(tmpdir): cache_file = os.path.join(tmpdir, "cache") cache = PersistentCache(cache_file) - # Set with short expiration - cache.set("short_lived", "value", expires_in=1) + # Set with short expiration. Use a generous window here so that the + # immediate read below cannot lose the set->get race on heavily loaded CI + # runners (e.g. Windows under SQLite lock contention), where opening the + # store and retrying can itself take over a second. + cache.set("short_lived", "value", expires_in=5) assert cache.get("short_lived") == "value" # Wait for expiration - time.sleep(1.1) + time.sleep(5.1) assert cache.get("short_lived") is None # Set with longer expiration From 28281523b3d81b5044c323dafa247e9932ca5b50 Mon Sep 17 00:00:00 2001 From: Tao Peng Date: Mon, 3 Aug 2026 15:45:35 -0700 Subject: [PATCH 2/3] Speed up test_persistent_cache stress tests (~3.3x faster) The concurrency stress tests dominated the suite runtime (267s total), with two tests alone accounting for ~74%. Reduce iteration counts while keeping the same concurrent code paths (16-40 workers on a shared SQLite file), so lock-contention and correctness coverage stays intact: - multithread_shared_cache_comprehensive: num_keys 5000 -> 1000 - multiprocess_shared_cache_comprehensive: keys_per_process 1000 -> 250 - multiprocess_write_without_database_lock_errors: 10000 -> 2000 keys - multiprocess/multithread_sqlite_database_locking: num_items 4000 -> 1000 Suite runtime: 267s -> 81s. No production code changed. --- tests/unit/test_persistent_cache.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_persistent_cache.py b/tests/unit/test_persistent_cache.py index a221263f..cba9ba5f 100644 --- a/tests/unit/test_persistent_cache.py +++ b/tests/unit/test_persistent_cache.py @@ -402,7 +402,7 @@ def test_multithread_shared_cache_comprehensive(tmpdir): shared_cache = PersistentCache(cache_file) shared_cache.clear_expired() - num_keys = 5_000 + num_keys = 1_000 # Generate key-value pairs for first run (overlapping patterns to ensure intersections) first_dict = {f"key_{i}": f"first_value_{i}" for i in range(num_keys)} @@ -526,7 +526,7 @@ def test_multiprocess_shared_cache_comprehensive(tmpdir): init_cache.clear_expired() num_processes = 4 - keys_per_process = 1000 + keys_per_process = 250 # Prepare arguments for each process process_args = [ @@ -613,12 +613,12 @@ def test_multiprocess_write_without_database_lock_errors(tmpdir): with concurrent.futures.ProcessPoolExecutor(max_workers=40) as executor: futures = [ executor.submit(_cache_set, cache_file, str(key), num_sets=1) - for key in range(10000) + for key in range(2000) ] r = [f.result() for f in futures] cache = PersistentCache(cache_file) - assert 10000 == len(cache.keys()) + assert 2000 == len(cache.keys()) def _sqlite_insert_rows(cache_file, value, num_inserts=1): @@ -650,7 +650,7 @@ def test_multiprocess_sqlite_database_locking(tmpdir): cache_file = os.path.join(tmpdir, "cache_sqlite_locking") assert not os.path.exists(cache_file) - num_items = 4000 + num_items = 1000 num_inserts = 1 with concurrent.futures.ProcessPoolExecutor(max_workers=40) as executor: @@ -692,7 +692,7 @@ def test_multithread_sqlite_database_locking(tmpdir): cache_file = os.path.join(tmpdir, "cache_sqlite_locking") assert not os.path.exists(cache_file) - num_items = 4000 + num_items = 1000 num_inserts = 1 with concurrent.futures.ThreadPoolExecutor(max_workers=40) as executor: From b3cdcd45c63678d1251f4d347dbf4ef24933a0a6 Mon Sep 17 00:00:00 2001 From: Tao Peng Date: Mon, 3 Aug 2026 17:09:11 -0700 Subject: [PATCH 3/3] Pin ruff lint rules to keep CI deterministic The repo had no [tool.ruff] config, so 'ruff check mapillary_tools' relied on ruff's built-in defaults. ruff is installed unpinned in CI, and newer ruff versions expanded their default rule set (adding UP, I, BLE, SIM, PL, etc.), which flags ~240 pre-existing issues and fails the lint step. Add an explicit [tool.ruff.lint] select matching ruff's historical default (E4, E7, E9, F) so linting is deterministic regardless of the installed ruff version. No source code changed. --- pyproject.toml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 81e58d81..9a0e890f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -75,6 +75,14 @@ dev = [ "usort", ] +# Ruff configuration +# Pin the linter to an explicit rule set so `ruff check` is deterministic +# regardless of the installed ruff version. These are ruff's historical default +# rules (pyflakes + a subset of pycodestyle); newer ruff versions expanded their +# built-in defaults, which would otherwise flag hundreds of pre-existing issues. +[tool.ruff.lint] +select = ["E4", "E7", "E9", "F"] + # Mypy configuration [tool.mypy] # Global mypy settings