ci: scoped 100% coverage gates + policy doc
This commit is contained in:
13
Makefile
13
Makefile
@@ -75,8 +75,17 @@ backend-test: ## Backend tests (pytest)
|
||||
cd $(BACKEND_DIR) && uv run pytest
|
||||
|
||||
.PHONY: backend-coverage
|
||||
backend-coverage: ## Backend tests with coverage gate (100% stmt + branch on covered src)
|
||||
cd $(BACKEND_DIR) && uv run pytest --cov=app --cov-branch --cov-report=term-missing --cov-report=xml:coverage.xml
|
||||
backend-coverage: ## Backend tests with coverage gate (scoped 100% stmt+branch on selected modules)
|
||||
# Policy: enforce 100% coverage only for the explicitly scoped, unit-testable backend modules.
|
||||
# Rationale: overall API/DB coverage is currently low; we will expand the scope as we add tests.
|
||||
cd $(BACKEND_DIR) && uv run pytest \
|
||||
--cov=app.core.error_handling \
|
||||
--cov=app.services.mentions \
|
||||
--cov-branch \
|
||||
--cov-report=term-missing \
|
||||
--cov-report=xml:coverage.xml \
|
||||
--cov-report=json:coverage.json \
|
||||
--cov-fail-under=100
|
||||
|
||||
.PHONY: frontend-test
|
||||
frontend-test: ## Frontend tests (vitest)
|
||||
|
||||
@@ -121,6 +121,11 @@ Open http://localhost:3000.
|
||||
|
||||
## Common commands
|
||||
|
||||
### Coverage policy
|
||||
|
||||
CI enforces a **scoped 100% coverage gate** (statements + branches) for a small set of unit-testable modules.
|
||||
See `docs/coverage-policy.md`.
|
||||
|
||||
From repo root:
|
||||
|
||||
```bash
|
||||
|
||||
@@ -4,7 +4,9 @@ from fastapi import FastAPI, HTTPException
|
||||
from fastapi.testclient import TestClient
|
||||
from pydantic import BaseModel, Field
|
||||
|
||||
from app.core.error_handling import REQUEST_ID_HEADER, install_error_handling
|
||||
from starlette.requests import Request
|
||||
|
||||
from app.core.error_handling import REQUEST_ID_HEADER, _error_payload, _get_request_id, install_error_handling
|
||||
|
||||
|
||||
def test_request_validation_error_includes_request_id():
|
||||
@@ -80,3 +82,38 @@ def test_response_validation_error_returns_500_with_request_id():
|
||||
assert body["detail"] == "Internal Server Error"
|
||||
assert isinstance(body.get("request_id"), str) and body["request_id"]
|
||||
assert resp.headers.get(REQUEST_ID_HEADER) == body["request_id"]
|
||||
|
||||
|
||||
def test_client_provided_request_id_is_preserved():
|
||||
app = FastAPI()
|
||||
install_error_handling(app)
|
||||
|
||||
@app.get("/needs-int")
|
||||
def needs_int(limit: int) -> dict[str, int]:
|
||||
return {"limit": limit}
|
||||
|
||||
client = TestClient(app)
|
||||
resp = client.get("/needs-int?limit=abc", headers={REQUEST_ID_HEADER: " req-123 "})
|
||||
|
||||
assert resp.status_code == 422
|
||||
body = resp.json()
|
||||
assert body["request_id"] == "req-123"
|
||||
assert resp.headers.get(REQUEST_ID_HEADER) == "req-123"
|
||||
|
||||
|
||||
def test_get_request_id_returns_none_for_missing_or_invalid_state() -> None:
|
||||
# Empty state
|
||||
req = Request({"type": "http", "headers": [], "state": {}})
|
||||
assert _get_request_id(req) is None
|
||||
|
||||
# Non-string request_id
|
||||
req = Request({"type": "http", "headers": [], "state": {"request_id": 123}})
|
||||
assert _get_request_id(req) is None
|
||||
|
||||
# Empty string request_id
|
||||
req = Request({"type": "http", "headers": [], "state": {"request_id": ""}})
|
||||
assert _get_request_id(req) is None
|
||||
|
||||
|
||||
def test_error_payload_omits_request_id_when_none() -> None:
|
||||
assert _error_payload(detail="x", request_id=None) == {"detail": "x"}
|
||||
|
||||
@@ -12,6 +12,21 @@ def test_matches_agent_mention_matches_first_name():
|
||||
assert matches_agent_mention(agent, {"cooper"}) is False
|
||||
|
||||
|
||||
def test_matches_agent_mention_no_mentions_is_false():
|
||||
agent = Agent(name="Alice")
|
||||
assert matches_agent_mention(agent, set()) is False
|
||||
|
||||
|
||||
def test_matches_agent_mention_empty_agent_name_is_false():
|
||||
agent = Agent(name=" ")
|
||||
assert matches_agent_mention(agent, {"alice"}) is False
|
||||
|
||||
|
||||
def test_matches_agent_mention_matches_full_normalized_name():
|
||||
agent = Agent(name="Alice Cooper")
|
||||
assert matches_agent_mention(agent, {"alice cooper"}) is True
|
||||
|
||||
|
||||
def test_matches_agent_mention_supports_reserved_lead_shortcut():
|
||||
lead = Agent(name="Riya", is_board_lead=True)
|
||||
other = Agent(name="Lead", is_board_lead=False)
|
||||
|
||||
87
backend/tests/test_request_id_middleware.py
Normal file
87
backend/tests/test_request_id_middleware.py
Normal file
@@ -0,0 +1,87 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Callable
|
||||
|
||||
import pytest
|
||||
|
||||
from app.core.error_handling import REQUEST_ID_HEADER, RequestIdMiddleware
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_request_id_middleware_passes_through_non_http_scope() -> None:
|
||||
called = False
|
||||
|
||||
async def app(scope, receive, send): # type: ignore[no-untyped-def]
|
||||
nonlocal called
|
||||
called = True
|
||||
|
||||
middleware = RequestIdMiddleware(app)
|
||||
|
||||
scope = {"type": "websocket", "headers": []}
|
||||
await middleware(scope, lambda: None, lambda message: None) # type: ignore[arg-type]
|
||||
|
||||
assert called is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_request_id_middleware_ignores_blank_client_header_and_generates_one() -> None:
|
||||
captured_request_id: str | None = None
|
||||
response_headers: list[tuple[bytes, bytes]] = []
|
||||
|
||||
async def app(scope, receive, send): # type: ignore[no-untyped-def]
|
||||
nonlocal captured_request_id
|
||||
captured_request_id = scope.get("state", {}).get("request_id")
|
||||
await send({"type": "http.response.start", "status": 200, "headers": []})
|
||||
await send({"type": "http.response.body", "body": b"ok"})
|
||||
|
||||
async def send(message): # type: ignore[no-untyped-def]
|
||||
if message["type"] == "http.response.start":
|
||||
response_headers.extend(list(message.get("headers") or []))
|
||||
|
||||
middleware = RequestIdMiddleware(app)
|
||||
|
||||
scope = {
|
||||
"type": "http",
|
||||
"headers": [(REQUEST_ID_HEADER.lower().encode("latin-1"), b" ")],
|
||||
}
|
||||
await middleware(scope, lambda: None, send)
|
||||
|
||||
assert isinstance(captured_request_id, str) and captured_request_id
|
||||
# Header should reflect the generated id, not the blank one.
|
||||
values = [v for k, v in response_headers if k.lower() == REQUEST_ID_HEADER.lower().encode("latin-1")]
|
||||
assert values == [captured_request_id.encode("latin-1")]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_request_id_middleware_does_not_duplicate_existing_header() -> None:
|
||||
sent_start = False
|
||||
start_headers: list[tuple[bytes, bytes]] | None = None
|
||||
|
||||
async def app(scope, receive, send): # type: ignore[no-untyped-def]
|
||||
# Simulate an app that already sets the request id header.
|
||||
await send(
|
||||
{
|
||||
"type": "http.response.start",
|
||||
"status": 200,
|
||||
"headers": [(REQUEST_ID_HEADER.lower().encode("latin-1"), b"already")],
|
||||
}
|
||||
)
|
||||
await send({"type": "http.response.body", "body": b"ok"})
|
||||
|
||||
async def send(message): # type: ignore[no-untyped-def]
|
||||
nonlocal sent_start, start_headers
|
||||
if message["type"] == "http.response.start":
|
||||
sent_start = True
|
||||
start_headers = list(message.get("headers") or [])
|
||||
|
||||
middleware = RequestIdMiddleware(app)
|
||||
|
||||
scope = {"type": "http", "headers": []}
|
||||
await middleware(scope, lambda: None, send)
|
||||
|
||||
assert sent_start is True
|
||||
assert start_headers is not None
|
||||
|
||||
# Ensure the middleware did not append a second copy.
|
||||
values = [v for k, v in start_headers if k.lower() == REQUEST_ID_HEADER.lower().encode("latin-1")]
|
||||
assert values == [b"already"]
|
||||
47
docs/coverage-policy.md
Normal file
47
docs/coverage-policy.md
Normal file
@@ -0,0 +1,47 @@
|
||||
# Coverage policy (CI gate)
|
||||
|
||||
## Why scoped coverage gates?
|
||||
|
||||
Today, overall repository coverage is low (especially for API routes and Next pages), but we still want CI to **enforce quality deterministically**.
|
||||
|
||||
So we start with a strict gate (100% statements + branches) on a **small, explicitly scoped** set of modules that are:
|
||||
|
||||
- unit-testable without external services
|
||||
- stable and high-signal for regressions
|
||||
|
||||
We then expand the gated scope as we add tests.
|
||||
|
||||
## Backend scope (100% required)
|
||||
|
||||
Enforced in `Makefile` target `backend-coverage`:
|
||||
|
||||
- `app.core.error_handling`
|
||||
- `app.services.mentions`
|
||||
|
||||
Command (CI):
|
||||
|
||||
```bash
|
||||
cd backend && uv run pytest \
|
||||
--cov=app.core.error_handling \
|
||||
--cov=app.services.mentions \
|
||||
--cov-branch \
|
||||
--cov-report=term-missing \
|
||||
--cov-report=xml:coverage.xml \
|
||||
--cov-report=json:coverage.json \
|
||||
--cov-fail-under=100
|
||||
```
|
||||
|
||||
## Frontend scope (100% required)
|
||||
|
||||
Enforced in `frontend/vitest.config.ts` coverage settings:
|
||||
|
||||
- include: `src/lib/backoff.ts`
|
||||
- thresholds: 100% for lines/statements/functions/branches
|
||||
|
||||
This is intentionally limited to a single pure utility module first. As we add more unit tests in `src/lib/**` and React Testing Library component tests for `src/app/**` + `src/components/**`, we should expand the include list and keep thresholds strict.
|
||||
|
||||
## How to expand the gate
|
||||
|
||||
- Add tests for the next-highest-signal modules.
|
||||
- Add them to the gated scope (backend `--cov=` list; frontend `coverage.include`).
|
||||
- Keep the threshold at 100% for anything included in the gate.
|
||||
@@ -20,6 +20,23 @@ describe("createExponentialBackoff", () => {
|
||||
expect(backoff.nextDelayMs()).toBe(250); // capped
|
||||
});
|
||||
|
||||
it("clamps invalid numeric options and treats negative jitter as zero", () => {
|
||||
vi.spyOn(Math, "random").mockReturnValue(0.9999);
|
||||
|
||||
// baseMs: NaN should clamp to min (50)
|
||||
// maxMs: Infinity should clamp to min (= baseMs)
|
||||
// jitter: negative -> treated as 0 (no extra delay)
|
||||
const backoff = createExponentialBackoff({
|
||||
baseMs: Number.NaN,
|
||||
maxMs: Number.POSITIVE_INFINITY,
|
||||
jitter: -1,
|
||||
});
|
||||
|
||||
// With maxMs clamped to baseMs, delay will always be baseMs
|
||||
expect(backoff.nextDelayMs()).toBe(50);
|
||||
expect(backoff.nextDelayMs()).toBe(50);
|
||||
});
|
||||
|
||||
it("reset brings attempt back to zero", () => {
|
||||
vi.spyOn(Math, "random").mockReturnValue(0);
|
||||
|
||||
@@ -30,4 +47,15 @@ describe("createExponentialBackoff", () => {
|
||||
backoff.reset();
|
||||
expect(backoff.attempt()).toBe(0);
|
||||
});
|
||||
|
||||
it("uses defaults when options are omitted", () => {
|
||||
vi.spyOn(Math, "random").mockReturnValue(0);
|
||||
|
||||
const backoff = createExponentialBackoff();
|
||||
expect(backoff.attempt()).toBe(0);
|
||||
|
||||
// Default baseMs is 1000 (clamped within bounds), jitter default is 0.2.
|
||||
// With Math.random=0, delay should be the normalized base (1000).
|
||||
expect(backoff.nextDelayMs()).toBe(1000);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -9,8 +9,16 @@ export default defineConfig({
|
||||
provider: "v8",
|
||||
reporter: ["text", "lcov"],
|
||||
reportsDirectory: "./coverage",
|
||||
include: ["src/**/*.{ts,tsx}"],
|
||||
// Policy (scoped gate): require 100% coverage on *explicitly listed* unit-testable modules first.
|
||||
// We'll expand this include list as we add more unit/component tests.
|
||||
include: ["src/lib/backoff.ts"],
|
||||
exclude: ["**/*.d.ts", "src/**/__generated__/**", "src/**/generated/**"],
|
||||
thresholds: {
|
||||
lines: 100,
|
||||
statements: 100,
|
||||
functions: 100,
|
||||
branches: 100,
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user