improve test

Author: OussamaSaoudi

kernel/src/log_reader/manifest.rs

@@ -5,10 +5,11 @@ use std::sync::{Arc, LazyLock};
 use itertools::Itertools;
 use url::Url;
 
-use crate::actions::{get_commit_schema, ADD_NAME};
-use crate::actions::{visitors::SidecarVisitor, SIDECAR_NAME};
+use crate::actions::visitors::SidecarVisitor;
+use crate::actions::SIDECAR_NAME;
+use crate::actions::{Add, Sidecar, ADD_NAME};
 use crate::log_replay::ActionsBatch;
-use crate::schema::SchemaRef;
+use crate::schema::{SchemaRef, StructField, StructType, ToSchema};
 use crate::utils::require;
 use crate::{DeltaResult, Engine, Error, FileMeta, RowVisitor};
 
@@ -49,11 +50,11 @@ impl ManifestPhase {
         log_root: Url,
         engine: Arc<dyn Engine>,
     ) -> DeltaResult<Self> {
-        #[allow(clippy::unwrap_used)]
         static MANIFEST_READ_SCHMEA: LazyLock<SchemaRef> = LazyLock::new(|| {
-            get_commit_schema()
-                .project(&[ADD_NAME, SIDECAR_NAME])
-                .unwrap()
+            Arc::new(StructType::new_unchecked([
+                StructField::nullable(ADD_NAME, Add::to_schema()),
+                StructField::nullable(SIDECAR_NAME, Sidecar::to_schema()),
+            ]))
         });
 
         let files = vec![manifest_file.clone()];
@@ -148,43 +149,37 @@ impl Iterator for ManifestPhase {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::engine::default::executor::tokio::TokioBackgroundExecutor;
     use crate::engine::default::DefaultEngine;
     use crate::log_replay::LogReplayProcessor;
     use crate::scan::log_replay::ScanLogReplayProcessor;
     use crate::scan::state_info::StateInfo;
+    use crate::SnapshotRef;
     use object_store::local::LocalFileSystem;
-    use std::path::PathBuf;
-    use std::sync::Arc as StdArc;
+    use std::sync::Arc;
+    use tempfile::TempDir;
 
     fn load_test_table(
         table_name: &str,
-    ) -> DeltaResult<(
-        StdArc<DefaultEngine<TokioBackgroundExecutor>>,
-        StdArc<crate::Snapshot>,
-        url::Url,
-    )> {
-        let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
-        path.push("tests/data");
-        path.push(table_name);
-
-        let path = std::fs::canonicalize(path)
-            .map_err(|e| crate::Error::Generic(format!("Failed to canonicalize path: {}", e)))?;
-
-        let url = url::Url::from_directory_path(path)
+    ) -> DeltaResult<(Arc<dyn Engine>, SnapshotRef, Url, TempDir)> {
+        let test_dir = test_utils::load_test_data("tests/data", table_name)
+            .map_err(|e| crate::Error::Generic(format!("Failed to load test data: {}", e)))?;
+        let test_path = test_dir.path().join(table_name);
+
+        let url = url::Url::from_directory_path(&test_path)
             .map_err(|_| crate::Error::Generic("Failed to create URL from path".to_string()))?;
 
-        let store = StdArc::new(LocalFileSystem::new());
-        let engine = StdArc::new(DefaultEngine::new(store));
+        let store = Arc::new(LocalFileSystem::new());
+        let engine = Arc::new(DefaultEngine::new(store));
         let snapshot = crate::Snapshot::builder_for(url.clone()).build(engine.as_ref())?;
 
-        Ok((engine, snapshot, url))
+        Ok((engine, snapshot, url, test_dir))
     }
 
     #[test]
     fn test_manifest_phase_with_checkpoint() -> DeltaResult<()> {
         // Use a table with v2 checkpoints where adds might be in sidecars
-        let (engine, snapshot, log_root) = load_test_table("v2-checkpoints-json-with-sidecars")?;
+        let (engine, snapshot, log_root, _tempdir) =
+            load_test_table("v2-checkpoints-json-with-sidecars")?;
         let log_segment = snapshot.log_segment();
 
         // Check if there are any checkpoint parts
@@ -193,7 +188,7 @@ mod tests {
             return Ok(());
         }
 
-        let state_info = StdArc::new(StateInfo::try_new(
+        let state_info = Arc::new(StateInfo::try_new(
             snapshot.schema(),
             snapshot.table_configuration(),
             None,
@@ -210,10 +205,9 @@ mod tests {
             ManifestPhase::new(manifest_file, log_root.clone(), engine.clone())?;
 
         // Count batches and collect results
-        let mut batch_count = 0;
         let mut file_paths = Vec::new();
 
-        while let Some(result) = manifest_phase.next() {
+        for result in manifest_phase {
             let batch = result?;
             let metadata = processor.process_actions_batch(batch)?;
             let paths = metadata.visit_scan_files(
@@ -223,18 +217,8 @@ mod tests {
                 },
             )?;
             file_paths.extend(paths);
-            batch_count += 1;
         }
-
-        // For v2 checkpoints with sidecars, the manifest might not contain adds directly.
-        // In this test table, all adds are in sidecars, so manifest should be empty.
-        assert_eq!(
-            batch_count, 1,
-            "Single manifest file should produce exactly 1 batch"
-        );
-
         // Verify the manifest itself contains no add files (they're all in sidecars)
-        file_paths.sort();
         assert_eq!(
             file_paths.len(), 0,
             "For this v2 checkpoint with sidecars, manifest should contain 0 add files (all in sidecars)"
@@ -245,7 +229,8 @@ mod tests {
 
     #[test]
     fn test_manifest_phase_collects_sidecars() -> DeltaResult<()> {
-        let (engine, snapshot, log_root) = load_test_table("v2-checkpoints-json-with-sidecars")?;
+        let (engine, snapshot, log_root, _tempdir) =
+            load_test_table("v2-checkpoints-json-with-sidecars")?;
         let log_segment = snapshot.log_segment();
 
         if log_segment.checkpoint_parts.is_empty() {
@@ -256,8 +241,6 @@ mod tests {
         let checkpoint_file = &log_segment.checkpoint_parts[0];
         let manifest_file = checkpoint_file.location.clone();
 
-        let schema = crate::actions::get_commit_schema().project(&[crate::actions::ADD_NAME])?;
-
         let mut manifest_phase =
             ManifestPhase::new(manifest_file, log_root.clone(), engine.clone())?;