maltzsama commented on issue #10: URL: https://github.com/apache/arrow-swift/issues/10#issuecomment-5078784516
Hi everyone, I'd like to propose a change to enable integration and IPC conformance testing in `arrow-swift`, aligning it with the established practices across the rest of the Apache Arrow ecosystem (C++, Python, Rust, Java, etc.). ### Current Situation Today, IPC conformance testing in `arrow-swift` relies on a local data corpus generated at build time by a Go program under `data-generator/`. This setup has several limitations: - **Limited Coverage:** It produces only 3 test cases, while the official `arrow-testing` repository offers roughly 30 test files covering multiple format versions (0.14.1, 1.0.0, etc.), compression, shared dictionaries, and big-endian. - **Provenance and Maintenance:** The data is unique to this repository rather than using the shared artifacts that every other implementation checks against. It also introduces a Go module and toolchain into a Swift repository solely to emit three test files that already exist upstream. - **No External Streaming Tests:** The current streaming tests are self-consistency checks (`writeStreaming` + `readStreaming`). A symmetric bug in both writer and reader would go undetected. ### Proposed Solution Replace the local generator with the official **`apache/arrow-testing`** repository, consumed as a submodule, following the pattern used by all other Arrow implementations. **Benefits:** - **Increased Coverage:** Jump from 3 to ~30 test cases, including backward-compatibility tests. - **Reduced Maintenance:** Eliminates the need to maintain a custom data generator. - **Ecosystem Alignment:** Adopts the standard Arrow test data source, making collaboration and cross-implementation comparisons easier. - **Robust Streaming Tests:** Enables testing against real `.stream` golden files, verifying conformance rather than just self-consistency. ### The RC Verification Challenge One legitimate concern is that the `arrow-testing` submodule is not included in the release tarball generated by `git archive` during RC verification. To address this, I propose following the approach already used by the main Apache Arrow project: the verification script (`verify_rc.sh`) can transparently clone `apache/arrow-testing` if the data is not available locally. This keeps test coverage during RC verification, does not introduce a network dependency that doesn't already exist (the current Go generator also runs `go get`), and does not compromise tarball reproducibility. ### Implementation Plan To ensure a smooth transition, I suggest breaking the work into clear, actionable steps: 1. [ ] Add the `arrow-testing` submodule and configure `build.sh` to use the `ARROW_TEST_DATA` environment variable, with a fallback to `git clone` during RC verification. 2. [ ] Add new tests consuming files from `arrow-testing`, such as `1.0.0-littleendian/generated_primitive.arrow_file` and its `.stream` counterpart. 3. [ ] Port existing tests (`testFileReader_bool`, etc.) to use the new golden files and assertions based on official data, rather than the current ones (which are specific to the Go generator). 4. [ ] Remove the `data-generator/` directory and all references to it: in `build.sh`, `Dockerfile`, `dependabot.yml`, and anywhere else. 5. [ ] Map supported and unsupported types, creating a conformance inventory. Each gap (e.g., `decimal`, `union`, `map`) can become a separate issue. **Note:** The `CDataWGo/` directory is explicitly out of scope, as it tests the C Data Interface against a real Go runtime, which cannot be replaced by static data. ### Questions for Maintainers 1. Is the fallback approach in `build.sh` (cloning `arrow-testing` if the submodule is missing) acceptable for the release process? 2. Are there any objections to using submodules in this repository, beyond the tarball issue already addressed? 3. Would the `data/forward-compatibility/schema_v6.arrow` file from `arrow-testing` be useful to test our `ArrowWriter`, which currently writes with `.max` metadata version? Including it in the conformance inventory makes sense to ensure write stability. I'm happy to do the work once there's consensus on the approach. Thank you for your feedback! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
