Populate the benchmark, stress and property-based test tiers - #90
Open
tschm wants to merge 1 commit into
Open
Conversation
The Makefile wires up three test tiers this repo never filled. `make test` passed --ignore=tests/benchmarks --ignore=tests/stress for directories that did not exist, `make benchmark` and `make stress` had nothing to run, and hypothesis shipped via .rhiza/requirements/tests.txt without a single import. tests/benchmarks/ measures the two hot paths: DataManager.get_chunk, which runs on every pan and zoom, and validate_input, which dominates plot() start-up. On this machine a 5k-bar viewport slice costs ~218us and a full 250k-bar serialize ~14.5ms. tests/stress/ pushes the shared _data_managers registry the unit suite only touches one session at a time: 100 concurrent registrations, 200 overlapping reads of one manager, and create/delete churn. The property tests state validate_input's contract as invariants rather than examples — every series normalizes to a length-n ndarray, length disagreement always raises, auto-filled high never falls below low, and list and ndarray inputs agree. They carry the `property` marker that pytest.ini already registers, so `make hypothesis-test` selects them. hypothesis is added to the test dependency group so `uv run pytest` works without `make install` having run first, matching that group's stated intent. Both new directories are exempt from check_test_layout.py by design, so test/source parity still holds. Coverage stays at 100%. Closes alihaskar#86 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This pull request fills out the previously-empty test tiers by adding benchmark and stress suites, plus property-based tests for validate_input, and wires hypothesis into the standard test dependency group so uv run pytest works without a separate install step.
Changes:
- Add
tests/benchmarks/withpytest-benchmarkbenchmarks forvalidate_inputandDataManager.get_chunk. - Add
tests/stress/to exercise the global_data_managersregistry under concurrent access patterns. - Add Hypothesis-based property tests for
validate_inputand includehypothesisin thetestdependency group.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Locks hypothesis and its dependency (sortedcontainers) and adds it to the test group. |
| pyproject.toml | Declares hypothesis under [project.optional-dependencies].test to support uv run pytest. |
| tests/benchmarks/init.py | Introduces the benchmarks test tier and documents exclusion from the default unit run. |
| tests/benchmarks/test_ingestion_benchmarks.py | Adds ingestion and slicing benchmarks for the core hot paths. |
| tests/stress/init.py | Introduces the stress test tier and documents exclusion from the default unit run. |
| tests/stress/test_session_registry_stress.py | Adds concurrency and churn stress tests around the session registry and slicing path. |
| tests/pycharting/data/test_ingestion.py | Adds Hypothesis property-based tests expressing validate_input invariants. |
Suppressed comments (2)
tests/benchmarks/test_ingestion_benchmarks.py:44
- This benchmark test uses the
ohlcfixture whose values are not allnp.ndarray(the index is apd.DatetimeIndex). Theohlcparameter type should be loosened to avoid incorrect typing.
def test_validate_input_on_dense_ohlc(benchmark, ohlc: dict[str, np.ndarray]) -> None:
tests/benchmarks/test_ingestion_benchmarks.py:63
- This benchmark test uses the
ohlcfixture whose values are not allnp.ndarray(the index is apd.DatetimeIndex). Theohlcparameter type should be loosened to avoid incorrect typing.
def test_data_manager_construction(benchmark, ohlc: dict[str, np.ndarray]) -> None:
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+25
to
+27
| def ohlc() -> dict[str, np.ndarray]: | ||
| """A dense OHLC series of ``N`` bars with a datetime index and two overlays.""" | ||
| rng = np.random.default_rng(seed=0) |
Comment on lines
+39
to
+41
| def manager(ohlc: dict[str, np.ndarray]) -> DataManager: | ||
| """A ``DataManager`` over the dense series, built once for the module.""" | ||
| return DataManager(**ohlc) |
|
|
||
|
|
||
| def test_concurrent_chunk_reads_are_consistent(client: TestClient) -> None: | ||
| """Overlapping viewport reads of one session all return the same bytes.""" |
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.
Closes #86
Problem
The Makefile wires up three test tiers this repo never filled:
make testpassed--ignore=tests/benchmarks --ignore=tests/stressfor directories that did not exist.make benchmarkandmake stresshad nothing to run — andpytest-benchmarkemitted a warning on everymake testrun about benchmarks being disabled, for benchmarks that did not exist.hypothesisshipped via.rhiza/requirements/tests.txtwithout a single import.Change
tests/benchmarks/measures the two hot paths:DataManager.get_chunk, which runs on every pan and zoom, andvalidate_input, which dominatesplot()start-up. Measured here:get_chunk— 5k-bar viewport slicevalidate_input— 250k barsDataManagerconstruction — 250k barsget_chunk— full 250k-bar serializetests/stress/pushes the shared_data_managersregistry that the unit suite only ever touches one session at a time: 100 concurrent registrations, 200 overlapping reads of one manager, and create/delete churn.Property-based tests state
validate_input's contract as invariants rather than examples — every series normalizes to a length-n ndarray, length disagreement always raises, auto-filledhighnever falls belowlow, and list and ndarray inputs agree. They carry thepropertymarkerpytest.inialready registers, somake hypothesis-testselects them.hypothesisis added to thetestdependency group souv run pytestworks withoutmake installhaving run first — matching that group's stated intent in the existing comment.A note on the stress tests
The first draft of
test_many_concurrent_sessions_stay_isolatedoffset onlyclose, leavinghighbelow it.validate_inputcorrectly rejected that withHigh must be >= max(Open, Close)— a good sign for the validation path. The test now shifts the whole bar.Verification
make test— 176 passed (was 172), coverage 100%make stress— 4 passed, 180 deselectedmake hypothesis-test— 4 passed, 50 examples eachmake benchmark— 4 benchmarks, numbers abovecheck_test_layout.py— parity still holds; both new directories are exempt by design🤖 Generated with Claude Code