-
Notifications
You must be signed in to change notification settings - Fork 22
chore: improve unit test coverage #173
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
|
@luoyuxia Hi, this pr is ready. PTAL if you have time. |
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.
Pull request overview
This PR significantly improves unit test coverage across the Fluss Rust codebase by adding comprehensive test suites for core modules and client functionality. The changes are almost entirely test additions, with only one functional change to implement actual checksum validation.
Changes:
- Implemented checksum validation in
LogRecordBatch::ensure_valid()(previously a TODO placeholder) - Added extensive test utilities including mock connection helpers and test data builders
- Added comprehensive test coverage for scanner, metadata, credentials, cluster, admin, and FFI modules
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/fluss/src/test_utils.rs | Added test utilities including TestCompletedFetch, mock connection builders, and helper functions for creating test data |
| crates/fluss/src/rpc/transport.rs | Added Test variant to Transport enum for test support with proper cfg annotations |
| crates/fluss/src/rpc/server_connection.rs | Added spawn_mock_server helper and insert_connection_for_test method |
| crates/fluss/src/rpc/mod.rs | Exported test-only ApiKey and Transport types |
| crates/fluss/src/record/arrow.rs | Implemented checksum validation in ensure_valid() and added 6 new test cases |
| crates/fluss/src/metadata/table.rs | Added 13 test cases covering schema, table descriptor, and table info functionality |
| crates/fluss/src/cluster/cluster.rs | Added 6 test cases for cluster construction and metadata handling |
| crates/fluss/src/client/table/scanner.rs | Added 30+ test cases for scanner functionality including error handling and fetching logic |
| crates/fluss/src/client/table/remote_log.rs | Added 8 test cases for remote log operations and download futures |
| crates/fluss/src/client/table/log_fetch_buffer.rs | Added 10 test cases for fetch buffer management and error propagation |
| crates/fluss/src/client/mod.rs | Exported test-only types for internal testing |
| crates/fluss/src/client/table/mod.rs | Changed log_fetch_buffer visibility to pub(crate) for test access |
| crates/fluss/src/client/metadata.rs | Added 5 test cases for metadata operations and cluster updates |
| crates/fluss/src/client/credentials.rs | Added 2 test cases for credentials cache and token handling |
| crates/fluss/src/client/connection.rs | Added 3 test cases for connection management and admin creation |
| crates/fluss/src/client/admin.rs | Added 2 comprehensive test cases covering all admin API operations |
| bindings/cpp/src/types.rs | Added 3 test cases for FFI type conversions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Purpose
Linked issue: close #xxx
Brief change log
Tests
API and Format
Documentation