Refactor test_cmd_garbage_collect_spares_files_within_grace_period (#2169)

Author: boraarslan

quickwit/quickwit-cli/tests/cli.rs

@@ -40,7 +40,7 @@ use quickwit_common::ChecklistError;
 use quickwit_config::service::QuickwitService;
 use quickwit_config::CLI_INGEST_SOURCE_ID;
 use quickwit_indexing::actors::INDEXING_DIR_NAME;
-use quickwit_metastore::{quickwit_metastore_uri_resolver, Metastore, MetastoreError};
+use quickwit_metastore::{quickwit_metastore_uri_resolver, Metastore, MetastoreError, SplitState};
 use serde_json::{json, Number, Value};
 use tokio::time::{sleep, Duration};
 
@@ -617,100 +617,97 @@ async fn test_garbage_collect_cli_no_grace() {
 }
 
 #[tokio::test]
-async fn test_cmd_garbage_collect_spares_files_within_grace_period() -> Result<()> {
+async fn test_garbage_collect_index_cli() {
     let index_id = append_random_suffix("test-gc-cmd");
-    let test_env = create_test_env(index_id, TestStorageType::LocalFileSystem)?;
+    let test_env = create_test_env(index_id.clone(), TestStorageType::LocalFileSystem).unwrap();
     create_logs_index(&test_env);
     ingest_docs(test_env.resource_files["logs"].as_path(), &test_env);
 
-    let metastore = test_env.metastore().await?;
-    let splits = metastore.list_all_splits(&test_env.index_id).await?;
+    let refresh_metastore = |metastore| {
+        // In this test we rely on the file backed metastore and
+        // modify it but the file backed metastore caches results.
+        // Therefore we need to force reading the disk to update split info.
+        //
+        // We do that by dropping and recreating our metastore.
+        drop(metastore);
+        quickwit_metastore_uri_resolver().resolve(&test_env.metastore_uri)
+    };
+
+    let create_gc_args = |grace_period_secs| GarbageCollectIndexArgs {
+        config_uri: test_env.config_uri.clone(),
+        index_id: index_id.clone(),
+        grace_period: Duration::from_secs(grace_period_secs),
+        dry_run: false,
+    };
+
+    let metastore = quickwit_metastore_uri_resolver()
+        .resolve(&test_env.metastore_uri)
+        .await
+        .unwrap();
+
+    let splits = metastore.list_all_splits(&test_env.index_id).await.unwrap();
     assert_eq!(splits.len(), 1);
-    make_command(
-        format!(
-            "index gc --index {} --config {}",
-            test_env.index_id,
-            test_env.resource_files["config"].display(),
-        )
-        .as_str(),
-    )
-    .assert()
-    .success()
-    .stdout(predicate::str::contains(
-        "No dangling files to garbage collect",
-    ));
 
     let index_path = test_env.indexes_dir_path.join(&test_env.index_id);
     let split_filename = quickwit_common::split_file(splits[0].split_metadata.split_id.as_str());
     let split_path = index_path.join(&split_filename);
     assert_eq!(split_path.exists(), true);
 
+    let args = create_gc_args(3600);
+
+    garbage_collect_index_cli(args).await.unwrap();
+
+    // Split should still exists within grace period.
+    let metastore = refresh_metastore(metastore).await.unwrap();
+    let splits = metastore.list_all_splits(&test_env.index_id).await.unwrap();
+    assert_eq!(splits.len(), 1);
+
     // The following steps help turn an existing published split into a staged one
     // without deleting the files.
     let split = splits[0].clone();
     let split_ids = [split.split_metadata.split_id.as_str()];
     metastore
         .mark_splits_for_deletion(&test_env.index_id, &split_ids)
-        .await?;
+        .await
+        .unwrap();
     metastore
         .delete_splits(&test_env.index_id, &split_ids)
-        .await?;
+        .await
+        .unwrap();
     metastore
         .stage_split(&test_env.index_id, split.split_metadata)
-        .await?;
+        .await
+        .unwrap();
     assert_eq!(split_path.exists(), true);
 
-    make_command(
-        format!(
-            "index gc --index {} --config {} --grace-period 2s",
-            test_env.index_id,
-            test_env.resource_files["config"].display(),
-        )
-        .as_str(),
-    )
-    .assert()
-    .success()
-    .stdout(predicate::str::contains(
-        "No dangling files to garbage collect",
-    ));
+    let metastore = refresh_metastore(metastore).await.unwrap();
+    let splits = metastore.list_all_splits(&test_env.index_id).await.unwrap();
+    assert_eq!(splits[0].split_state, SplitState::Staged);
+
+    let args = create_gc_args(1);
+
+    garbage_collect_index_cli(args).await.unwrap();
+
     assert_eq!(split_path.exists(), true);
+    // Staged splits should still exist within grace period.
+    let metastore = refresh_metastore(metastore).await.unwrap();
+    let splits = metastore.list_all_splits(&test_env.index_id).await.unwrap();
+    assert_eq!(splits.len(), 1);
+    assert_eq!(splits[0].split_state, SplitState::Staged);
 
     // Wait for grace period.
     // TODO: edit split update timestamps and remove this sleep.
-    sleep(Duration::from_secs(3)).await;
-    make_command(
-        format!(
-            "index gc --index {} --config {} --dry-run --grace-period 2s",
-            test_env.index_id,
-            test_env.resource_files["config"].display(),
-        )
-        .as_str(),
-    )
-    .assert()
-    .success()
-    .stdout(predicate::str::contains(
-        "The following files will be garbage collected.",
-    ))
-    .stdout(predicate::str::contains(&split_filename));
-    assert_eq!(split_path.exists(), true);
+    sleep(Duration::from_secs(2)).await;
 
-    make_command(
-        format!(
-            "index gc --index {} --config {} --grace-period 2s",
-            test_env.index_id,
-            test_env.resource_files["config"].display(),
-        )
-        .as_str(),
-    )
-    .assert()
-    .success()
-    .stdout(predicate::str::contains(format!(
-        "Index `{}` successfully garbage collected",
-        test_env.index_id
-    )));
-    assert_eq!(split_path.exists(), false);
+    let args = create_gc_args(1);
 
-    Ok(())
+    garbage_collect_index_cli(args).await.unwrap();
+
+    let metastore = refresh_metastore(metastore).await.unwrap();
+    let splits = metastore.list_all_splits(&test_env.index_id).await.unwrap();
+    // Splits should be deleted from both metastore and file system.
+    assert_eq!(splits.len(), 0);
+    assert_eq!(split_path.exists(), false);
 }
 
 #[tokio::test]