From 3b9c632a1fa339abcfd457973dcf10810640e760 Mon Sep 17 00:00:00 2001 From: xuhaonan013 <78591720+xuhaonan013@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:53:47 +0800 Subject: [PATCH] test: make model cache regressions hermetic (#7235) * test: make model cache regressions hermetic * test: wait for model cache worker cleanup * test: fully invalidate model cache state * ci: re-trigger checks (first-time contributor workflow approval) --------- Co-authored-by: nesquena-hermes Co-authored-by: n --- tests/test_ttl_cache.py | 50 +++++++++++------- tests/test_wakeup_model_resolve_hang.py | 68 +++++++++++++++++-------- 2 files changed, 79 insertions(+), 39 deletions(-) diff --git a/tests/test_ttl_cache.py b/tests/test_ttl_cache.py index 33c2a0b6..a803c16f 100644 --- a/tests/test_ttl_cache.py +++ b/tests/test_ttl_cache.py @@ -217,7 +217,8 @@ def test_invalidate_models_cache_direct(): """Call invalidate_models_cache() after populating the cache. _AVAILABLE_MODELS_CACHE should be None and the next call should re-scan. """ - _reset_cache() + config.invalidate_models_cache() + saved_mtime = config._cfg_mtime # Ensure _cfg_mtime matches file so mtime check doesn't invalidate try: @@ -225,24 +226,35 @@ def test_invalidate_models_cache_direct(): except OSError: config._cfg_mtime = 0.0 - # First call populates cache - result1 = config.get_available_models() - assert config._available_models_cache is not None, "Cache should be populated" - first_ts = config._available_models_cache_ts + # A bounded cold rebuild may legitimately return the static fallback before + # its worker publishes the cache. Force the synchronous path so this test + # establishes its own cache-populated precondition instead of relying on a + # sibling test having warmed it first. + with patch.object(config, "_LIVE_REBUILD_BUDGET_SECONDS", 0.0): + try: + result1 = config.get_available_models() + assert config._available_models_cache is not None, ( + "Cache should be populated" + ) + first_ts = config._available_models_cache_ts - # Directly invalidate - config.invalidate_models_cache() + # Directly invalidate + config.invalidate_models_cache() - # Cache must be cleared - assert config._available_models_cache is None, ( - "invalidate_models_cache() should set _AVAILABLE_MODELS_CACHE to None" - ) + # Cache must be cleared + assert config._available_models_cache is None, ( + "invalidate_models_cache() should set _AVAILABLE_MODELS_CACHE to None" + ) - # Next call should re-scan and produce a fresh cache - result2 = config.get_available_models() - assert config._available_models_cache is not None, "Cache should be re-populated" - assert config._available_models_cache_ts >= first_ts, ( - "Cache timestamp should be updated after re-scan" - ) - - _reset_cache() + # Next call should re-scan and produce a fresh cache + result2 = config.get_available_models() + assert config._available_models_cache is not None, ( + "Cache should be re-populated" + ) + assert config._available_models_cache_ts >= first_ts, ( + "Cache timestamp should be updated after re-scan" + ) + assert "groups" in result1 and "groups" in result2 + finally: + config._cfg_mtime = saved_mtime + config.invalidate_models_cache() diff --git a/tests/test_wakeup_model_resolve_hang.py b/tests/test_wakeup_model_resolve_hang.py index 9da26b98..a030914f 100644 --- a/tests/test_wakeup_model_resolve_hang.py +++ b/tests/test_wakeup_model_resolve_hang.py @@ -29,6 +29,7 @@ Two fixes, two tests: from __future__ import annotations +import threading import time from pathlib import Path @@ -176,15 +177,23 @@ def test_chat_start_survives_slow_provider_probe(monkeypatch): cfg.invalidate_models_cache() monkeypatch.setattr(cfg, "_LIVE_REBUILD_BUDGET_SECONDS", 0.4, raising=True) - started = {"n": 0} + probe_started = threading.Event() + release_probe = threading.Event() + real_thread = threading.Thread + rebuild_workers = [] + rebuild_worker = None + + def _record_thread(*args, **kwargs): + worker = real_thread(*args, **kwargs) + if kwargs.get("name") == "models-catalog-rebuild": + rebuild_workers.append(worker) + return worker def _slow_rebuild(_builder): - started["n"] += 1 - # >> budget — models the hung Copilot HTTPS call. 0.8s is 2× the - # monkeypatched 0.4s budget, which is the smallest gap that still - # robustly proves the contract while keeping suite wall-clock low - # (was 3.0s; suite-latency cleanup per Copilot review). - time.sleep(0.8) + probe_started.set() + # Hold the probe past the foreground budget without assuming how + # quickly a loaded CI worker schedules or sleeps. + assert release_probe.wait(timeout=5.0), "test did not release slow probe" return { "active_provider": "anthropic", "default_model": "anthropic/claude-sonnet-4", @@ -197,21 +206,40 @@ def test_chat_start_survives_slow_provider_probe(monkeypatch): monkeypatch.setattr(cfg, "_invoke_models_rebuild", _slow_rebuild, raising=True) # Ensure no disk cache short-circuits the cold path. monkeypatch.setattr(cfg, "_load_models_cache_from_disk", lambda: None, raising=True) + monkeypatch.setattr(cfg.threading, "Thread", _record_thread, raising=True) - t0 = time.monotonic() - result = cfg.get_available_models() - elapsed = time.monotonic() - t0 + try: + result = cfg.get_available_models() - assert started["n"] == 1, "the rebuild worker should have been started" - assert elapsed < 2.0, ( - f"get_available_models() blocked {elapsed:.2f}s on a hung probe — " - f"the {cfg._LIVE_REBUILD_BUDGET_SECONDS}s budget did not bound it" - ) - # The fallback must be a structurally valid, usable catalog. - assert isinstance(result, dict) - for k in ("active_provider", "default_model", "configured_model_badges", "groups"): - assert k in result, f"fallback catalog missing {k!r}" - assert isinstance(result["groups"], list) + assert probe_started.wait(timeout=5.0), ( + "the rebuild worker should have started" + ) + assert len(rebuild_workers) == 1 + rebuild_worker = rebuild_workers[0] + # The foreground returned while the probe was deliberately blocked, so + # the result can only be the bounded-path fallback. Assert that contract + # directly instead of comparing wall-clock time on a contended runner. + assert rebuild_worker.is_alive() + assert isinstance(result, dict) + for k in ( + "active_provider", + "default_model", + "configured_model_badges", + "groups", + ): + assert k in result, f"fallback catalog missing {k!r}" + assert isinstance(result["groups"], list) + finally: + release_probe.set() + for worker in rebuild_workers: + worker.join(timeout=5.0) + for worker in rebuild_workers: + assert not worker.is_alive(), "slow rebuild worker did not finish" + if rebuild_workers: + assert cfg._cache_build_in_progress is False + cfg.invalidate_models_cache() + + assert rebuild_worker is not None def test_minimal_static_catalog_is_network_free(monkeypatch):