test: mirror data/ingestion tests + fix layout parity (#55)#58
Open
tschm wants to merge 5 commits into
Open
Conversation
…haskar#56, alihaskar#50) - mkdocs.yml: replace placeholder site_description with the real description from pyproject.toml (alihaskar#56) - interface.py: split the broad except in plot() so DataValidationError is reported with stage="validation" (fixable input) separately from server/runtime failures reported with stage="server" (alihaskar#50) - test_interface.py: assert the validation stage is reported Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Splitting the broad except in plot() left the generic (server/runtime) except branch unexercised once validation errors took their own branch, dropping interface.py coverage below 100%. Add a test that forces a ChartServer startup failure and asserts stage="server". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The migration to the mirrored tests/<pkg>/** layout never created the data package mirror, so check_test_layout.py failed with a missing tests/pycharting/data/test_ingestion.py. Two problems compounded it: - The flat tests/test_data_ingestion.py and tests/test_data_slicing.py were deleted in the migration without a mirrored replacement. Restore them, merged into tests/pycharting/data/test_ingestion.py with the class layout the checker enforces: TestDataValidationError and TestDataManager (get_chunk slicing folded in), plus module-level functions for validate_input behaviour. - .gitignore had an unanchored `data/` rule (for local sample data) that silently ignored tests/pycharting/data/. Anchor it to `/data/`. check_test_layout.py now exits 0 and data/ingestion.py stays at 100%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Restores test-layout parity for pycharting.data by reintroducing a mirrored ingestion test suite under tests/pycharting/data/ and fixing a .gitignore rule that previously prevented that directory from being committed.
Changes:
- Add
tests/pycharting/data/test_ingestion.pyto mirror and coversrc/pycharting/data/ingestion.py(validation +DataManager.get_chunk). - Add
tests/pycharting/data/__init__.pyto complete the mirrored test package structure. - Anchor the
.gitignoredata/rule to/data/so it no longer matchestests/pycharting/data/.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/pycharting/data/test_ingestion.py | Adds the missing mirrored ingestion/validation/slicing test suite for data/ingestion.py. |
| tests/pycharting/data/init.py | Creates the mirrored tests.pycharting.data test package. |
| .gitignore | Anchors data/ ignore rule to the repo root to avoid ignoring the mirrored tests directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Verify timestamps are in the correct range (milliseconds since epoch) | ||
| # For 2024-01-01, timestamps should be around 1704067200000 (ms) | ||
| expected_first_ts = int(pd.Timestamp("2024-01-01").timestamp() * 1000) |
Comment on lines
+722
to
+741
| def test_get_chunk_performance_large_dataset(self): | ||
| """Test performance with large dataset (100k points).""" | ||
| n = 100000 | ||
| index = np.arange(n) | ||
| open_data = np.random.uniform(100, 200, n) | ||
| high = open_data + np.random.uniform(0, 10, n) | ||
| low = open_data - np.random.uniform(0, 10, n) | ||
| close = np.random.uniform(low, high) | ||
|
|
||
| dm = DataManager(index, open_data, high, low, close) | ||
|
|
||
| # Measure slicing performance | ||
| start_time = time.time() | ||
| chunk = dm.get_chunk(10000, 20000) # 10k points | ||
| elapsed_ms = (time.time() - start_time) * 1000 | ||
|
|
||
| # Should be well under 100ms for 10k points | ||
| assert elapsed_ms < 100, f"Slicing took {elapsed_ms:.2f}ms, expected <100ms" | ||
| assert len(chunk["index"]) == 10000 | ||
|
|
Comment on lines
+742
to
+761
| def test_get_chunk_performance_small_slice_large_dataset(self): | ||
| """Test performance of small slice from large dataset.""" | ||
| n = 100000 | ||
| index = np.arange(n) | ||
| open_data = np.random.uniform(100, 200, n) | ||
| high = open_data + np.random.uniform(0, 10, n) | ||
| low = open_data - np.random.uniform(0, 10, n) | ||
| close = np.random.uniform(low, high) | ||
|
|
||
| dm = DataManager(index, open_data, high, low, close) | ||
|
|
||
| # Small slice should be extremely fast | ||
| start_time = time.time() | ||
| chunk = dm.get_chunk(50000, 50100) # Just 100 points | ||
| elapsed_ms = (time.time() - start_time) * 1000 | ||
|
|
||
| # Should be very fast (< 10ms) | ||
| assert elapsed_ms < 10, f"Small slice took {elapsed_ms:.2f}ms, expected <10ms" | ||
| assert len(chunk["index"]) == 100 | ||
|
|
…alihaskar#45, alihaskar#47, alihaskar#49) alihaskar#45 — Test the pragma-excluded websocket/auto-shutdown paths Removed the 5 `# pragma: no cover` markers on the concurrency-critical paths (websocket heartbeat, _monitor_connection stale/disconnect branches, _run_server exception handler, and plot()'s blocking loop) and covered them with tests that drive the handlers directly — no real sockets or timing races. Coverage 100%. alihaskar#47 — Eliminate test-suite flakiness (xdist worker crash, sleep-based readiness) Added tests/pycharting/conftest.py with: - a `wait_until` polling helper replacing fixed `time.sleep` readiness waits, and - a `FakeUvicornServer` / `fake_uvicorn` fixture so behaviour tests exercise the full ChartServer lifecycle without binding real ports. Only the two genuine integration tests still start a real server (now waited on via health polling). Suite went from ~88s to ~9s and passes 10/10 consecutive runs under `-n auto` with no worker crashes. alihaskar#49 — Type the session registry and route responses - `_data_managers` is now `dict[str, DataManager]`. - Route responses use concrete Pydantic models (InitDataResponse, SessionInfo, SessionListResponse, DeleteSessionResponse, StatusResponse). - `plot()` returns a `PlotResult` TypedDict and `get_server_status()` a `ServerStatus` TypedDict instead of `dict[str, Any]`. This surfaced (and fixed) an unchecked-None in `_repr_html_`. `ty` stays clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
Updated: merged #57 + addressed #45, #47, #49This branch now contains the original Merged #57 (no conflicts) — brings in #56 (mkdocs description), #50 (staged #49 — Type the API boundary
#45 — Cover the pragma-excluded websocket/auto-shutdown paths
#47 — Eliminate test-suite flakiness
Verification
Closes #45 🤖 Generated with Claude Code |
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.
Fixes the
check_test_layout.pyparity failure for thedatapackage.Root cause
The migration to the mirrored
tests/<pkg>/**layout created mirrors forapiandcorebut never created thedatamirror — the flattests/test_data_ingestion.pyandtests/test_data_slicing.pywere deleted without replacement, sodata/ingestion.pyhad no test file at all.Two problems compounded it:
Missing mirror. Restored both deleted suites, merged into a single
tests/pycharting/data/test_ingestion.py. Organized to satisfy the checker's class-parity rule (eachTest<Class>must map to a source class):TestDataValidationError— every case that raisesDataValidationErrorTestDataManager— construction, properties, repr, andget_chunkslicing (folded in, sinceget_chunkis aDataManagermethod)validate_inputbehaviour (it's a free function, so unconstrained).gitignoreswallowed the dir. An unanchoreddata/rule (meant for local sample data) matchedtests/pycharting/data/— which is why the mirror could never be committed. Anchored it to/data/.Verification
check_test_layout.py→ exits 0 ("tests mirror sources 1:1")data/ingestion.py181/181)Closes #55
Also in this PR
Merged #57 and addressed three further quality issues (see the PR comment for the full summary):
-n autoruns)plotresponses (dropdict[str, Any])Closes #45
Closes #47
Closes #49
🤖 Generated with Claude Code