alamb opened a new issue, #10540:
URL: https://github.com/apache/arrow-rs/issues/10540

   ### Is your feature request related to a problem or challenge?
   
   While working on the ALP encoder/decoder with @sdf-jkl in 
https://github.com/apache/arrow-rs/pull/9372, I found the parquet tests were 
spread out all over the place and thus is was hard to know where to put end to 
end tests and what was current covered or not covered. 
   
   There are several categories of tests that appear frequently in the parquet 
crate and are currently scattered about
   - "round trip" tests -- they write data to a parquet file and then read it 
back, ensuring the exact same values are returned
   - "known fixture tests" -- read files from `parquet-testing` and assert that 
their contents match the expected values
   
   This approach directly tests the decoders, and transitively tests the 
encoders (since we know the decoder can read third-party files, we have some 
confidence that the writer is writing valid data as well)
   
   ### Describe the solution you'd like
   
   I would like to consolidate the end to end testing into arrow_reader and 
arrow_writer tests
   
   * consolidate "fixture" tests in 
parquet/tests/arrow_reader/parquet_testing.rs
   * consolidate "round trip" tests in parquet/tests/arrow_writer/roundtrip.rs
   * consolidate arrow_writer tests: move arrow_writer_layout.rs into 
arrow_writer/layout.rs
   
   ### Describe alternatives you've considered
   
   <details><summary>AI Research</summary>
   <p>
   
   
   # Parquet roundtrip test locations
   
   Tests that write full Arrow arrays / `RecordBatch`es to a complete parquet 
file, read it back, and compare the **data** to the original. (Statistics-only, 
layout/memory, metadata/schema-only, bloom-filter-probe, and error-assertion 
tests are excluded.)
   
   ## Core suites
   
   | # | Location | What lives there |
   |---|----------|------------------|
   | 1 | 
[parquet/src/arrow/arrow_writer/mod.rs](https://github.com/apache/arrow-rs/blob/main/parquet/src/arrow/arrow_writer/mod.rs)
 | **The main per-type suite** (~95 tests). Helper family: `roundtrip`, 
`roundtrip_opts`, `roundtrip_opts_with_array_validation`, 
`one_column_roundtrip`, `one_column_roundtrip_with_schema`/`_with_options`, 
`values_required`, `values_optional`, `required_and_optional`, plus the 
REE-specific `ree_write_read_roundtrip` family. |
   | 2 | 
[parquet/src/arrow/arrow_reader/mod.rs](https://github.com/apache/arrow-rs/blob/main/parquet/src/arrow/arrow_reader/mod.rs)
 | ~24 standalone `ArrowWriter` → `ParquetRecordBatchReader` data roundtrips: 
`test_unsigned_roundtrip`, `test_float16_roundtrip`, `test_time_utc_roundtrip`, 
`test_date32_roundtrip`, `test_date64_roundtrip`, `test_decimal32_roundtrip`, 
`test_decimal64_roundtrip`, `test_decimal_roundtrip`, `test_decimal_list`, 
`test_decimal_nullable_struct`, `test_int32_nullable_struct`, `test_list_skip`, 
`test_list_selection`, `test_list_selection_fuzz`, `test_row_group_batch`, etc. 
Also the big `run_single_column_reader_tests`/`TestOptions` harness — it 
compares full arrow data, though its write leg uses the low-level 
`SerializedFileWriter` rather than `ArrowWriter`. |
   
   ## Async
   
   | # | Location | What lives there |
   |---|----------|------------------|
   | 3 | 
[parquet/src/arrow/async_writer/mod.rs](https://github.com/apache/arrow-rs/blob/main/parquet/src/arrow/async_writer/mod.rs)
 | 4–5 `AsyncArrowWriter` → read-back-and-`assert_eq!` tests 
(`test_async_writer`, `test_async_writer_file`, async-vs-sync comparison). |
   | 4 | 
[parquet/src/arrow/async_writer/store.rs](https://github.com/apache/arrow-rs/blob/main/parquet/src/arrow/async_writer/store.rs)
 | 1 object-store `AsyncArrowWriter` data roundtrip. |
   | 5 | 
[parquet/src/arrow/async_reader/mod.rs](https://github.com/apache/arrow-rs/blob/main/parquet/src/arrow/async_reader/mod.rs)
 | ~6 hand-rolled `ArrowWriter` → `ParquetRecordBatchStreamBuilder` data 
roundtrips: `test_nested_skip`, `test_nested_lists`, 
`test_limit_multiple_row_groups`, `test_parquet_record_batch_stream_schema`, 
`test_predicate_cache_disabled`. No shared helper. |
   
   ## Push decoder
   
   | # | Location | What lives there |
   |---|----------|------------------|
   | 6 | 
[parquet/src/arrow/push_decoder/mod.rs](https://github.com/apache/arrow-rs/blob/main/parquet/src/arrow/push_decoder/mod.rs)
 | ~35 tests decoding a shared `ArrowWriter`-written `TEST_FILE_DATA` fixture 
and comparing batches (`test_decoder_all_data`, 
incremental/filter/selection/limit variants). |
   | 7 | 
[parquet/src/arrow/push_decoder/reader_builder/mod.rs](https://github.com/apache/arrow-rs/blob/main/parquet/src/arrow/push_decoder/reader_builder/mod.rs)
 | 6 more tests against the same fixture. |
   
   ## Integration tests (`parquet/tests/`)
   
   | # | Location | What lives there |
   |---|----------|------------------|
   | 8 | 
[parquet/tests/arrow_reader/row_filter/sync.rs](https://github.com/apache/arrow-rs/blob/main/parquet/tests/arrow_reader/row_filter/sync.rs)
 | 5 tests: write batch with `ArrowWriter`, read back with 
`RowFilter`/`RowSelection`, compare filtered data. |
   | 9 | 
[parquet/tests/arrow_reader/row_filter/async.rs](https://github.com/apache/arrow-rs/blob/main/parquet/tests/arrow_reader/row_filter/async.rs)
 | 11 async equivalents; local helper `make_two_column_i64_file`. |
   | 10 | 
[parquet/tests/arrow_reader/predicate_cache.rs](https://github.com/apache/arrow-rs/blob/main/parquet/tests/arrow_reader/predicate_cache.rs)
 | 6 sync+async roundtrips through the predicate cache, comparing decoded data 
(plus cache metrics). |
   | 11 | 
[parquet/tests/arrow_reader/large_string_overflow.rs](https://github.com/apache/arrow-rs/blob/main/parquet/tests/arrow_reader/large_string_overflow.rs)
 | 4 tests (one per encoding): write large binary batch, read back, compare. |
   
   ## Encryption
   
   | # | Location | What lives there |
   |---|----------|------------------|
   | 12 | 
[parquet/tests/encryption/encryption_util.rs](https://github.com/apache/arrow-rs/blob/main/parquet/tests/encryption/encryption_util.rs)
 | Shared helper `read_and_roundtrip_to_encrypted_file` + 
`verify_encryption_test_data` (data comparison). |
   | 13 | 
[parquet/tests/encryption/encryption.rs](https://github.com/apache/arrow-rs/blob/main/parquet/tests/encryption/encryption.rs)
 | Sync encrypted data roundtrips: `uniform_encryption_roundtrip`, 
key-retriever variants, several `write_*_encryption` tests. |
   | 14 | 
[parquet/tests/encryption/encryption_async.rs](https://github.com/apache/arrow-rs/blob/main/parquet/tests/encryption/encryption_async.rs)
 | Async + multi-threaded encrypted-write roundtrips 
(`test_multi_threaded_encrypted_writing`, 
`test_concurrent_encrypted_writing_over_multiple_row_groups`). |
   
   ## Variant
   
   | # | Location | What lives there |
   |---|----------|------------------|
   | 15 | 
[parquet/src/variant.rs](https://github.com/apache/arrow-rs/blob/main/parquet/src/variant.rs)
 | 3 tests with their own local `roundtrip(VariantArray)` / `write_to_buffer` / 
`read_to_batch` helpers. |
   
   ## One-off hand-rolled roundtrips
   
   | # | Location | What lives there |
   |---|----------|------------------|
   | 16 | 
[parquet/src/arrow/array_reader/fixed_size_list_array.rs](https://github.com/apache/arrow-rs/blob/main/parquet/src/arrow/array_reader/fixed_size_list_array.rs)
 | ~2 tests (`test_read_as_dyn_list` and neighbors). |
   | 17 | 
[parquet/src/arrow/array_reader/map_array.rs](https://github.com/apache/arrow-rs/blob/main/parquet/src/arrow/array_reader/map_array.rs)
 | 1 test via `read_map_array_column`. |
   | 18 | 
[parquet/src/arrow/array_reader/fixed_len_byte_array.rs](https://github.com/apache/arrow-rs/blob/main/parquet/src/arrow/array_reader/fixed_len_byte_array.rs)
 | 1 test (`test_decimal_list`). |
   | 19 | 
[parquet/src/arrow/array_reader/list_array.rs](https://github.com/apache/arrow-rs/blob/main/parquet/src/arrow/array_reader/list_array.rs)
 | 1 test (`test_nested_lists`). |
   | 20 | 
[parquet/src/column/chunker/cdc.rs](https://github.com/apache/arrow-rs/blob/main/parquet/src/column/chunker/cdc.rs)
 | ~25 content-defined-chunking tests: write batches, read back, compare data 
via own `write_with_cdc_options` / `read_batches` / `find_differences` helpers. 
|
   
   ## Borderline (data is compared, but that's not the test's main point)
   
   - 
[parquet/tests/arrow_reader/io/](https://github.com/apache/arrow-rs/blob/main/parquet/tests/arrow_reader/io/mod.rs)
 — writes a `TEST_FILE_DATA` fixture with `ArrowWriter` and checks decoded 
batches, but the assertions are primarily about which byte ranges get read.
   - 
[parquet/src/file/writer.rs](https://github.com/apache/arrow-rs/blob/main/parquet/src/file/writer.rs)
 — `test_byte_stream_split_extended_roundtrip` reads a file, rewrites with 
`ArrowWriter`, and verifies mostly encodings.
   
   ## Summary
   
   There are at least **five independent helper families** doing 
write-batch-then-read-and-compare (`roundtrip*` in `arrow_writer/mod.rs`, 
`run_single_column_reader_tests` in `arrow_reader/mod.rs`, the encryption 
`read_and_roundtrip_to_encrypted_file`, the CDC helpers, and the variant 
helpers), two duplicated `TEST_FILE_DATA` fixture patterns (`push_decoder` and 
`tests/arrow_reader/io`), and roughly a dozen files that hand-roll 
`ArrowWriter` → reader inline.
   
   Related issues: #9269 (split up Parquet reader tests), #9348 (split up 
Parquet async reader tests).
   
   
   </p>
   </details> 
   
   ### Additional context
   
   Related issues
   - https://github.com/apache/arrow-rs/issues/9348


-- 
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]

Reply via email to