Repository navigation
Add Hegel coverage for Arrow extension round trips - #11361
connortsui20 wants to merge 1 commit into
Conversation
fc51d62 to
f2987e6
Compare
f2987e6 to
73f31f1
Compare
f32781e to
73a471c
Compare
73a471c to
53f31ab
Compare
53f31ab to
d5c2d05
Compare
d5c2d05 to
9e3f744
Compare
|
I think I want to also delete a lot of the existing tests too |
9e3f744 to
f0887af
Compare
Merging this PR will degrade performance by 11.55%2 warnings
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
8e2125f to
35520db
Compare
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
35520db to
0d6def1
Compare
|
Merge #11184 before this PR. It establishes the contract that metadata determines extension storage, and reads must reject stored storage that disagrees with that declaration. The conformance tests here need to target that contract. After rebasing, this PR also needs a testing redesign: the properties should replace ordinary positive unit coverage, with independent expectations and useful extension-specific checks. Adding round-trip properties while leaving all the old happy-path tests intact is not the intended result. Handoff for a future agent: merge order, API migration, and the conformance test redesign1. Resume from the actual branch stateAt this handoff, the published head of this PR is Refresh #11184, resolve its conflicts, complete review and validation, and merge it first. It was still draft and conflicting at this handoff, and its listed CI jobs were skipped. Do not treat that as a merge-ready result. Keep the production contract change and the test redesign separately reviewable, and preserve existing regression coverage until its replacement is demonstrated. Then rebase this PR onto the updated There are also interrupted, uncommitted edits in the original 2. Migrate the fixtures to the landed extension APIThe following describes the inspected #11184 patch. Confirm it against the final merged version.
Update both the owning crate property suites and the dtype factory in Metadata properties must test Move wrong-storage checks to the boundary that can receive wrong storage: dtype deserialization, array construction, or scalar construction. Use the full dtype/plugin deserialization path when testing exact storage agreement, rather than assuming the lower-level Do not assume that every mutation of stored storage must fail. A different valid vector width or float element type can describe another valid vector, since its bytes leave those facts out. A nullable vector element must fail. A tensor list width that disagrees with its serialized shape must fail. Derive each expected outcome from the type's contract. Add a direct property that changing top-level nullability preserves metadata and nested storage constraints. Deserializing storage forms that differ only in top-level nullability must reconstruct the same metadata. 3. Make generated conformance own ordinary successful behaviorThe current suites mostly build a Vortex array, let Vortex choose an Arrow field, and ask Vortex to read its own output. That can miss matching import/export mistakes. The existing numeric generator is also limited to Generate a small typed logical case beside each extension's tests: source values, nullability, metadata, and shape information. Build the Vortex array and the reference Arrow field/array separately from that case. Expected Arrow layouts and native metadata must come from arrow-rs, GeoArrow, or test-owned mappings, never from the Vortex exporter under test. Share small one-way assertions that import independently built Arrow inputs and export to independently expected Arrow representations. Also check canonical fields and schemas against independent expectations. Keep round-trip assertions for composition and nested transport. Test JSON's supported string layouts and WKB's binary layouts, including explicit targets and the canonical default behavior. Keep the shared harness free of extension-specific policies. Use ordinary local generators and check functions, not a trait that forces every extension to implement every check. Keep Hegel's native backend out of the libFuzzer dependency path, and preserve existing fuzz seed decoding. Strengthen the generators:
4. Reuse cases for extension-specific propertiesUse separate properties with shared local generators. A tensor stride test must not run the Arrow matrix first, and a UUID aggregate test must not depend on Arrow transport.
Keep vector numerical algorithms, spatial predicates, and unrelated core conformance suites outside this pass. The additional properties above have direct references and reuse the same logical cases. 5. Delete the tests that the stronger properties supersedeAudit tests from the rebased tree, including changes introduced by #11184. Remove whole redundant test functions and unused helpers, not only individual value assertions.
For each deletion, identify its replacement property and any important trigger that remains explicit. The acceptance criterion is stronger coverage with less duplicate scaffolding, not a fixed deletion count. Use Hegel explicit examples with the same check functions as generated cases, but keep only good examples. Do not move every old fixture into an attribute. Useful directed cases include a zero-width array with nonzero row count, nulls masking stored values, nonidentity dictionary selection, unequal runs, and a nontrivial tensor permutation containing a zero axis. Preserve relevant historical bug triggers. Finite dispatch choices such as supported temporal units and coordinate dimensions can use compact deterministic mapping checks, while deep combinations remain generated. 6. Validate and hand offLocal targeted tests and CI checks are authorized for this implementation. Run the affected crate tests and fuzz seed tests, scoped formatting and Clippy, and targeted docs and MSRV checks that match CI's flags. Keep workspace-wide execution in CI. Follow the repository guidance and the pinned formatting toolchain. The last published commit includes two earlier CI fixes that must survive the rebase: Review the complete diff with |
Summary
Builds on #11360.
Plugin tests need to check complete dtype identity and array values across nesting boundaries. Generate each plugin's metadata and storage in its owning crate, with shared transport checks in vortex-arrow, so neither vortex-array nor vortex-arrow needs to depend on the plugin crates.
Changes
Use the workspace's existing Hegel dependency. Each generated type is checked at the top level and inside structs, lists, fixed-size lists, map values, dictionaries, and run-end arrays, including nullability, explicit fields, and targetless exports. Top-level FixedShapeTensor exports still require an explicit field.
Keep runs bounded to 100 Hegel cases per suite, at most 32 leaf rows, and three extra nesting levels. Independent draws let Hegel shrink metadata choices, null masks, and values, and failures print a replay blob.
The fuzz target wraps storage from the existing arbitrary array generator. Hegel generation has a separate feature from the shared transport checks because Hegel's native backend cannot link under libFuzzer's coverage flags on macOS. Seeded tests cover all 15 included type families. Variant is deliberately excluded pending #11359.
The existing generators exercise the typed adapters through the owning crates' initializers. The adapter's policy and malformed-metadata regressions live in #11360, so this layer keeps its generated transport coverage without duplicating those cases.
Validation