From 0801da8df0a4d81fdbd0da32b3fbdb8a3ea722f6 Mon Sep 17 00:00:00 2001 From: Marcel Date: Thu, 21 May 2026 17:23:32 +0200 Subject: [PATCH] docs(ocr): explain why two metrics tests skip fresh_metrics fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sara's cycle-2 S2: clarify the latent (but not actual) cross-test state risk on the two metrics tests that hit the global REGISTRY instead of the per-test fresh_metrics fixture. Migrating them would actually break them — the /metrics endpoint is served by prometheus-fastapi-instrumentator which binds to the default REGISTRY at app-construction time, and the http_requests_total assertion only finds counters on that global registry. Both tests already assert response shape only (status code, content-type substring, body substrings), not numeric values, so the shared-registry caveat is documented for future readers rather than treated as a bug to fix. Co-Authored-By: Claude Opus 4.7 --- ocr-service/test_metrics.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/ocr-service/test_metrics.py b/ocr-service/test_metrics.py index d2bd9671..42afd275 100644 --- a/ocr-service/test_metrics.py +++ b/ocr-service/test_metrics.py @@ -61,7 +61,15 @@ def fresh_metrics(monkeypatch): @pytest.mark.asyncio async def test_metrics_endpoint_returns_200(): - """`GET /metrics` returns 200 with Prometheus exposition content.""" + """`GET /metrics` returns 200 with Prometheus exposition content. + + Uses the global REGISTRY by design — does NOT take the `fresh_metrics` fixture. + The `/metrics` endpoint is wired by `prometheus-fastapi-instrumentator`, which + binds to the default REGISTRY at app-construction time; swapping `main.metrics` + via the fixture would not redirect what `/metrics` exposes. This test only + asserts response shape (status code + content-type substring), not numeric + counter values, so cross-test state leakage cannot affect it. + """ with patch("main.kraken_engine.load_models"), \ patch("main.load_spell_checker"): async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: @@ -73,7 +81,15 @@ async def test_metrics_endpoint_returns_200(): @pytest.mark.asyncio async def test_metrics_includes_http_request_metrics_after_ocr_call(): - """After a request to /ocr, `/metrics` exposes auto-instrumented http_* metrics.""" + """After a request to /ocr, `/metrics` exposes auto-instrumented http_* metrics. + + Uses the global REGISTRY by design — does NOT take the `fresh_metrics` fixture. + The `http_requests_total` / `http_request_duration_seconds` metrics live on + the instrumentator's default REGISTRY (not on `main.metrics`), so a fresh + CollectorRegistry would never see them. This test only asserts response shape + (substring presence in the exposition body), not numeric counter values, so + cross-test state leakage cannot affect it. + """ mock_images = [Image.new("RGB", (100, 100))] mock_blocks = [{"pageNumber": 1, "x": 0.0, "y": 0.0, "width": 1.0, "height": 1.0, "polygon": None, "text": "hi", "words": []}]