fix: correct invalid testing feature gate usage (#5258)

* fix: correct invalid testing feature gate usage

* test: refactor tests to avoid test code leak

* fix: sync main

src/cli/Cargo.toml

@@ -15,7 +15,7 @@ cache.workspace = true
 catalog.workspace = true
 chrono.workspace = true
 clap.workspace = true
-client.workspace = true
+client = { workspace = true, features = ["testing"] }
 common-base.workspace = true
 common-catalog.workspace = true
 common-config.workspace = true
@@ -56,7 +56,6 @@ tokio.workspace = true
 tracing-appender.workspace = true
 
 [dev-dependencies]
-client = { workspace = true, features = ["testing"] }
 common-test-util.workspace = true
 common-version.workspace = true
 serde.workspace = true

src/common/meta/src/key/schema_metadata_manager.rs

@@ -28,13 +28,10 @@ pub type SchemaMetadataManagerRef = Arc<SchemaMetadataManager>;
 pub struct SchemaMetadataManager {
     table_id_schema_cache: TableSchemaCacheRef,
     schema_cache: SchemaCacheRef,
-    #[cfg(any(test, feature = "testing"))]
-    kv_backend: crate::kv_backend::KvBackendRef,
 }
 
 impl SchemaMetadataManager {
     /// Creates a new database meta
-    #[cfg(not(any(test, feature = "testing")))]
     pub fn new(table_id_schema_cache: TableSchemaCacheRef, schema_cache: SchemaCacheRef) -> Self {
         Self {
             table_id_schema_cache,
@@ -42,20 +39,6 @@ impl SchemaMetadataManager {
         }
     }
 
-    /// Creates a new database meta
-    #[cfg(any(test, feature = "testing"))]
-    pub fn new(
-        kv_backend: crate::kv_backend::KvBackendRef,
-        table_id_schema_cache: TableSchemaCacheRef,
-        schema_cache: SchemaCacheRef,
-    ) -> Self {
-        Self {
-            table_id_schema_cache,
-            schema_cache,
-            kv_backend,
-        }
-    }
-
     /// Gets schema options by table id.
     pub async fn get_schema_options_by_table_id(
         &self,
@@ -80,6 +63,7 @@ impl SchemaMetadataManager {
         schema_name: &str,
         catalog_name: &str,
         schema_value: Option<crate::key::schema_name::SchemaNameValue>,
+        kv_backend: crate::kv_backend::KvBackendRef,
     ) {
         use table::metadata::{RawTableInfo, TableType};
         let value = crate::key::table_info::TableInfoValue::new(RawTableInfo {
@@ -91,19 +75,18 @@ impl SchemaMetadataManager {
             meta: Default::default(),
             table_type: TableType::Base,
         });
-        let table_info_manager =
-            crate::key::table_info::TableInfoManager::new(self.kv_backend.clone());
+        let table_info_manager = crate::key::table_info::TableInfoManager::new(kv_backend.clone());
         let (txn, _) = table_info_manager
             .build_create_txn(table_id, &value)
             .unwrap();
-        let resp = self.kv_backend.txn(txn).await.unwrap();
+        let resp = kv_backend.txn(txn).await.unwrap();
         assert!(resp.succeeded, "Failed to create table metadata");
         let key = crate::key::schema_name::SchemaNameKey {
             catalog: catalog_name,
             schema: schema_name,
         };
 
-        crate::key::schema_name::SchemaManager::new(self.kv_backend.clone())
+        crate::key::schema_name::SchemaManager::new(kv_backend.clone())
             .create(key, schema_value, false)
             .await
             .expect("Failed to create schema metadata");

src/datanode/src/datanode.rs

@@ -224,7 +224,6 @@ impl DatanodeBuilder {
             cache_registry.get().context(MissingCacheSnafu)?;
 
         let schema_metadata_manager = Arc::new(SchemaMetadataManager::new(
-            kv_backend.clone(),
             table_id_schema_cache,
             schema_cache,
         ));

src/mito2/src/compaction.rs

@@ -695,7 +695,7 @@ mod tests {
         let (tx, _rx) = mpsc::channel(4);
         let mut scheduler = env.mock_compaction_scheduler(tx);
         let mut builder = VersionControlBuilder::new();
-        let schema_metadata_manager = mock_schema_metadata_manager();
+        let (schema_metadata_manager, kv_backend) = mock_schema_metadata_manager();
         schema_metadata_manager
             .register_region_table_info(
                 builder.region_id().table_id(),
@@ -703,6 +703,7 @@ mod tests {
                 "test_catalog",
                 "test_schema",
                 None,
+                kv_backend,
             )
             .await;
         // Nothing to compact.
@@ -759,7 +760,7 @@ mod tests {
         let purger = builder.file_purger();
         let region_id = builder.region_id();
 
-        let schema_metadata_manager = mock_schema_metadata_manager();
+        let (schema_metadata_manager, kv_backend) = mock_schema_metadata_manager();
         schema_metadata_manager
             .register_region_table_info(
                 builder.region_id().table_id(),
@@ -767,6 +768,7 @@ mod tests {
                 "test_catalog",
                 "test_schema",
                 None,
+                kv_backend,
             )
             .await;
 

src/mito2/src/engine/alter_test.rs

@@ -116,6 +116,7 @@ async fn test_alter_region() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 
@@ -210,6 +211,7 @@ async fn test_put_after_alter() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 
@@ -315,6 +317,7 @@ async fn test_alter_region_retry() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 
@@ -374,6 +377,7 @@ async fn test_alter_on_flushing() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 
@@ -477,6 +481,7 @@ async fn test_alter_column_fulltext_options() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 
@@ -594,6 +599,7 @@ async fn test_alter_region_ttl_options() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
     engine
@@ -644,6 +650,7 @@ async fn test_write_stall_on_altering() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 

src/mito2/src/engine/append_mode_test.rs

@@ -104,6 +104,7 @@ async fn test_append_mode_compaction() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 

src/mito2/src/engine/compaction_test.rs

@@ -119,6 +119,7 @@ async fn test_compaction_region() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 
@@ -190,6 +191,7 @@ async fn test_compaction_region_with_overlapping() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 
@@ -245,6 +247,7 @@ async fn test_compaction_region_with_overlapping_delete_all() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 
@@ -319,6 +322,7 @@ async fn test_readonly_during_compaction() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 
@@ -390,6 +394,7 @@ async fn test_compaction_update_time_window() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 

src/mito2/src/engine/drop_test.rs

@@ -17,6 +17,7 @@ use std::time::Duration;
 
 use api::v1::Rows;
 use common_meta::key::SchemaMetadataManager;
+use common_meta::kv_backend::KvBackendRef;
 use object_store::util::join_path;
 use store_api::region_engine::RegionEngine;
 use store_api::region_request::{RegionDropRequest, RegionRequest};
@@ -49,6 +50,7 @@ async fn test_engine_drop_region() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 
@@ -102,6 +104,7 @@ async fn test_engine_drop_region_for_custom_store() {
     async fn setup(
         engine: &MitoEngine,
         schema_metadata_manager: &SchemaMetadataManager,
+        kv_backend: &KvBackendRef,
         region_id: RegionId,
         storage_name: &str,
     ) {
@@ -123,6 +126,7 @@ async fn test_engine_drop_region_for_custom_store() {
                 "test_catalog",
                 "test_schema",
                 None,
+                kv_backend.clone(),
             )
             .await;
 
@@ -145,17 +149,26 @@ async fn test_engine_drop_region_for_custom_store() {
         .await;
     let schema_metadata_manager = env.get_schema_metadata_manager();
     let object_store_manager = env.get_object_store_manager().unwrap();
+    let kv_backend = env.get_kv_backend();
 
     let global_region_id = RegionId::new(1, 1);
     setup(
         &engine,
         &schema_metadata_manager,
+        &kv_backend,
         global_region_id,
         "default",
     )
     .await;
     let custom_region_id = RegionId::new(2, 1);
-    setup(&engine, &schema_metadata_manager, custom_region_id, "Gcs").await;
+    setup(
+        &engine,
+        &schema_metadata_manager,
+        &kv_backend,
+        custom_region_id,
+        "Gcs",
+    )
+    .await;
 
     let global_region = engine.get_region(global_region_id).unwrap();
     let global_region_dir = global_region.access_layer.region_dir().to_string();

src/mito2/src/engine/edit_region_test.rs

@@ -72,6 +72,7 @@ async fn test_edit_region_schedule_compaction() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
     engine

src/mito2/src/engine/filter_deleted_test.rs

@@ -40,6 +40,7 @@ async fn test_scan_without_filtering_deleted() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
     let request = CreateRequestBuilder::new()

src/mito2/src/engine/flush_test.rs

@@ -52,6 +52,7 @@ async fn test_manual_flush() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 
@@ -109,6 +110,7 @@ async fn test_flush_engine() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 
@@ -178,6 +180,7 @@ async fn test_write_stall() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
     let request = CreateRequestBuilder::new().build();
@@ -251,6 +254,7 @@ async fn test_flush_empty() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
     let request = CreateRequestBuilder::new().build();
@@ -295,6 +299,7 @@ async fn test_flush_reopen_region(factory: Option<LogStoreFactory>) {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 
@@ -415,6 +420,7 @@ async fn test_auto_flush_engine() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 
@@ -484,6 +490,7 @@ async fn test_flush_workers() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 

src/mito2/src/engine/merge_mode_test.rs

@@ -104,6 +104,7 @@ async fn test_merge_mode_compaction() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 

src/mito2/src/engine/open_test.rs

@@ -252,6 +252,7 @@ async fn test_open_region_skip_wal_replay() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 
@@ -441,6 +442,7 @@ async fn test_open_compaction_region() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
     let request = CreateRequestBuilder::new().build();

src/mito2/src/engine/parallel_test.rs

@@ -84,6 +84,7 @@ async fn test_parallel_scan() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 

src/mito2/src/engine/prune_test.rs

@@ -159,6 +159,7 @@ async fn test_prune_memtable() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 

src/mito2/src/engine/row_selector_test.rs

@@ -36,6 +36,7 @@ async fn test_last_row(append_mode: bool) {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
     let request = CreateRequestBuilder::new()

src/mito2/src/engine/truncate_test.rs

@@ -159,6 +159,7 @@ async fn test_engine_truncate_after_flush() {
             "test_catalog",
             "test_schema",
             None,
+            env.get_kv_backend(),
         )
         .await;
 

src/mito2/src/test_util.rs

@@ -201,6 +201,7 @@ pub struct TestEnv {
     log_store_factory: LogStoreFactory,
     object_store_manager: Option<ObjectStoreManagerRef>,
     schema_metadata_manager: SchemaMetadataManagerRef,
+    kv_backend: KvBackendRef,
 }
 
 impl Default for TestEnv {
@@ -212,37 +213,40 @@ impl Default for TestEnv {
 impl TestEnv {
     /// Returns a new env with empty prefix for test.
     pub fn new() -> TestEnv {
-        let schema_metadata_manager = mock_schema_metadata_manager();
+        let (schema_metadata_manager, kv_backend) = mock_schema_metadata_manager();
         TestEnv {
             data_home: create_temp_dir(""),
             log_store: None,
             log_store_factory: LogStoreFactory::RaftEngine(RaftEngineLogStoreFactory),
             object_store_manager: None,
             schema_metadata_manager,
+            kv_backend,
         }
     }
 
     /// Returns a new env with specific `prefix` for test.
     pub fn with_prefix(prefix: &str) -> TestEnv {
-        let schema_metadata_manager = mock_schema_metadata_manager();
+        let (schema_metadata_manager, kv_backend) = mock_schema_metadata_manager();
         TestEnv {
             data_home: create_temp_dir(prefix),
             log_store: None,
             log_store_factory: LogStoreFactory::RaftEngine(RaftEngineLogStoreFactory),
             object_store_manager: None,
             schema_metadata_manager,
+            kv_backend,
         }
     }
 
     /// Returns a new env with specific `data_home` for test.
     pub fn with_data_home(data_home: TempDir) -> TestEnv {
-        let schema_metadata_manager = mock_schema_metadata_manager();
+        let (schema_metadata_manager, kv_backend) = mock_schema_metadata_manager();
         TestEnv {
             data_home,
             log_store: None,
             log_store_factory: LogStoreFactory::RaftEngine(RaftEngineLogStoreFactory),
             object_store_manager: None,
             schema_metadata_manager,
+            kv_backend,
         }
     }
 
@@ -653,6 +657,10 @@ impl TestEnv {
     pub fn get_schema_metadata_manager(&self) -> SchemaMetadataManagerRef {
         self.schema_metadata_manager.clone()
     }
+
+    pub fn get_kv_backend(&self) -> KvBackendRef {
+        self.kv_backend.clone()
+    }
 }
 
 /// Builder to mock a [RegionCreateRequest].
@@ -1143,7 +1151,7 @@ pub async fn reopen_region(
     }
 }
 
-pub(crate) fn mock_schema_metadata_manager() -> Arc<SchemaMetadataManager> {
+pub(crate) fn mock_schema_metadata_manager() -> (Arc<SchemaMetadataManager>, KvBackendRef) {
     let kv_backend = Arc::new(MemoryKvBackend::new());
     let table_schema_cache = Arc::new(new_table_schema_cache(
         "table_schema_name_cache".to_string(),
@@ -1155,9 +1163,8 @@ pub(crate) fn mock_schema_metadata_manager() -> Arc<SchemaMetadataManager> {
         CacheBuilder::default().build(),
         kv_backend.clone(),
     ));
-    Arc::new(SchemaMetadataManager::new(
+    (
+        Arc::new(SchemaMetadataManager::new(table_schema_cache, schema_cache)),
         kv_backend as KvBackendRef,
-        table_schema_cache,
-        schema_cache,
-    ))
+    )
 }