-
Notifications
You must be signed in to change notification settings - Fork 915
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
Reduce execution time of Parquet C++ tests #14750
Reduce execution time of Parquet C++ tests #14750
Conversation
CC @nvdbaranec @etseidl who wrote or modified many of these tests. |
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.
Good stuff. Amazing how much fat there was to trim 😅
…impr-reduce-pq-cpp-tests
@@ -688,60 +687,9 @@ TEST_P(ParquetV2Test, PartitionedWriteEmptyColumns) | |||
CUDF_TEST_EXPECT_TABLES_EQUAL(expected2, result2.tbl->view()); | |||
} | |||
|
|||
TEST_P(ParquetV2Test, LargeColumnIndex) |
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.
Can you help me understand why this test has been removed?
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.
That's a great question, I should have left a comment before reviews.
This test was included only to test the case where the writer writes the data in two batches (not the same as chunks!). Batching has since been disabled so we don't need this (huge) test.
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.
Understood, thanks!
Co-authored-by: Nghia Truong <[email protected]>
/merge |
Description
Reduced time from 90s to 25s on local system. Very few tests are impacted, and there should be no impact on code coverage.
Checklist