liurenjie1024 commented on code in PR #428:
URL: https://github.com/apache/iceberg-rust/pull/428#discussion_r1668547755
##########
crates/catalog/hms/tests/hms_catalog_test.rs:
##########
@@ -106,23 +124,18 @@ fn set_table_creation(location: impl ToString, name: impl
ToString) -> Result<Ta
#[tokio::test]
async fn test_rename_table() -> Result<()> {
- let fixture = set_test_fixture("test_rename_table").await;
- let creation = set_table_creation("s3a://warehouse/hive", "my_table")?;
- let namespace = Namespace::new(NamespaceIdent::new("default".into()));
+ let catalog = get_catalog().await;
+ let creation: TableCreation = set_table_creation("s3a://warehouse/hive",
"my_table")?;
+ let namespace =
Namespace::new(NamespaceIdent::new("test_rename_table".into()));
+ set_test_namespace(&catalog, namespace.name()).await?;
Review Comment:
This is extra step compared with previous one?
##########
crates/catalog/hms/tests/hms_catalog_test.rs:
##########
@@ -131,16 +144,14 @@ async fn test_rename_table() -> Result<()> {
#[tokio::test]
async fn test_table_exists() -> Result<()> {
- let fixture = set_test_fixture("test_table_exists").await;
+ let catalog = get_catalog().await;
let creation = set_table_creation("s3a://warehouse/hive", "my_table")?;
- let namespace = Namespace::new(NamespaceIdent::new("default".into()));
+ let namespace =
Namespace::new(NamespaceIdent::new("test_table_exists".into()));
+ set_test_namespace(&catalog, namespace.name()).await?;
Review Comment:
Ditto?
##########
crates/catalog/glue/tests/glue_catalog_test.rs:
##########
@@ -387,18 +354,13 @@ async fn test_create_namespace() -> Result<()> {
#[tokio::test]
async fn test_list_namespace() -> Result<()> {
- let fixture = set_test_fixture("test_list_namespace").await;
+ let catalog = get_catalog().await;
- let expected = vec![];
Review Comment:
Seems we missed empty namespace check?
##########
crates/catalog/hms/tests/hms_catalog_test.rs:
##########
@@ -169,17 +178,14 @@ async fn test_drop_table() -> Result<()> {
#[tokio::test]
async fn test_load_table() -> Result<()> {
- let fixture = set_test_fixture("test_load_table").await;
+ let catalog = get_catalog().await;
let creation = set_table_creation("s3a://warehouse/hive", "my_table")?;
- let namespace = Namespace::new(NamespaceIdent::new("default".into()));
+ let namespace =
Namespace::new(NamespaceIdent::new("test_load_table".into()));
+ set_test_namespace(&catalog, namespace.name()).await?;
Review Comment:
Ditto?
##########
crates/catalog/hms/tests/hms_catalog_test.rs:
##########
@@ -195,22 +201,19 @@ async fn test_load_table() -> Result<()> {
#[tokio::test]
async fn test_create_table() -> Result<()> {
- let fixture = set_test_fixture("test_create_table").await;
+ let catalog = get_catalog().await;
let creation = set_table_creation("s3a://warehouse/hive", "my_table")?;
- let namespace = Namespace::new(NamespaceIdent::new("default".into()));
+ let namespace =
Namespace::new(NamespaceIdent::new("test_create_table".into()));
+ set_test_namespace(&catalog, namespace.name()).await?;
Review Comment:
Ditto?
##########
crates/integrations/datafusion/tests/integration_datafusion_hms_test.rs:
##########
Review Comment:
The tests in this file still share same namespaces, we should also change
them?
##########
crates/catalog/hms/tests/hms_catalog_test.rs:
##########
@@ -149,18 +160,16 @@ async fn test_table_exists() -> Result<()> {
#[tokio::test]
async fn test_drop_table() -> Result<()> {
- let fixture = set_test_fixture("test_drop_table").await;
+ let catalog = get_catalog().await;
let creation = set_table_creation("s3a://warehouse/hive", "my_table")?;
- let namespace = Namespace::new(NamespaceIdent::new("default".into()));
+ let namespace =
Namespace::new(NamespaceIdent::new("test_drop_table".into()));
+ set_test_namespace(&catalog, namespace.name()).await?;
Review Comment:
Ditto?
##########
Cargo.toml:
##########
@@ -82,6 +82,7 @@ serde_repr = "0.1.16"
serde_with = "3.4.0"
tempfile = "3.8"
tokio = { version = "1", features = ["macros"] }
+tokio-shared-rt = "0.1.0"
Review Comment:
Should we remove this?
##########
crates/test_utils/Cargo.toml:
##########
@@ -27,6 +27,8 @@ license = { workspace = true }
[dependencies]
env_logger = { workspace = true }
+tokio = { workspace = true }
+async-trait = { workspace = true }
Review Comment:
We should remove this?
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
