-
Notifications
You must be signed in to change notification settings - Fork 23
Kv Table Integration tests #192
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
Conversation
|
@luoyuxia @fresh-borzoni @AndreaBozzo Appreciate your reviews here |
fresh-borzoni
left a comment
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.
@leekeiabstraction Thanks for the PR. Left minor comments.
PTAL
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 pull request adds comprehensive integration tests for KV (Key-Value) table operations in the Fluss client library. The tests verify core KV table functionality including upsert, lookup, update, delete operations, and advanced features like composite primary keys and partial column updates.
Changes:
- Added new integration test module
kv_table.rswith 6 test cases covering all major KV table operations - Fixed unnecessary
mutqualifier on unused variable intable.rs - Registered the new test module in the test suite
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/fluss/tests/test_fluss.rs | Added kv_table module to the integration test suite |
| crates/fluss/tests/integration/table.rs | Removed unnecessary mut qualifier from all_ids variable that is never mutated |
| crates/fluss/tests/integration/kv_table.rs | New comprehensive integration test suite for KV table operations including upsert, lookup, update, delete, composite keys, and partial updates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@fresh-borzoni TY for your review. Updated, PTAL! |
luoyuxia
left a comment
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.
@leekeiabstraction Thanks for the pr. Left minor commet
|
Thank you @luoyuxia for the review. Addressed your comment, PTAL! |
fresh-borzoni
left a comment
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.
@leekeiabstraction LGTM.
Thank you for the PR
luoyuxia
left a comment
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.
@leekeiabstraction Thanks. LGTM
|
@leekeiabstraction The test fails |
|
@luoyuxia Thank you, I've updated the polling logic to wait for Tablet server availability as well. PTAL |
Purpose
Linked issue: close #182
Brief change log
Add integration tests for KvTable"
Tests
Test runs locally successfully