-
Notifications
You must be signed in to change notification settings - Fork 119
chore: make tests async if they rely on async #1438
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
base: main
Are you sure you want to change the base?
Conversation
| LogPathFileType::MultiPartCheckpoint { | ||
| part_num, | ||
| num_parts, | ||
| } => { |
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.
NOTE: This PR is best review with whitespace ignored, so these big indentation changes don't clutter the diff.
| #[tokio::test] | ||
| async fn test_create_checkpoint_metadata_batch() -> DeltaResult<()> { |
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.
The changes in this PR are extremely mechanical: add async, .await, and #[tokio::test] annotations... plus cargo fmt and indentation noise.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1438 +/- ##
==========================================
+ Coverage 84.85% 85.32% +0.47%
==========================================
Files 119 119
Lines 30961 32336 +1375
Branches 30961 32336 +1375
==========================================
+ Hits 26271 27590 +1319
- Misses 3408 3413 +5
- Partials 1282 1333 +51 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
What changes are proposed in this pull request?
A whole bunch of unit tests are async code masquerading as sync, by various hacks to obtain an executor and block on futures produced by object_store and parquet calls. Clean up the mess by plumbing async through as appropriate.
How was this change tested?
Test-only change.