test(api): cover frontend-required routes (#84)#108
Conversation
SHAURYAKSHARMA24
left a comment
There was a problem hiding this comment.
Thanks for the work on this, Hardik. The overall approach is solid and the added route coverage is useful, but I found two issues that need addressing before approval:
-
Make
assert_error_responseenforce the exact response contract.
ErrorResponse.model_validate()currently accepts missing optional fields and silently ignores unexpected fields. This means a response missingdetails, or even containing an unexpected field such astraceback, can pass. Please require exactlycode,message,details, andrequest_id, and add focused tests for missing fields, unexpected fields, and mismatched request IDs. -
Add a failure-path contract test for
POST /auth/logout.
The current logout test only covers successful/idempotent204responses. Issue #84 requires both a happy path and a failure path for each frontend-required route. Please add a deterministic ASGI integration test for a documented logout failure (429or500) and validate it with the strict shared error helper. If testing500, also confirm no exception or traceback information is exposed.
Please keep the changes narrowly scoped, reuse the existing fixtures, and include the relevant focused and non-benchmark backend test results when ready for re-review.
Summary
ErrorResponseenvelopes and request correlation on representative failuresWhy
The frontend depends on these routes across authentication, repositories, analysis, documentation, exports, and AI settings. This gives each route a real success path and an error contract check without changing runtime behavior.
Validation
python -m pytest --basetemp ..\\.pytest-tmp-84-audit-focused ...: 106 passed, 1 skippedpython -m pytest --ignore=tests/benchmark --basetemp ..\\.pytest-tmp-84-audit-nonbenchmark: 323 passed, 4 skippedpython -m pip check: no broken requirementsThe complete local suite has two unrelated benchmark assertions on Windows because a fixture checked in as LF is materialized as CRLF (97 bytes instead of the manifest's 96). No benchmark files are part of this PR.
Closes #84