fix(tests): banner update-check prefetch no longer poisons process-wide subprocess mocks
The prefetch_update_check daemon thread (started at tui_gateway.server import time) shells out to git via the shared subprocess singleton at an arbitrary point after import. Tests that patch subprocess.run/Popen process-wide can capture that stray spawn in call_args, flaking their assertions: on 2026-08-28 CI, test_slash_worker_popen_uses_utf8_replace saw encoding=None from the thread's un-encoded 'git fetch' (red on main, run 33175879563) and test_deliver_validates_profile_and_runs_transport captured argv ['rev-parse', 'FETCH_HEAD'] from the shallow-checkout banner path (FLAKY frame, run 33183215857). Fix the class at the source: _skip_background_prefetch() makes both prefetch_update_check and prefetch_banner_data no-ops under pytest (nothing in tests needs a live update check; the done event is set so get_update_result callers don't burn their timeout). Tests exercising the prefetch itself monkeypatch the predicate. Sabotage-verified regression tests pin both no-ops.
This commit is contained in:
@@ -4,6 +4,7 @@ import logging
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
import threading
|
||||
import time
|
||||
from pathlib import Path
|
||||
@@ -505,8 +506,36 @@ def _daemon(name: Optional[str], target) -> None:
|
||||
threading.Thread(target=lambda: _quiet(target), name=name, daemon=True).start()
|
||||
|
||||
|
||||
def _skip_background_prefetch() -> bool:
|
||||
"""True when the banner's background prefetch threads must not start.
|
||||
|
||||
Under pytest the prefetch daemon threads shell out to git
|
||||
(``fetch``/``rev-parse``/``rev-list``) at an arbitrary point after import,
|
||||
and any test that patches the process-wide ``subprocess`` singleton
|
||||
(``patch("subprocess.run")`` / ``patch("subprocess.Popen")`` — note
|
||||
``subprocess.run`` calls ``subprocess.Popen`` internally, so a Popen patch
|
||||
captures run() spawns too) can record that stray git spawn instead of —
|
||||
or in addition to — the call it meant to pin. That cross-talk
|
||||
manufactured CI flakes in tests/tui_gateway/test_subprocess_encoding.py
|
||||
(``encoding=None`` from the update thread's un-encoded ``git fetch``) and
|
||||
tests/tui_gateway/test_bot_relay_methods.py (``argv == ['rev-parse',
|
||||
'FETCH_HEAD']`` from the shallow-checkout path), both of which import
|
||||
``tui_gateway.server`` — which starts this prefetch at import time.
|
||||
Nothing under pytest needs a live update check; tests that exercise the
|
||||
prefetch itself monkeypatch this predicate to False.
|
||||
"""
|
||||
return "PYTEST_CURRENT_TEST" in os.environ or "pytest" in sys.modules
|
||||
|
||||
|
||||
def prefetch_update_check():
|
||||
"""Kick off update check in a background daemon thread."""
|
||||
"""Kick off update check in a background daemon thread.
|
||||
|
||||
No-op under pytest — see ``_skip_background_prefetch``.
|
||||
"""
|
||||
if _skip_background_prefetch():
|
||||
_update_check_done.set()
|
||||
return
|
||||
|
||||
def _run():
|
||||
global _update_result
|
||||
_update_result = check_for_updates(passive=True)
|
||||
@@ -527,6 +556,13 @@ def prefetch_banner_data():
|
||||
global _banner_data_prefetch_started
|
||||
if _banner_data_prefetch_started:
|
||||
return
|
||||
if _skip_background_prefetch():
|
||||
# Same stray-git-spawn cross-talk class as prefetch_update_check:
|
||||
# get_git_banner_state() shells out via the shared subprocess
|
||||
# singleton from a daemon thread, poisoning process-wide subprocess
|
||||
# mocks in unrelated tests.
|
||||
_banner_data_prefetch_started = True
|
||||
return
|
||||
_banner_data_prefetch_started = True
|
||||
_daemon("banner-data-prefetch", lambda: [_quiet(warm) for warm in (
|
||||
get_git_banner_state, get_latest_release_tag, get_available_skills)])
|
||||
|
||||
@@ -98,10 +98,12 @@ def test_cache_is_daily_but_invalidated_when_head_moves(git_repo, monkeypatch):
|
||||
tip.assert_called_once()
|
||||
|
||||
|
||||
def test_prefetch_non_blocking():
|
||||
def test_prefetch_non_blocking(monkeypatch):
|
||||
"""prefetch_update_check() should return immediately without blocking."""
|
||||
# Reset module state; force the real (non-pytest) thread path.
|
||||
banner._update_result = None
|
||||
banner._update_check_done = threading.Event()
|
||||
monkeypatch.setattr(banner, "_skip_background_prefetch", lambda: False)
|
||||
|
||||
with patch.object(banner, "check_for_updates", return_value=5):
|
||||
start = time.monotonic()
|
||||
@@ -111,6 +113,39 @@ def test_prefetch_non_blocking():
|
||||
assert banner._update_result == 5
|
||||
|
||||
|
||||
def test_prefetch_update_check_is_noop_under_pytest():
|
||||
"""Under pytest the prefetch must NOT start the git-spawning daemon
|
||||
thread: a process-wide ``patch("subprocess.run")`` in an unrelated test
|
||||
can capture the thread's ``git fetch``/``rev-parse`` spawns, flaking the
|
||||
unrelated test's call_args assertions (seen in
|
||||
tests/tui_gateway/test_subprocess_encoding.py and
|
||||
test_bot_relay_methods.py on CI, 2026-08-28)."""
|
||||
banner._update_result = None
|
||||
banner._update_check_done = threading.Event()
|
||||
|
||||
before = {t.ident for t in threading.enumerate()}
|
||||
with patch.object(banner, "check_for_updates") as mock_check:
|
||||
banner.prefetch_update_check()
|
||||
# The done event is set synchronously so get_update_result() callers
|
||||
# don't burn their timeout waiting on a check that will never run.
|
||||
assert banner._update_check_done.is_set()
|
||||
mock_check.assert_not_called()
|
||||
after = {t.ident for t in threading.enumerate()}
|
||||
assert after <= before, "prefetch_update_check spawned a thread under pytest"
|
||||
|
||||
|
||||
def test_prefetch_banner_data_is_noop_under_pytest(monkeypatch):
|
||||
"""Same stray-git-spawn class: prefetch_banner_data must not start its
|
||||
daemon thread under pytest."""
|
||||
monkeypatch.setattr(banner, "_banner_data_prefetch_started", False)
|
||||
with patch.object(banner, "get_git_banner_state") as mock_state:
|
||||
banner.prefetch_banner_data()
|
||||
# Give a hypothetical stray thread a beat to run — nothing should.
|
||||
time.sleep(0.05)
|
||||
mock_state.assert_not_called()
|
||||
assert banner._banner_data_prefetch_started is True
|
||||
|
||||
|
||||
def test_upstream_main_sha_ls_remote_fallback_disables_git_prompts(monkeypatch):
|
||||
"""When the API is unreachable the HTTPS ls-remote fallback must never inherit the terminal."""
|
||||
monkeypatch.setattr(banner, "_github_branch_tip", lambda slug, branch: None)
|
||||
|
||||
Reference in New Issue
Block a user