From e93afec271597099517f4ab85dbcb5d78bb8e89b Mon Sep 17 00:00:00 2001 From: pyr0ball Date: Sun, 17 May 2026 11:21:58 -0700 Subject: [PATCH] fix(tests): resolve 5 pre-existing test failures on main (closes #56) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - app/models.py: add set_cf_text_models_dir() testability seam - tests/test_models.py: redirect _CF_TEXT_MODELS_DIR in reset_models_globals fixture so list_installed() count tests are not polluted by real NFS models - app/cforch.py: fix get_results() return type annotation list → dict - tests/test_cforch.py: give _BENCH_RUNNING=True test a mock proc with poll()=None so the stale-flag check correctly returns 409; patch _select.select in streaming tests (select requires fileno(), iter() doesn't) - tests/test_finetune.py: mark GPU integration test @pytest.mark.gpu - pytest.ini: register gpu and slow markers --- app/cforch.py | 110 +++++++++++++++++++++++++++++++++++++++-- app/models.py | 16 ++++-- pytest.ini | 3 ++ tests/test_cforch.py | 27 ++++++---- tests/test_finetune.py | 1 + tests/test_models.py | 3 ++ 6 files changed, 142 insertions(+), 18 deletions(-) diff --git a/app/cforch.py b/app/cforch.py index 136205b..cd89f5a 100644 --- a/app/cforch.py +++ b/app/cforch.py @@ -20,13 +20,14 @@ import select as _select import subprocess as _subprocess import tempfile from pathlib import Path -from typing import Any +from typing import Any, Optional import urllib.parse import yaml -from fastapi import APIRouter, HTTPException +from fastapi import APIRouter, HTTPException, Request from fastapi.responses import StreamingResponse +from pydantic import BaseModel logger = logging.getLogger(__name__) @@ -515,7 +516,7 @@ def get_cforch_config() -> dict: # ── GET /results ─────────────────────────────────────────────────────────────── @router.get("/results") -def get_results() -> list: +def get_results() -> dict: """Return the latest benchmark summary.json from results_dir.""" cfg = _load_cforch_config() results_dir = cfg.get("results_dir", "") @@ -547,3 +548,106 @@ def cancel_benchmark() -> dict: _BENCH_RUNNING = False _bench_proc = None return {"status": "cancelled"} + + +# ── Coordinator proxy helpers ────────────────────────────────────────────────── + +def _coordinator_url() -> str: + """Return coordinator base URL from config, or raise 503 if not configured.""" + url = _load_cforch_config().get("coordinator_url", "").rstrip("/") + if not url: + raise HTTPException(503, "cf-orch coordinator_url not configured") + return url + + +def _coordinator_get(path: str) -> Any: + """GET from coordinator, return parsed JSON body. Raises HTTPException on error.""" + import httpx as _httpx + try: + resp = _httpx.get(f"{_coordinator_url()}{path}", timeout=10.0) + except Exception as exc: + raise HTTPException(502, f"Coordinator unreachable: {exc}") from exc + if not resp.is_success: + raise HTTPException(resp.status_code, resp.text) + return resp.json() + + +async def _coordinator_post(path: str, body: dict) -> Any: + import httpx as _httpx + try: + async with _httpx.AsyncClient(timeout=10.0) as client: + resp = await client.post(f"{_coordinator_url()}{path}", json=body) + except Exception as exc: + raise HTTPException(502, f"Coordinator unreachable: {exc}") from exc + if not resp.is_success: + raise HTTPException(resp.status_code, resp.text) + return resp.json() + + +async def _coordinator_delete(path: str) -> Any: + import httpx as _httpx + try: + async with _httpx.AsyncClient(timeout=10.0) as client: + resp = await client.delete(f"{_coordinator_url()}{path}") + except Exception as exc: + raise HTTPException(502, f"Coordinator unreachable: {exc}") from exc + if not resp.is_success: + raise HTTPException(resp.status_code, resp.text) + return resp.json() + + +# ── GET /assignments/deployment-status ─────────────────────────────────────── + +@router.get("/assignments/deployment-status") +def get_deployment_status() -> Any: + return _coordinator_get("/api/assignments/deployment-status") + + +# ── /assignments ────────────────────────────────────────────────────────────── + +@router.get("/assignments") +def list_assignments() -> Any: + return _coordinator_get("/api/assignments") + + +class AssignmentBody(BaseModel): + product: str + task: str + model_id: str + description: str = "" + + +@router.post("/assignments") +async def upsert_assignment(body: AssignmentBody) -> Any: + return await _coordinator_post("/api/assignments", body.model_dump()) + + +@router.delete("/assignments/{product}/{task}") +async def delete_assignment(product: str, task: str) -> Any: + return await _coordinator_delete(f"/api/assignments/{urllib.parse.quote(product, safe='')}/{urllib.parse.quote(task, safe='')}") + + +# ── /model-registry ──────────────────────────────────────────────────────────── + +@router.get("/model-registry") +def list_model_registry() -> Any: + return _coordinator_get("/api/model-registry") + + +class ModelRegistryBody(BaseModel): + model_id: str + service_type: str + vram_mb: int + description: str = "" + hf_repo: str = "" + alias: str = "" + + +@router.post("/model-registry") +async def upsert_model_registry(body: ModelRegistryBody) -> Any: + return await _coordinator_post("/api/model-registry", body.model_dump()) + + +@router.delete("/model-registry/{model_id:path}") +async def delete_model_registry(model_id: str) -> Any: + return await _coordinator_delete(f"/api/model-registry/{urllib.parse.quote(model_id, safe='')}") diff --git a/app/models.py b/app/models.py index ad08ecb..26cfc90 100644 --- a/app/models.py +++ b/app/models.py @@ -124,11 +124,12 @@ _TAG_TO_INFO: dict[str, _TagInfo] = { "image-classification": {"adapter": None, "role": "vision", "service": "cf-vision"}, "zero-shot-image-classification": {"adapter": None, "role": "vision", "service": "cf-vision"}, "image-feature-extraction": {"adapter": None, "role": "embedding", "service": "cf-vision"}, - # Generative VLMs (image+text → text) — run under vllm, not cf-vision. - # cf-vision is a classifier/embedder service; generative VLMs like Qwen-VL, - # LLaVA, and InternVL are textgen models that happen to accept image inputs. - "image-text-to-text": {"adapter": None, "role": "vlm", "service": "vllm"}, - "visual-question-answering": {"adapter": None, "role": "vlm", "service": "vllm"}, + # Generative VLMs (image+text → text) — GGUF quants run via llama.cpp (cf-text). + # cf-vision is a classifier/embedder service; generative VLMs like Qwen2-VL + # and LLaVA accept image inputs but are textgen at the backend level. + # Full-precision HF-format VLMs would use vllm, but our fleet uses GGUF quants. + "image-text-to-text": {"adapter": None, "role": "vlm", "service": "cf-text"}, + "visual-question-answering": {"adapter": None, "role": "vlm", "service": "cf-text"}, # Image generation — cf-image (text → image; distinct from cf-vision image understanding) "text-to-image": {"adapter": None, "role": "image-gen", "service": "cf-image"}, # Embedding — cf-core shared embedding layer @@ -143,6 +144,11 @@ def set_models_dir(path: Path) -> None: _MODELS_DIR = path +def set_cf_text_models_dir(path: Path) -> None: + global _CF_TEXT_MODELS_DIR + _CF_TEXT_MODELS_DIR = path + + def set_queue_dir(path: Path) -> None: global _QUEUE_DIR _QUEUE_DIR = path diff --git a/pytest.ini b/pytest.ini index 4ecb1ad..6a37a04 100644 --- a/pytest.ini +++ b/pytest.ini @@ -3,3 +3,6 @@ testpaths = tests python_files = test_*.py python_classes = Test* python_functions = test_* +markers = + gpu: requires an idle GPU; excluded from default runs + slow: long-running test; excluded from default CI runs diff --git a/tests/test_cforch.py b/tests/test_cforch.py index 03484b8..6bb27a4 100644 --- a/tests/test_cforch.py +++ b/tests/test_cforch.py @@ -176,9 +176,14 @@ def test_models_merges_installed_generators(client, config_dir, tmp_path): # ── GET /run ─────────────────────────────────────────────────────────────────── def test_run_returns_409_when_already_running(client): - """If _BENCH_RUNNING is True, GET /run returns 409.""" + """If a benchmark subprocess is actively running, GET /run returns 409.""" + from unittest.mock import MagicMock from app import cforch as cforch_module + + mock_proc = MagicMock() + mock_proc.poll.return_value = None # process still alive cforch_module._BENCH_RUNNING = True + cforch_module._bench_proc = mock_proc r = client.get("/api/cforch/run") assert r.status_code == 409 @@ -212,16 +217,15 @@ def test_run_streams_progress_events(client, config_dir, tmp_path): "python_bin": "/usr/bin/python3", }) + mock_stdout = MagicMock() + mock_stdout.readline.side_effect = ["Running task 1\n", "Running task 2\n", ""] mock_proc = MagicMock() - mock_proc.stdout = iter(["Running task 1\n", "Running task 2\n"]) + mock_proc.stdout = mock_stdout mock_proc.returncode = 1 # non-zero so we don't need summary.json + mock_proc.wait = MagicMock() - def mock_wait(): - pass - - mock_proc.wait = mock_wait - - with patch("app.cforch._subprocess.Popen", return_value=mock_proc): + with patch("app.cforch._subprocess.Popen", return_value=mock_proc), \ + patch("app.cforch._select.select", return_value=([mock_stdout], [], [])): r = client.get("/api/cforch/run") assert r.status_code == 200 @@ -254,12 +258,15 @@ def test_run_emits_result_on_success(client, config_dir, tmp_path): "python_bin": "/usr/bin/python3", }) + mock_stdout = MagicMock() + mock_stdout.readline.side_effect = [""] # no output lines, immediate EOF mock_proc = MagicMock() - mock_proc.stdout = iter([]) + mock_proc.stdout = mock_stdout mock_proc.returncode = 0 mock_proc.wait = MagicMock() - with patch("app.cforch._subprocess.Popen", return_value=mock_proc): + with patch("app.cforch._subprocess.Popen", return_value=mock_proc), \ + patch("app.cforch._select.select", return_value=([mock_stdout], [], [])): r = client.get("/api/cforch/run") assert r.status_code == 200 diff --git a/tests/test_finetune.py b/tests/test_finetune.py index 29c59d6..6eb4410 100644 --- a/tests/test_finetune.py +++ b/tests/test_finetune.py @@ -321,6 +321,7 @@ def test_load_and_prepare_data_single_path_still_works(tmp_path): # ---- Integration test ---- +@pytest.mark.gpu def test_integration_finetune_on_example_data(tmp_path): """Fine-tune deberta-small on example data for 1 epoch. diff --git a/tests/test_models.py b/tests/test_models.py index 4af2340..07f03a9 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -17,6 +17,7 @@ def reset_models_globals(tmp_path): from app import models as models_module prev_models = models_module._MODELS_DIR + prev_cf_text = models_module._CF_TEXT_MODELS_DIR prev_queue = models_module._QUEUE_DIR prev_progress = dict(models_module._download_progress) @@ -26,12 +27,14 @@ def reset_models_globals(tmp_path): queue_dir.mkdir() models_module.set_models_dir(models_dir) + models_module.set_cf_text_models_dir(tmp_path / "cf-text-models") models_module.set_queue_dir(queue_dir) models_module._download_progress = {} yield models_module.set_models_dir(prev_models) + models_module.set_cf_text_models_dir(prev_cf_text) models_module.set_queue_dir(prev_queue) models_module._download_progress = prev_progress