test: add more test cases to improve test coverage for the write functionality in Lance #5619
+653
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lichuang
force-pushed
the
more-test-write
branch
2 times, most recently
from
1304652 to
8335288
Compare
wjones127
requested changes
Jan 5, 2026
Contributor
wjones127
left a comment
wjones127
left a comment
There was a problem hiding this comment.
Also changed the type in the PR title to test.
rust/lance/src/dataset/write.rs
Outdated
Comment on lines
1366
to
1367
| #[tokio::test] | ||
| async fn test_stable_row_ids() { |
Contributor
There was a problem hiding this comment.
I think this is already covered in test_new_row_ids and test_row_ids_append, right? If you think there's something missing in those test cases, can you add them there?
wjones127
changed the title
…ctionality in Lance
lichuang
force-pushed
the
more-test-write
branch
from
8335288 to
0f0d455
Compare
wjones127
reviewed
Jan 6, 2026
Comment on lines
+2381
to
+2402
| // Empty stream should be handled gracefully | ||
| // It should create an empty dataset or return an appropriate result | ||
| match result { | ||
| Ok((fragments, _)) => { | ||
| // If successful, verify it creates an empty result | ||
| assert!( | ||
| fragments.is_empty(), | ||
| "Empty stream should create no fragments" | ||
| ); | ||
| } | ||
| Err(e) => { | ||
| // If it returns an error, verify it's an appropriate error type | ||
| let error_msg = e.to_string(); | ||
| assert!( | ||
| error_msg.contains("empty") | ||
| || error_msg.contains("no data") | ||
| || error_msg.contains("batch"), | ||
| "Error should mention empty or no data: {}", | ||
| error_msg | ||
| ); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
We should test for one behavior; as it would be a breaking change to have one behavior and change to another.
An empty stream should output empty fragments.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds four new test cases to improve test coverage for the write functionality in Lance:
Changes
test_max_rows_per_file- Tests themax_rows_per_fileparameter behaviortest_max_rows_per_group- Testsmax_rows_per_groupparameter across different Lance file versionstest_empty_stream_write: Verifies graceful handling of empty input streams to prevent unexpected panics or cryptic errors.
test_schema_mismatch_on_append: Ensures clear error messages and data integrity when attempting to append data with incompatible schemas.
test_disk_full_error: Validates proper error propagation for storage-related failures to help users quickly identify and debug disk space issues.
test_write_interruption_recovery: Tests the complete transaction flow for interrupted writes, ensuring dataset consistency, data integrity, and successful retry capability.
Motivation
These tests improve confidence in the write functionality by covering important parameters and features that were previously untested or under-tested. They help prevent regressions and ensure correct behavior across different Lance file versions.