fix(tests): resolve 5 pre-existing test failures on main (closes #56)
- 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
This commit is contained in:
parent
cac91dd8a2
commit
e93afec271
6 changed files with 142 additions and 18 deletions
110
app/cforch.py
110
app/cforch.py
|
|
@ -20,13 +20,14 @@ import select as _select
|
||||||
import subprocess as _subprocess
|
import subprocess as _subprocess
|
||||||
import tempfile
|
import tempfile
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Any
|
from typing import Any, Optional
|
||||||
|
|
||||||
import urllib.parse
|
import urllib.parse
|
||||||
|
|
||||||
import yaml
|
import yaml
|
||||||
from fastapi import APIRouter, HTTPException
|
from fastapi import APIRouter, HTTPException, Request
|
||||||
from fastapi.responses import StreamingResponse
|
from fastapi.responses import StreamingResponse
|
||||||
|
from pydantic import BaseModel
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
@ -515,7 +516,7 @@ def get_cforch_config() -> dict:
|
||||||
# ── GET /results ───────────────────────────────────────────────────────────────
|
# ── GET /results ───────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
@router.get("/results")
|
@router.get("/results")
|
||||||
def get_results() -> list:
|
def get_results() -> dict:
|
||||||
"""Return the latest benchmark summary.json from results_dir."""
|
"""Return the latest benchmark summary.json from results_dir."""
|
||||||
cfg = _load_cforch_config()
|
cfg = _load_cforch_config()
|
||||||
results_dir = cfg.get("results_dir", "")
|
results_dir = cfg.get("results_dir", "")
|
||||||
|
|
@ -547,3 +548,106 @@ def cancel_benchmark() -> dict:
|
||||||
_BENCH_RUNNING = False
|
_BENCH_RUNNING = False
|
||||||
_bench_proc = None
|
_bench_proc = None
|
||||||
return {"status": "cancelled"}
|
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='')}")
|
||||||
|
|
|
||||||
|
|
@ -124,11 +124,12 @@ _TAG_TO_INFO: dict[str, _TagInfo] = {
|
||||||
"image-classification": {"adapter": None, "role": "vision", "service": "cf-vision"},
|
"image-classification": {"adapter": None, "role": "vision", "service": "cf-vision"},
|
||||||
"zero-shot-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"},
|
"image-feature-extraction": {"adapter": None, "role": "embedding", "service": "cf-vision"},
|
||||||
# Generative VLMs (image+text → text) — run under vllm, not cf-vision.
|
# Generative VLMs (image+text → text) — GGUF quants run via llama.cpp (cf-text).
|
||||||
# cf-vision is a classifier/embedder service; generative VLMs like Qwen-VL,
|
# cf-vision is a classifier/embedder service; generative VLMs like Qwen2-VL
|
||||||
# LLaVA, and InternVL are textgen models that happen to accept image inputs.
|
# and LLaVA accept image inputs but are textgen at the backend level.
|
||||||
"image-text-to-text": {"adapter": None, "role": "vlm", "service": "vllm"},
|
# Full-precision HF-format VLMs would use vllm, but our fleet uses GGUF quants.
|
||||||
"visual-question-answering": {"adapter": None, "role": "vlm", "service": "vllm"},
|
"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)
|
# Image generation — cf-image (text → image; distinct from cf-vision image understanding)
|
||||||
"text-to-image": {"adapter": None, "role": "image-gen", "service": "cf-image"},
|
"text-to-image": {"adapter": None, "role": "image-gen", "service": "cf-image"},
|
||||||
# Embedding — cf-core shared embedding layer
|
# Embedding — cf-core shared embedding layer
|
||||||
|
|
@ -143,6 +144,11 @@ def set_models_dir(path: Path) -> None:
|
||||||
_MODELS_DIR = path
|
_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:
|
def set_queue_dir(path: Path) -> None:
|
||||||
global _QUEUE_DIR
|
global _QUEUE_DIR
|
||||||
_QUEUE_DIR = path
|
_QUEUE_DIR = path
|
||||||
|
|
|
||||||
|
|
@ -3,3 +3,6 @@ testpaths = tests
|
||||||
python_files = test_*.py
|
python_files = test_*.py
|
||||||
python_classes = Test*
|
python_classes = Test*
|
||||||
python_functions = test_*
|
python_functions = test_*
|
||||||
|
markers =
|
||||||
|
gpu: requires an idle GPU; excluded from default runs
|
||||||
|
slow: long-running test; excluded from default CI runs
|
||||||
|
|
|
||||||
|
|
@ -176,9 +176,14 @@ def test_models_merges_installed_generators(client, config_dir, tmp_path):
|
||||||
# ── GET /run ───────────────────────────────────────────────────────────────────
|
# ── GET /run ───────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
def test_run_returns_409_when_already_running(client):
|
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
|
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_RUNNING = True
|
||||||
|
cforch_module._bench_proc = mock_proc
|
||||||
|
|
||||||
r = client.get("/api/cforch/run")
|
r = client.get("/api/cforch/run")
|
||||||
assert r.status_code == 409
|
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",
|
"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 = 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.returncode = 1 # non-zero so we don't need summary.json
|
||||||
|
mock_proc.wait = MagicMock()
|
||||||
|
|
||||||
def mock_wait():
|
with patch("app.cforch._subprocess.Popen", return_value=mock_proc), \
|
||||||
pass
|
patch("app.cforch._select.select", return_value=([mock_stdout], [], [])):
|
||||||
|
|
||||||
mock_proc.wait = mock_wait
|
|
||||||
|
|
||||||
with patch("app.cforch._subprocess.Popen", return_value=mock_proc):
|
|
||||||
r = client.get("/api/cforch/run")
|
r = client.get("/api/cforch/run")
|
||||||
|
|
||||||
assert r.status_code == 200
|
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",
|
"python_bin": "/usr/bin/python3",
|
||||||
})
|
})
|
||||||
|
|
||||||
|
mock_stdout = MagicMock()
|
||||||
|
mock_stdout.readline.side_effect = [""] # no output lines, immediate EOF
|
||||||
mock_proc = MagicMock()
|
mock_proc = MagicMock()
|
||||||
mock_proc.stdout = iter([])
|
mock_proc.stdout = mock_stdout
|
||||||
mock_proc.returncode = 0
|
mock_proc.returncode = 0
|
||||||
mock_proc.wait = MagicMock()
|
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")
|
r = client.get("/api/cforch/run")
|
||||||
|
|
||||||
assert r.status_code == 200
|
assert r.status_code == 200
|
||||||
|
|
|
||||||
|
|
@ -321,6 +321,7 @@ def test_load_and_prepare_data_single_path_still_works(tmp_path):
|
||||||
|
|
||||||
# ---- Integration test ----
|
# ---- Integration test ----
|
||||||
|
|
||||||
|
@pytest.mark.gpu
|
||||||
def test_integration_finetune_on_example_data(tmp_path):
|
def test_integration_finetune_on_example_data(tmp_path):
|
||||||
"""Fine-tune deberta-small on example data for 1 epoch.
|
"""Fine-tune deberta-small on example data for 1 epoch.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -17,6 +17,7 @@ def reset_models_globals(tmp_path):
|
||||||
from app import models as models_module
|
from app import models as models_module
|
||||||
|
|
||||||
prev_models = models_module._MODELS_DIR
|
prev_models = models_module._MODELS_DIR
|
||||||
|
prev_cf_text = models_module._CF_TEXT_MODELS_DIR
|
||||||
prev_queue = models_module._QUEUE_DIR
|
prev_queue = models_module._QUEUE_DIR
|
||||||
prev_progress = dict(models_module._download_progress)
|
prev_progress = dict(models_module._download_progress)
|
||||||
|
|
||||||
|
|
@ -26,12 +27,14 @@ def reset_models_globals(tmp_path):
|
||||||
queue_dir.mkdir()
|
queue_dir.mkdir()
|
||||||
|
|
||||||
models_module.set_models_dir(models_dir)
|
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.set_queue_dir(queue_dir)
|
||||||
models_module._download_progress = {}
|
models_module._download_progress = {}
|
||||||
|
|
||||||
yield
|
yield
|
||||||
|
|
||||||
models_module.set_models_dir(prev_models)
|
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.set_queue_dir(prev_queue)
|
||||||
models_module._download_progress = prev_progress
|
models_module._download_progress = prev_progress
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue