diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 0f583a4a..504ec728 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -109,3 +109,42 @@ def _clear_asr_installed_memo(request): _clear_all() yield _clear_all() + + +@pytest.fixture(autouse=True) +def _clean_model_manager_shutdown_state(): + """Start every test with the model manager NOT in shutdown mode (#1269). + + ``model_manager._shutting_down`` is a module-global Event and the GPU pool is + a module-global executor. Any test that runs the app lifespan flips both on + the way out — ``begin_shutdown()`` plus ``_reset_gpu_pool()`` — and nothing + puts them back, because in production that state is correct: the process is + ending. + + Across a combined ``pytest tests/ backend/tests/`` session it is not + correct, and it is not a cosmetic leak. A test that arrives with the flag set + finds a shut-down executor, so its very first ``run_in_executor`` raises + "cannot schedule new futures after shutdown" — which the preload path + classifies as a benign shutdown and swallows. The symptom is a load that + silently never starts: ``test_lifespan_shutdown_mid_load_is_clean_and_clears + _sentinel`` failed on ``assert started.is_set()`` for exactly this reason, + while passing alone. + + Reset before AND after: before so an inherited flag cannot decide this test, + after so a test that legitimately shuts down does not hand the state on. + """ + import services.model_manager as _mm + + def _clean(): + # Deliberately NOT wrapped in try/except. A reset that fails silently + # leaves the next test with stale shutdown or executor state, which is + # precisely the order-dependent failure this fixture exists to remove — + # swallowing the error would defeat the fixture while looking like it + # worked (CodeRabbit). If either of these can raise, that is a real + # problem in model_manager and it should be loud. + _mm.reset_shutdown_flag() + _mm._reset_gpu_pool() + + _clean() + yield + _clean() diff --git a/backend/tests/test_shutdown_state_isolation.py b/backend/tests/test_shutdown_state_isolation.py new file mode 100644 index 00000000..cb736222 --- /dev/null +++ b/backend/tests/test_shutdown_state_isolation.py @@ -0,0 +1,59 @@ +"""The autouse shutdown-state reset must actually reset, and fail loudly. + +``conftest._clean_model_manager_shutdown_state`` is the fix for #1269: a test +that runs the app lifespan leaves ``model_manager._shutting_down`` set and the +GPU pool torn down, and the next test then finds every ``run_in_executor`` +raising "cannot schedule new futures after shutdown" — swallowed by the preload +path as a benign shutdown, so the symptom is a load that silently never starts. + +A fixture with nothing asserting it is a fixture nobody notices breaking. The +pair below is deliberately order-dependent (pytest runs tests in definition +order within a file): the first test dirties exactly the state the lifespan +dirties, the second asserts it arrived clean. Delete the fixture and the second +test fails; that is the fail-before/pass-after this file exists to provide. +""" + +import os + +import services.model_manager as mm + +_CONFTEST = os.path.join(os.path.dirname(os.path.abspath(__file__)), "conftest.py") + + +def test_dirty_the_shutdown_state(): + """Stand in for any lifespan-running test: leave the module globals in the + exact state graceful shutdown leaves them.""" + mm.begin_shutdown() + mm._reset_gpu_pool() + assert mm.is_shutting_down() + + +def test_next_test_starts_clean(): + """Runs immediately after the test above and must not inherit its state.""" + assert not mm.is_shutting_down(), ( + "the shutdown flag leaked from the previous test — the autouse fixture " + "in backend/tests/conftest.py is not resetting it, and every executor " + "submit in this test will now raise 'cannot schedule new futures after " + "shutdown' and be misread as a benign cancellation (#1269)" + ) + + +def test_reset_failures_are_not_swallowed(): + """A reset wrapped in ``except: pass`` hands the next test stale state while + reporting success — the fixture would look like it worked and #1269 would + come back with the evidence removed. Mechanical, so it stays true.""" + with open(_CONFTEST, encoding="utf-8") as fh: + src = fh.read() + marker = "def _clean_model_manager_shutdown_state(" + assert marker in src, f"fixture renamed or removed from {_CONFTEST}" + body = src.split(marker, 1)[1].split("\n@", 1)[0] + # Comments and the docstring explain *why* there is no try/except; only the + # code is evidence of whether there is one. + code = "\n".join( + line for line in body.split('"""')[-1].splitlines() + if not line.lstrip().startswith("#") + ) + assert "except" not in code, ( + "the shutdown-state reset catches exceptions; a failed reset must fail " + "the test that caused it, not leak into the next one:\n" + code + ) diff --git a/tests/test_torch_compile_path_gate.py b/tests/test_torch_compile_path_gate.py index c5ccf2cb..b920ecab 100644 --- a/tests/test_torch_compile_path_gate.py +++ b/tests/test_torch_compile_path_gate.py @@ -35,9 +35,17 @@ def _env(monkeypatch, *, torch_file, triton=True): monkeypatch.setattr( engine_env, "settings_store", None, raising=False ) - import sys as _sys, types as _types - _sys.modules["services.settings_store"] = _types.SimpleNamespace( - get_text=lambda *a, **k: "0" + # monkeypatch.setitem, never a raw assignment: a bare object dropped into + # sys.modules leaks process-wide out of collection and breaks every later + # import of the real module in a mixed run. backend/tests/ + # test_no_module_stubs.py exists to catch exactly that, and caught this. + import sys as _sys + import types as _types + + monkeypatch.setitem( + _sys.modules, + "services.settings_store", + _types.SimpleNamespace(get_text=lambda *a, **k: "0"), ) monkeypatch.setattr( engine_env, "_cuda_arch_supported_for_compile", lambda: (True, "")