Minor: migrate more arrow round trip tests to use RoundTripTest fixture - #10585
Conversation
| roundtrip(batch, Some(SMALL_SIZE / 2)); | ||
| RoundTripTest::new(Arc::new(a)) | ||
| .with_schema(Arc::new(schema)) | ||
| .run(); |
There was a problem hiding this comment.
This actually increases test coverage as
RoundTripTest::run() actually already calls into the same roundtrip code, but does so with:
- Multiple different row group sizes (including SMALL_SIZE/2)
- All supported encodings
- Dictionary/no dictionary pages
- DataPage V1 and V2
| .build() | ||
| .unwrap(); | ||
| let a = ListArray::from(a_list_data); | ||
| assert_eq!(a.null_count(), 1); |
There was a problem hiding this comment.
as above, this actually increases coverage substantially
| assert_eq!(binary_values.null_count(), 0); | ||
|
|
||
| roundtrip(batch, Some(SMALL_SIZE / 2)); | ||
| RoundTripTest::new(Arc::new(string_values)).run(); |
There was a problem hiding this comment.
technically speaking this encodes a single column batch rather than a multi-column batch, but the point of these tests is to test the round tripping of data through the whole parquet machiner; Batches with multiple columns are tested elsewhere
|
|
||
| roundtrip(batch.clone(), Some(SMALL_SIZE / 2)); | ||
| roundtrip(batch, None); | ||
| RoundTripTest::new(Arc::new(string_view_values)).run(); |
There was a problem hiding this comment.
As above, this ctually increases coverage because it tries a both these two sets of max row group sizes, and a bunch of other permutations
| .unwrap(); | ||
|
|
||
| // Disable dictionary to exercise plain encoding paths in the reader. | ||
| for version in [WriterVersion::PARQUET_1_0, WriterVersion::PARQUET_2_0] { |
There was a problem hiding this comment.
RoundtripTest also tests with/without dictionary and data page formats (and several other parameters)
|
Thank you @Jefffrey |
…re (apache#10585) # Which issue does this PR close? - part of apache#10540 - Follow on to apache#10545 # Rationale for this change There are 4000+ lines of tests in arrow_writer (and a bunch more in arrow_reader, I might point out) -- this makes it hard to find and add new tests or to evaluate test coverage I encapsulated the testing harness into a struct in apache#10545 and now I want to (slowly) move more tests to use this new harness. # What changes are included in this PR? 1. Migrate a few tests from using `roundtrip()` to using `RoundTripTest`; This actually increases coverage as I will descsribe below 2. Note this is less code and more coverage # Are these changes tested? yes by CI # Are there any user-facing changes? No
2 participants
Which issue does this PR close?
Rationale for this change
There are 4000+ lines of tests in arrow_writer (and a bunch more in arrow_reader, I might point out) -- this makes it hard to find and add new tests or to evaluate test coverage
I encapsulated the testing harness into a struct in #10545 and now I want to (slowly) move more tests to use this new harness.
What changes are included in this PR?
roundtrip()to usingRoundTripTest; This actually increases coverage as I will descsribe belowAre these changes tested?
yes by CI
Are there any user-facing changes?
No