New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cleanup parquet tests #3116
Cleanup parquet tests #3116
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM -- my only hesitation is the removal of assert_send
but otherwise nice cleanup
assert_eq!(res, *item); | ||
} | ||
file_metadata | ||
} | ||
|
||
fn assert_send<T: Send>(t: T) -> T { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this check covered elsewhere ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, all the async plumbing requires it
"row count in metadata not equal to number of rows written" | ||
#[test] | ||
fn test_boolean_roundtrip() { | ||
let my_bool_values: Vec<_> = (0..2049).map(|idx| idx % 2 == 0).collect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I double checked that 2049
is the same size as parquet/tests/boolean_writer.rs
assert_eq!(&bytes[0..4], &[b'P', b'A', b'R', b'1']); | ||
|
||
// Now that we have written our data and are happy with it, make | ||
// sure we can read it back in < 5 seconds... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤯 not sure what this was all about
Benchmark runs are scheduled for baseline = de05308 and contender = 004a151. 004a151 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?
Closes #.
Rationale for this change
Cleans up some test duplication noticed whilst reviewing #3103
What changes are included in this PR?
Are there any user-facing changes?