Copilot commented on code in PR #32:
URL: https://github.com/apache/fluss-rust/pull/32#discussion_r2434417825
##########
crates/fluss/tests/integration/admin.rs:
##########
@@ -126,6 +129,169 @@ mod admin_test {
#[tokio::test]
async fn test_create_table() {
- // todo
+ let cluster = get_fluss_cluster();
+ let connection = cluster.get_fluss_connection().await;
+ let admin = connection
+ .get_admin()
+ .await
+ .expect("Failed to get admin client");
+
+ let test_db_name = "test_create_table_db";
+ let db_descriptor = DatabaseDescriptorBuilder::default()
+ .comment("Database for test_create_table")
+ .build();
+
+ admin.drop_database(test_db_name, true, true);
+ admin
+ .create_database(test_db_name, false, Some(&db_descriptor))
+ .await;
Review Comment:
The result of `create_database(...).await` is ignored; `Result` is
`#[must_use]` and ignoring it can hide failures. Consider unwrapping with
context: `.await.expect(\"Failed to create test database\")`.
```suggestion
.await
.expect("Failed to create test database");
```
##########
crates/fluss/tests/integration/admin.rs:
##########
@@ -126,6 +129,169 @@ mod admin_test {
#[tokio::test]
async fn test_create_table() {
- // todo
+ let cluster = get_fluss_cluster();
+ let connection = cluster.get_fluss_connection().await;
+ let admin = connection
+ .get_admin()
+ .await
+ .expect("Failed to get admin client");
+
+ let test_db_name = "test_create_table_db";
+ let db_descriptor = DatabaseDescriptorBuilder::default()
+ .comment("Database for test_create_table")
+ .build();
+
+ admin.drop_database(test_db_name, true, true);
+ admin
Review Comment:
The initial database drop is not awaited, so the operation never executes.
This can lead to flakiness if the DB already exists. Await and handle the
result, e.g., `admin.drop_database(test_db_name, true,
true).await.expect(\"Failed to drop existing test DB\")`.
```suggestion
admin
.drop_database(test_db_name, true, true)
.await
.expect("Failed to drop existing test DB");
admin
```
##########
crates/fluss/tests/integration/admin.rs:
##########
@@ -126,6 +129,169 @@ mod admin_test {
#[tokio::test]
async fn test_create_table() {
- // todo
+ let cluster = get_fluss_cluster();
+ let connection = cluster.get_fluss_connection().await;
+ let admin = connection
+ .get_admin()
+ .await
+ .expect("Failed to get admin client");
+
+ let test_db_name = "test_create_table_db";
+ let db_descriptor = DatabaseDescriptorBuilder::default()
+ .comment("Database for test_create_table")
+ .build();
+
+ admin.drop_database(test_db_name, true, true);
+ admin
+ .create_database(test_db_name, false, Some(&db_descriptor))
+ .await;
+
+ let test_table_name = "test_user_table";
+ let table_path = TablePath::new(test_db_name.to_string(),
test_table_name.to_string());
+
+ // build table schema
+ let table_schema = Schema::builder()
+ .column("id", DataTypes::int())
+ .column("name", DataTypes::string())
+ .column("age", DataTypes::int())
+ .with_comment("User's age (optional)")
+ .column("email", DataTypes::string())
+ .primary_key_named("PK_test_user_table", vec!["id".to_string()])
+ .build()
+ .expect("Failed to build table schema");
+
+ // build table descriptor
+ let table_descriptor = TableDescriptor::builder()
+ .schema(table_schema.clone())
+ .comment("Test table for user data (id, name, age, email)")
+ .distributed_by(Some(3), vec!["id".to_string()])
+ .property("table.replication.factor", "1")
+ .log_format(LogFormat::ARROW)
+ .kv_format(KvFormat::INDEXED)
+ .build()
+ .expect("Failed to build table descriptor");
+
+ // create test table
+ admin
+ .create_table(&table_path, &table_descriptor, false)
+ .await
+ .expect("Failed to create test table");
+
+ assert!(
+ admin.table_exists(&table_path).await.unwrap(),
+ "Table {:?} should exist after creation",
+ table_path
+ );
+
+ let tables = admin.list_tables(test_db_name).await.unwrap();
+ assert!(
+ tables.len() == 1,
+ "There should be exactly one table in the database"
+ );
+ assert!(
+ tables.contains(&test_table_name.to_string()),
+ "Table list should contain the created table"
+ );
+
+ let table_info = admin
+ .get_table(&table_path)
+ .await
+ .expect("Failed to get table info");
+
+ // verify table comment
+ assert_eq!(
+ table_info.get_comment(),
+ Some("Test table for user data (id, name, age, email)"),
+ "Table comment mismatch"
+ );
+
+ // verify schema columns
+ let actual_columns = table_info.get_schema().columns();
+ let expected_columns = vec![
+ ("id", DataTypes::int().as_non_nullable(), None),
+ ("name", DataTypes::string(), None),
+ ("age", DataTypes::int(), Some("User's age (optional)")),
+ ("email", DataTypes::string(), None),
+ ];
+
+ assert_eq!(
+ actual_columns.len(),
+ expected_columns.len(),
+ "Number of columns mismatch"
+ );
+ for (idx, (exp_name, exp_type, exp_comment)) in
expected_columns.into_iter().enumerate() {
+ let actual_col = &actual_columns[idx];
+ assert_eq!(
+ actual_col.name(),
+ exp_name,
+ "Column name mismatch at index {}",
+ idx
+ );
+ assert_eq!(
+ actual_col.data_type(),
+ &exp_type,
+ "Column type mismatch for {}",
+ exp_name
+ );
+ assert_eq!(
+ actual_col.comment(),
+ exp_comment,
+ "Column comment mismatch for {}",
+ exp_name
+ );
+ }
+
+ // verify primary key
+ assert_eq!(
+ table_info.get_primary_keys(),
+ &vec!["id".to_string()],
+ "Primary key columns mismatch"
+ );
+ assert_eq!(
+ table_info
+ .get_schema()
+ .primary_key()
+ .unwrap()
+ .constraint_name(),
+ "PK_id",
Review Comment:
Primary key constraint name assertion does not match the name used when
building the schema (`primary_key_named(\"PK_test_user_table\", ...)`). Update
the expected value to `\"PK_test_user_table\"` or change the builder to use
`\"PK_id\"` for consistency.
```suggestion
"PK_test_user_table",
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
