alamb opened a new pull request, #10543: URL: https://github.com/apache/arrow-rs/pull/10543
# Which issue does this PR close? - Part of #10540 # Rationale for this change The "round trip" tests (write data with `ArrowWriter`, read it back, verify the exact same values are returned) are currently embedded in the `#[cfg(test)]` module of `parquet/src/arrow/arrow_writer/mod.rs`, mixed in with layout / statistics / metadata tests. #10540 proposes consolidating round trip tests in `parquet/tests/arrow_writer/roundtrip.rs` so it is easier to see what is covered and where to add new tests. This PR **only moves code** — no tests are added, removed, or modified — so it is easy to verify the coverage is exactly the same. # What changes are included in this PR? 1. First commit: moves `parquet/tests/arrow_writer.rs` to `parquet/tests/arrow_writer/mod.rs` (and updates the target path in `Cargo.toml`) so the test binary can have submodules, following the pattern of `tests/arrow_reader/` 2. Second commit: moves the 97 round trip tests, and the helper family they use (`roundtrip`, `roundtrip_opts`, `one_column_roundtrip*`, `values_required`/`values_optional`/`required_and_optional`, `check_bloom_filter`, `ree_write_read_roundtrip`), verbatim into the new `parquet/tests/arrow_writer/roundtrip.rs` Test count is conserved: the tests module had 128 tests; 97 moved and 31 remain (layout, page-index/statistics, metadata/schema-only, error-assertion, page-store, and row-group-limit tests, which are not data round trips per the definition in #10540). The only changes beyond the move itself: - Import adjustments (`crate::` → `parquet::`, plus trimming now-unused imports from the source module) and `rustfmt` re-joining a handful of lines that fit on one line after losing an indentation level - The three `test_page_stats_*` tests use `roundtrip_opts` but **cannot** move to an integration test because they decode page headers with crate-private thrift APIs (`crate::parquet_thrift`). They stay behind with a retained copy of `roundtrip_opts` / `roundtrip_opts_with_array_validation` (with a comment explaining why) Judgment calls reviewers may want to double-check: - Moved even though they are borderline: the bloom filter probe tests and `u32_min_max`/`u64_min_max`/`statistics_null_counts_only_nulls` (they assert on metadata but do their write/read/compare through the shared `one_column_roundtrip` helpers), and `bool_large_single_column` (write-only, but part of the per-type single-column suite) - Left behind even though they compare data: `test_aggregates_records` and `complex_aggregate` (their focus is row-group aggregation, and they share the `row_group_sizes` helper with the row-group-limit tests), and `ree_struct_with_ree_child` (schema-only assertions) # Are these changes tested? Covered by the moved tests themselves: `cargo test -p parquet --test arrow_writer` runs 100 tests (97 moved + 3 pre-existing) and `cargo test -p parquet --lib -- arrow::arrow_writer::tests` runs the remaining 31. # Are there any user-facing changes? No, test-only change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
