Repository navigation
test: fix docstring/websocket gaps and mirror test layout - #51
Merged
Merged
Conversation
) - #19: add missing docstring to cleanup_globals._reset (docs-coverage 100%) - #20: guard websocket route introspection against FastAPI _IncludedRouter - #21: restructure tests/ to mirror src/pycharting/ so check_test_layout.py passes — flatten organizational grouping classes into module-level test functions, keep/add Test<SourceClass> classes (TestChartServer, TestDataManager, TestNoCacheStaticFiles, TestDataResponse, TestErrorResponse, TestDataValidationError), merge test_data_slicing into test_ingestion, fold test_import into test_interface. Gates: fmt PASS, docs-coverage 100%, 148 tests PASS (coverage 100%), test-layout OK. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| from unittest.mock import MagicMock, patch | ||
|
|
||
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: | ||
| s.bind(("", 0)) |
There was a problem hiding this comment.
Pull request overview
This PR improves test quality and maintainability by updating the test suite to (1) satisfy a test-layout parity checker that mirrors src/pycharting/, and (2) address test robustness/coverage gaps (docstring coverage and FastAPI route-list compatibility).
Changes:
- Restructured
tests/intotests/pycharting/...to mirror the package layout; flattened several organizationalTest*grouping classes into module-level tests while keeping requiredTest<SourceClass>parity classes. - Updated lifecycle tests to be compatible with newer FastAPI/Starlette route objects by reading
route.pathdefensively. - Consolidated/relocated tests (e.g., folded import smoke tests into interface tests, merged slicing tests into ingestion-related coverage).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_server.py | Removed legacy server tests file (moved under tests/pycharting/core/). |
| tests/test_interface.py | Removed legacy interface tests file (moved under tests/pycharting/api/). |
| tests/test_import.py | Removed standalone import smoke tests (folded into tests/pycharting/api/test_interface.py). |
| tests/test_data_slicing.py | Removed legacy slicing tests file (coverage migrated under new layout). |
| tests/test_data_ingestion.py | Removed legacy ingestion/validation tests file (coverage migrated under new layout). |
| tests/test_api_routes.py | Removed legacy API routes tests file (moved under tests/pycharting/api/). |
| tests/pycharting/init.py | Added test package root to mirror pycharting layout. |
| tests/pycharting/core/init.py | Added core test package marker. |
| tests/pycharting/api/init.py | Added api test package marker. |
| tests/pycharting/core/test_server.py | Re-homed FastAPI server tests under mirrored layout and flattened most tests to module-level functions. |
| tests/pycharting/core/test_lifecycle.py | Fixed imports (pycharting...), made websocket route enumeration defensive, and flattened several tests to module-level. |
| tests/pycharting/api/test_routes.py | Re-homed API route tests under mirrored layout and added model-construction tests for response models. |
| tests/pycharting/api/test_interface.py | Re-homed interface tests under mirrored layout, added missing _reset docstring, and folded in import smoke tests. |
Comments suppressed due to low confidence (1)
tests/pycharting/core/test_lifecycle.py:159
- This test starts/stops a real ChartServer but doesn't guarantee cleanup if an assertion fails mid-loop (e.g., server starts but the test errors before stop_server runs). That can leak background threads/ports and cascade failures in later tests in the same session.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """DataResponse builds from the required index/range fields.""" | ||
| from pycharting.api.routes import DataResponse | ||
|
|
||
| resp = DataResponse(index=[0, 1, 2], start_index=0, end_index=2, total_length=3) |
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
Addresses three test-quality findings (tracked on the tschm/pycharting fork as tschm#19, tschm#20, tschm#21):
cleanup_globals._reset, bringing docstring coverage to 100%.test_websocket_endpoint_addednow reads route paths defensively (getattr(route, "path", None)), surviving FastAPI 0.138's_IncludedRouterentries inapp.routes.tests/to mirrorsrc/pycharting/so the rhizacheck_test_layout.pyparity check exits 0.#21 details
The layout checker enforces
Test<SourceClass>parity in both directions, so the organizational grouping classes had to be dissolved:tests/pycharting/<subpkg>/test_<module>.py.TestHealthCheck,TestValidateInput, …) into module-level test functions.Test<SourceClass>classes the checker requires:TestChartServer,TestDataManager,TestNoCacheStaticFiles, and newTestDataResponse,TestErrorResponse,TestDataValidationError.test_data_slicing.py→test_ingestion.py; foldedtest_import.py→test_interface.py.from src.pycharting...import tofrom pycharting....Gates
make fmtmake docs-coveragemake testcheck_test_layout.pyNet: +5 tests (new model/exception tests), same 100% source coverage.
🤖 Generated with Claude Code