Consolidate arrow ipc tests and increase coverage #3427
Conversation
github-actions
bot
added
arrow
| .unwrap(); | ||
|
|
||
| let mut reader = FileReader::try_new(file, None).unwrap(); | ||
| verify_arrow_file(&testdata, version, path); |
There was a problem hiding this comment.
I consolidated the tests so the streams and file tests are in the same function, which both reduces code duplication as well as making it easier to audit which files were included in the test
| let testdata = arrow_test_data(); | ||
| let version = "0.17.1"; | ||
| let paths = vec!["generated_union"]; | ||
| paths.iter().for_each(|path| { |
There was a problem hiding this comment.
This used to be (partially) covered in arrow/tests/ipc.rs
| "generated_datetime", | ||
| "generated_custom_metadata", |
There was a problem hiding this comment.
I was quite pleased to find that the reader/writer works for many more integration tests 
| }); | ||
| } | ||
|
|
||
| #[test] | ||
| fn read_generated_streams_014() { |
There was a problem hiding this comment.
the stream tests have been incorporated into the same test as the file tests (so I didn't have to audit two separate lists of files)
| }); | ||
| } | ||
|
|
||
| #[test] | ||
| fn read_and_rewrite_generated_streams_014() { | ||
| fn write_0_1_7() { |
There was a problem hiding this comment.
This used to be (partially) covered in arrow/tests/ipc.rs
| @@ -1,61 +0,0 @@ | |||
| // Licensed to the Apache Software Foundation (ASF) under one | |||
There was a problem hiding this comment.
This is now completely covered by arrow-integration-testing/tests/ipc_writer.rs and arrow-integration-testing/tests/ipc_reader.rs in a uniform manner
| @@ -102,163 +95,119 @@ fn read_generated_be_files_should_work() { | |||
| .unwrap(); | |||
|
|
|||
| FileReader::try_new(file, None).unwrap(); | |||
|
|
|||
| // While the the reader doesn't error but the values are not read correctly | |||
There was a problem hiding this comment.
Should we file a ticket for this?
| "generated_map", | ||
| // fails with | ||
| // thread 'read_1_0_0_littleendian' panicked at 'assertion failed: `(left == right)` | ||
| //"generated_map_non_canonical", |
There was a problem hiding this comment.
I think it might be worth a ticket for this test, it seems to fail in multiple places
|
Benchmark runs are scheduled for baseline = e256e3d and contender = 81abc1a. 81abc1a is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
re #3389
Rationale for this change
I want to ensure that the Arrow IPC writer had sufficient test coverage before I potentially messed around with it
What changes are included in this PR?
Are there any user-facing changes?
No this is all test changes