test: add Test classes for the seven response models (#65) - #77
Open
tschm wants to merge 1 commit into
Open
Conversation
Rhiza's check_test_layout requires every source class to have a mirrored
Test<Name>. Seven response models had none:
interface.py PlotResult, ServerStatus (TypedDict)
routes.py InitDataResponse, SessionInfo,
SessionListResponse, DeleteSessionResponse,
StatusResponse (BaseModel)
Chose real mirroring over the [tool.check_test_layout] enforce=false
opt-out: the opt-out is all-or-nothing, so it would also stop catching a
genuinely missing test_<module>.py in future. The new tests pin contracts
worth pinning — the TypedDict key sets are checked against what plot() and
get_server_status() actually return, and two models are validated against
live endpoint payloads.
Follows the existing TestDataResponse/TestErrorResponse style.
Closes alihaskar#65
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds missing mirrored Test* classes for seven API response models so Rhiza’s check_test_layout gate passes while also asserting key aspects of the models’ runtime contracts.
Changes:
- Added
TestInitDataResponse,TestSessionInfo,TestSessionListResponse,TestDeleteSessionResponse, andTestStatusResponseto coverBaseModelresponse schemas and validate against live endpoint payloads. - Added
TestPlotResultandTestServerStatusto coverTypedDictresponse contracts and compare declared keys vs real function outputs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/pycharting/api/test_routes.py | Adds mirrored tests for five BaseModel response models in api.routes, including endpoint-payload validation. |
| tests/pycharting/api/test_interface.py | Adds mirrored tests for two TypedDict response models in api.interface, checking declared keys and real return shapes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+553
to
+559
| def test_every_key_is_optional(self): | ||
| """``total=False`` means success and failure shapes share one type.""" | ||
| from pycharting.api.interface import PlotResult | ||
|
|
||
| assert PlotResult.__total__ is False | ||
| assert PlotResult.__required_keys__ == frozenset() | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rhiza's
check_test_layoutrequires every sourceclass Ato have a mirroredTestA. Seven response models had none, failing the gate.api/interface.pyPlotResult,ServerStatusTypedDictapi/routes.pyInitDataResponse,SessionInfo,SessionListResponse,DeleteSessionResponse,StatusResponseBaseModelWhy mirroring rather than the opt-out
The issue offered a documented
[tool.check_test_layout] enforce = falseopt-out as the lighter option. I went the other way after reading the checker:enforceis all-or-nothing — there is no per-class exemption, onlyenforce,reasonandexempt_dirs. Disabling it would also stop catching a genuinely missingtest_<module>.pylater, which is the part of the gate that currently passes and is worth keeping.So the gate stays fully enforced.
The tests earn their place
They aren't schema tautologies — they pin contracts that can silently drift:
PlotResult/ServerStatuskey sets are asserted against whatplot()andget_server_status()actually return, on both the success and validation-failure paths. A field added to one and not the other now fails.PlotResult.__total__ is FalsevsServerStatus.__total__ is Truedocuments why one type covers both success and failure shapes.SessionInfoandStatusResponseare validated against live/api/sessionsand/api/statuspayloads.BaseModeltests cover construction, required-field validation, nested coercion and the empty-listing case.Style follows the existing
TestDataResponse/TestErrorResponseclasses.Verification
Tests-only change — no
src/or behaviour touched.Closes #65