Skip to content

Commit 27989d6

Browse files
committed
ci: fix PR #35 — fastapi dep + Transport._send_batch typo + coverage padding
PR #35 (release/0.7.6) failed all four CI jobs (test 3.10/3.11/3.12, coverage, codecov/patch) on the same root cause + one latent bug masked by it. This commit lands the fixes plus the last-mile tests that bring coverage above the 82% threshold. CI failure root --------------- * tests/test_integrations_fastapi.py does from fastapi import ... at module top-level. CI installs only pip install -e '.[dev]', and fastapi was declared as an *optional* [fastapi] extra, NOT in [dev]. Pytest collection aborted with ModuleNotFoundError: No module named 'fastapi' → all 4 jobs red. * Fix: add fastapi>=0.100,<1.0 to [dev]. Same precedent as langchain-core (already in [dev] for the same import-time contract: nullrun.instrumentation.langgraph is eager-imported from nullrun.decorators at collection time, so the test extras must cover the import chain). Latent bug surfaced by the first fix ------------------------------------ The same PR refactored Transport._send_batch_with_retry_info to route the /track/batch body through _signed_request_body for canonical-JSON serialization (matching /gate and /execute). The two sibling call sites use the module-level helper _signed_request_body (no self.); this one used self._signed_request_body by typo. Result: AttributeError on every batch flush, breaking 15 existing tests across test_transport.py / test_track_batch_retry.py / test_integration_contract.py / test_signal_safety.py. As long as the fastapi collection error aborted pytest, this was hidden. Fixed to _signed_request_body(...) with a docstring noting why it is module-level and what the bug looked like. Coverage padding (codecov/patch was failing on this too) -------------------------------------------------------- Total coverage on the failing CI run was 81.98% — 0.02pp under the fail-under=82 gate. After the two fixes above it would have recovered to ~82.0% on the dot, so I added minimal tests for the cheapest-to-cover gaps: * tests/test_breaker_main.py (new) — covers the 5 statements in nullrun.breaker.__main__.main() (0% → 100%). The module exists so python -m nullrun.breaker exits cleanly instead of failing with No module named nullrun.breaker.__main__; the previous fix-mechanism was return 0 after a print, but no test was exercising it. * tests/test_status.py — extends TestSummary with seven scenarios covering each conditional branch of NullRunStatus.summary() (organization_id, workflow_id, workflow_state != Normal, backend_reachable=False, ws_connected=False, recent_errors). status.py jumps 84.52% → 98.81%. * tests/test_integrations_fastapi.py — four tests on _build_headers covering non-numeric, zero, negative, and resume_after (the WorkflowPausedException code path). integrations/fastapi.py jumps 90.22% → 94.57%. After all three: TOTAL 81.98% → 82.46%, comfortably above the gate. Verification ------------ * Local pytest: 997 passed, 13 skipped, 0 failed (Windows / Python 3.14.2, 8m47s — same env the original commit was validated in). * python -m coverage report — 82.46%, no fail-under complaint.
1 parent 992fdc0 commit 27989d6

5 files changed

Lines changed: 192 additions & 1 deletion

File tree

‎pyproject.toml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,15 @@ autogen = [
9292
"autogen-agentchat>=0.4,<1.0",
9393
"autogen-ext[openai]>=0.4,<1.0",
9494
]
95+
# Server-framework integrations. Each one pulls the framework so the
96+
# corresponding ``nullrun.integrations.<framework>`` module can be
97+
# imported. ``nullrun.integrations.__init__`` does NOT eager-import
98+
# these (the submodules are loaded lazily on first ``from
99+
# nullrun.integrations import <framework>``), so users who don't use
100+
# a given framework don't pay its install cost.
101+
fastapi = [
102+
"fastapi>=0.100,<1.0",
103+
]
95104
all = [
96105
"openai>=1.0,<2.0",
97106
"anthropic>=0.20,<1.0",
@@ -124,6 +133,13 @@ dev = [
124133
# the import succeed; the `langgraph` and `langchain` extras pull
125134
# in heavier stacks that the unit tests don't need.
126135
"langchain-core>=0.3,<1.0",
136+
# `tests/test_integrations_fastapi.py` does `from fastapi import ...`
137+
# at module top-level, so pytest collection aborts the entire suite
138+
# with ModuleNotFoundError if FastAPI isn't installed. Same import-
139+
# time contract as `langchain-core` above; pin to the same lower
140+
# bound as the `[fastapi]` extra so CI and end users agree on the
141+
# minimum. `httpx` (already a core dep) covers `TestClient`.
142+
"fastapi>=0.100,<1.0",
127143
]
128144

129145
[project.urls]

‎src/nullrun/transport.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1050,7 +1050,13 @@ def _send_batch_with_retry_info(self, batch: list[dict[str, Any]]) -> "SendResul
10501050
# (it hashes the bytes either way), but consistent serialization
10511051
# means future audits / contract tests don't have to special-case
10521052
# this endpoint.
1053-
body = self._signed_request_body({"events": batch})
1053+
# NOTE: _signed_request_body is a MODULE-LEVEL helper, not a
1054+
# method on Transport. The two siblings in this file
1055+
# (``execute`` and ``check``) call it without ``self.``; calling
1056+
# ``self._signed_request_body`` here raised AttributeError on
1057+
# every batch flush and broke 15 tests across test_transport.py
1058+
# / test_track_batch_retry.py / test_integration_contract.py.
1059+
body = _signed_request_body({"events": batch})
10541060
self._add_hmac_headers(headers, body)
10551061

10561062
# Inject trace context for distributed tracing (W3C Trace Context)

‎tests/test_breaker_main.py‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
"""Coverage padding for ``nullrun.breaker.__main__``.
2+
3+
The module exists so ``python -m nullrun.breaker`` exits cleanly
4+
instead of failing with ``No module named nullrun.breaker.__main__``.
5+
Containerized deployments that previously relied on the broken
6+
entrypoint should call ``nullrun-doctor`` (see
7+
``nullrun.toolbox.diagnostics``) for runtime checks.
8+
9+
Pinned by ``pyproject.toml::[tool.coverage.report].fail_under = 82`` —
10+
without this test, the five statements in ``main()`` stay at 0% and
11+
the suite trips the threshold by a hair.
12+
"""
13+
from __future__ import annotations
14+
15+
import io
16+
17+
import pytest
18+
19+
from nullrun.breaker.__main__ import main
20+
21+
22+
def test_main_returns_zero_and_writes_helpful_message(capsys: pytest.CaptureFixture[str]) -> None:
23+
"""``main()`` is informational, not an error: return code 0, the
24+
message goes to stderr (so it doesn't pollute the consumer's
25+
stdout pipe)."""
26+
rc = main()
27+
captured = capsys.readouterr()
28+
assert rc == 0
29+
# Message goes to stderr so a stdout pipe stays clean.
30+
assert captured.out == ""
31+
assert "nullrun-doctor" in captured.err
32+
assert "library module" in captured.err
33+
34+
35+
def test_main_runs_under_dunder_main(monkeypatch: pytest.MonkeyPatch) -> None:
36+
"""Smoke: ``python -m nullrun.breaker`` path — exercise the
37+
``if __name__ == "__main__":`` guard via ``runpy`` so the
38+
``SystemExit`` branch is hit."""
39+
import runpy
40+
41+
with pytest.raises(SystemExit) as info:
42+
runpy.run_module("nullrun.breaker.__main__", run_name="__main__")
43+
assert info.value.code == 0

‎tests/test_integrations_fastapi.py‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
"""
1111
from __future__ import annotations
1212

13+
from typing import Any
14+
1315
import pytest
1416
from fastapi import FastAPI, Request
1517
from fastapi.testclient import TestClient
@@ -287,3 +289,43 @@ def trigger():
287289
# Single 429, not a double-handler crash.
288290
assert resp.status_code == 429
289291
assert resp.json()["error_code"] == "NR-B004"
292+
293+
294+
# ---------------------------------------------------------------------------
295+
# _build_headers edge cases — Retry-After handling
296+
# ---------------------------------------------------------------------------
297+
class _AttrBag:
298+
"""Minimal stand-in for a NullRun exception — only the attrs
299+
that ``_build_headers`` reads (``retry_after`` / ``resume_after``)
300+
matter."""
301+
302+
def __init__(self, **kwargs: Any) -> None:
303+
for k, v in kwargs.items():
304+
setattr(self, k, v)
305+
306+
307+
def test_build_headers_returns_empty_when_no_retry_hint():
308+
"""No ``retry_after`` / ``resume_after`` → no Retry-After header."""
309+
assert nr_fastapi._build_headers(_AttrBag()) == {}
310+
311+
312+
def test_build_headers_returns_empty_when_retry_after_non_numeric():
313+
"""A non-numeric ``retry_after`` must NOT raise; it just yields
314+
no header. The exception class is opaque to the renderer, so a
315+
typo'd string field shouldn't break the response."""
316+
assert nr_fastapi._build_headers(_AttrBag(retry_after="soon")) == {}
317+
318+
319+
def test_build_headers_returns_empty_when_retry_after_is_zero():
320+
"""Zero or negative ``retry_after`` is not meaningful for
321+
Retry-After (RFC 9110 allows zero but a real client would
322+
spin; the renderer drops it to avoid hot-looping)."""
323+
assert nr_fastapi._build_headers(_AttrBag(retry_after=0)) == {}
324+
assert nr_fastapi._build_headers(_AttrBag(retry_after=-5)) == {}
325+
326+
327+
def test_build_headers_falls_back_to_resume_after():
328+
"""``WorkflowPausedException`` uses ``resume_after`` instead of
329+
``retry_after`` — the renderer normalizes on the canonical
330+
HTTP field name."""
331+
assert nr_fastapi._build_headers(_AttrBag(resume_after=42)) == {"Retry-After": "42"}

‎tests/test_status.py‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,90 @@ def test_ok_summary(self):
248248
assert "ok" in out
249249
assert "nr_live_te" in out
250250

251+
def test_summary_with_organization_and_workflow(self):
252+
# Covers the ``if self.organization_id`` and
253+
# ``if self.workflow_id`` branches of summary().
254+
rt = _make_runtime()
255+
rt.organization_id = "org_abcdef1234567890"
256+
rt.workflow_id = "wf_xyzzy1234567890"
257+
s = nullrun.status()
258+
out = s.summary()
259+
assert "org=org_abcd" in out
260+
assert "wf=wf_xyzzy" in out
261+
262+
def test_summary_includes_workflow_state_when_not_normal(self):
263+
# Branch: ``self.workflow_state and .state != "Normal"``.
264+
rt = _make_runtime()
265+
rt.workflow_id = "wf-test-1"
266+
rt._set_remote_state(
267+
"wf-test-1",
268+
{"state": "Killed", "version": 5, "reason": "manual kill"},
269+
)
270+
s = nullrun.status()
271+
out = s.summary()
272+
assert "wf_state=Killed" in out
273+
274+
def test_summary_omits_normal_workflow_state(self):
275+
# Sanity: a Normal workflow state should NOT appear in summary.
276+
rt = _make_runtime()
277+
rt.workflow_id = "wf-test-1"
278+
rt._set_remote_state(
279+
"wf-test-1",
280+
{"state": "Normal", "version": 1, "reason": None},
281+
)
282+
s = nullrun.status()
283+
out = s.summary()
284+
assert "wf_state=" not in out
285+
286+
def test_summary_includes_backend_unreachable(self):
287+
# Branch: ``self.backend_reachable is False``.
288+
# ``backend_reachable`` is a local in ``status()``, not a stored
289+
# attribute on the runtime — construct the snapshot directly.
290+
s = NullRunStatus(
291+
state="degraded",
292+
api_key_valid=True,
293+
api_key_prefix="nr_live_te",
294+
organization_id=None,
295+
workflow_id=None,
296+
api_url="https://api.nullrun.io",
297+
backend_reachable=False,
298+
ws_connected=None,
299+
workflow_state=None,
300+
recent_errors=[],
301+
)
302+
assert "backend=unreachable" in s.summary()
303+
304+
def test_summary_includes_ws_disconnected(self):
305+
# Branch: ``self.ws_connected is False``. Same reasoning as above.
306+
s = NullRunStatus(
307+
state="degraded",
308+
api_key_valid=True,
309+
api_key_prefix="nr_live_te",
310+
organization_id=None,
311+
workflow_id=None,
312+
api_url="https://api.nullrun.io",
313+
backend_reachable=None,
314+
ws_connected=False,
315+
workflow_state=None,
316+
recent_errors=[],
317+
)
318+
assert "ws=False" in s.summary()
319+
320+
def test_summary_includes_recent_errors_count(self):
321+
# Branch: ``if self.recent_errors``.
322+
rt = _make_runtime()
323+
from nullrun.observability.error_hooks import ErrorContext
324+
from nullrun.breaker.exceptions import NullRunError
325+
326+
for i in range(3):
327+
rt._emit_sdk_error(
328+
NullRunError(f"err-{i}", error_code="NR-X000"),
329+
stage="init",
330+
)
331+
s = nullrun.status()
332+
out = s.summary()
333+
assert "errors=3" in out
334+
251335

252336
# ---------------------------------------------------------------------------
253337
# 7. Public API surface

0 commit comments

Comments
 (0)