Test infrastructure: detect missing data types, enum not written out manually - #96
Open
konstibob wants to merge 2 commits into
Open
Test infrastructure: detect missing data types, enum not written out manually#96konstibob wants to merge 2 commits into
konstibob wants to merge 2 commits into
Conversation
The dtype test providers mirrored our own DataType enum by hand, which made them tautological: they could only assert that what we implemented is implemented, so a data type we never added was invisible and no test could fail for it. That is how float16 and string went unnoticed despite a broad interop matrix. Drive coverage from outside our code instead: - ZarrTest.dataTypeProviderV3/V2 now derive from DataType.values(), so a new data type is picked up by every test using them without anyone remembering to extend a list. - New DataTypeConformanceTest checks an external list (generated from zarr-python's registry, committed as spec-data-types-v3.json) against our enum. Each of the 11 data types we lack is enumerated in KNOWN_UNSUPPORTED with a note on what it needs. The assertion is bidirectional, so implementing a data type shows up as a deletion there and a stale entry fails too. Metadata-only: 46 tests in 0.6s, no I/O and no Python. - ZarrV3Test.dataTypeAndEndianProvider derives from the enum as well. The hand-written version had already drifted, omitting INT64 and UINT64, so 8-byte byte-order handling was untested. Also cut the interop tier's cost. It ran one `uv run` per test case, paying interpreter startup plus `import zarr, numpy` roughly 136 times; the 16 KB test arrays were never the expense. zarr-python now runs as one long-lived worker speaking JSON lines over stdin/stdout. A worker rather than an up-front batch keeps per-test semantics: each test still makes its own call and asserts its own result, so failures stay attributed to the test that caused them. Fixture logic moved to zarr_fixtures.py, which both the worker and the existing CLI scripts call, so the two cannot drift. Full interop run: 129.3s -> 19.2s for the same 174 passing tests. Tag the interop tests `interop` and exclude them from `mvn test` by default, so pull requests get the fast offline tiers and the cross-implementation matrix moves to a nightly job. Interop coverage is not optional for a format library: a zarr-java write followed by a zarr-java read passes even when reader and writer share the same misreading of the spec, and only a second implementation catches that.
Member
|
Hmm. I am not sure, I want this. I don't think zarr-java needs to implement all the data types that zarr-python supports. We should add support based on our users' needs. Alos, this adds a lot of code that needs to be maintained. |
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
2 participants
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.
The dtype test providers mirrored our own DataType enum by hand, which made
them tautological: they could only assert that what we implemented is
implemented, so a data type we never added was invisible and no test could
fail for it. That is how float16 and string went unnoticed despite a broad
interop matrix.
Drive coverage from outside our code instead:
zarr-python's data type registry and committed alongside it. What we lack is
enumerated in KNOWN_UNSUPPORTED with a note on what each needs, and the
assertion is bidirectional, so implementing a data type shows up as a
deletion there and a stale entry fails too. Metadata-only: no I/O and no
Python.
pull request it is checked against the zarr-python we pin; nightly, against
the latest release. The nightly one is deliberately not a pull-request gate,
since no pull request causes upstream to publish a version.
Also cut the interop tier's cost. It ran one
uv runper test case, payinginterpreter startup plus
import zarr, numpyevery time; the test arrays werenever the expense. zarr-python now runs as one long-lived worker speaking JSON
lines over stdin/stdout. A worker rather than an up-front batch keeps per-test
semantics, so failures stay attributed to the test that caused them. Fixture
logic moved to zarr_fixtures.py, which both the worker and the CLI scripts
call, so the two cannot drift.
Full interop run: 129.3s -> 19.2s.