review & nits

sebastiantia · Feb 19, 2025 · 038d3e7 · 038d3e7
1 parent 57d4dc7
commit 038d3e7
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 13 deletions.
diff --git a/kernel/src/scan/mod.rs b/kernel/src/scan/mod.rs
@@ -699,6 +699,7 @@ pub(crate) mod test_utils {
     ) -> Box<ArrowEngineData> {
         let handler = SyncJsonHandler {};
 
+        let json_strings = paths
         let mut json_strings: Vec<String> = paths
         .iter()
         .map(|path| {

diff --git a/kernel/src/table_configuration.rs b/kernel/src/table_configuration.rs
@@ -262,8 +262,12 @@ mod test {
         let protocol = Protocol::try_new(
             3,
             7,
-            Some([ReaderFeatures::UnsupportedFeature]),
-            Some([WriterFeatures::UnsupportedFeature]),
+            Some([ReaderFeatures::UnrecognizedReaderFeature(
+                "unrecognizedReaderFeature".to_string(),
+            )]),
+            Some([WriterFeatures::UnrecognizedWriterFeature(
+                "unrecognizedWriterFeature".to_string(),
+            )]),
         )
         .unwrap();
         let table_root = Url::try_from("file:///").unwrap();

diff --git a/kernel/src/table_features/mod.rs b/kernel/src/table_features/mod.rs
@@ -49,7 +49,8 @@ pub enum ReaderFeatures {
     /// protocol checks during VACUUM operations
     VacuumProtocolCheck,
     /// A dummy variant used to represent an unsupported feature for testing purposes
-    UnsupportedFeature,
+    #[cfg(test)]
+    UnrecognizedReaderFeature(String),
 }
 
 /// Similar to reader features, writer features communicate capabilities that must be implemented
@@ -112,7 +113,8 @@ pub enum WriterFeatures {
     /// protocol checks during VACUUM operations
     VacuumProtocolCheck,
     /// A dummy variant used to represent an unsupported feature for testing purposes
-    UnsupportedFeature,
+    #[cfg(test)]
+    UnrecognizedWriterFeature(String),
 }
 
 impl From<ReaderFeatures> for String {
@@ -149,6 +151,7 @@ pub(crate) static SUPPORTED_WRITER_FEATURES: LazyLock<HashSet<WriterFeatures>> =
 mod tests {
     use super::*;
 
+    // TODO: Test the UnrecognizedReaderFeature variant
     #[test]
     fn test_roundtrip_reader_features() {
         let cases = [
@@ -159,10 +162,9 @@ mod tests {
             (ReaderFeatures::TypeWideningPreview, "typeWidening-preview"),
             (ReaderFeatures::V2Checkpoint, "v2Checkpoint"),
             (ReaderFeatures::VacuumProtocolCheck, "vacuumProtocolCheck"),
-            (ReaderFeatures::UnsupportedFeature, "unsupportedFeature"),
         ];
 
-        assert_eq!(ReaderFeatures::VARIANTS.len(), cases.len());
+        assert_eq!(ReaderFeatures::VARIANTS.len() - 1, cases.len());
 
         for ((feature, expected), name) in cases.into_iter().zip(ReaderFeatures::VARIANTS) {
             assert_eq!(*name, expected);
@@ -178,6 +180,7 @@ mod tests {
         }
     }
 
+    // TODO: Test the UnrecognizedWriterFeature variant
     #[test]
     fn test_roundtrip_writer_features() {
         let cases = [
@@ -198,10 +201,9 @@ mod tests {
             (WriterFeatures::IcebergCompatV1, "icebergCompatV1"),
             (WriterFeatures::IcebergCompatV2, "icebergCompatV2"),
             (WriterFeatures::VacuumProtocolCheck, "vacuumProtocolCheck"),
-            (WriterFeatures::UnsupportedFeature, "unsupportedFeature"),
         ];
 
-        assert_eq!(WriterFeatures::VARIANTS.len(), cases.len());
+        assert_eq!(WriterFeatures::VARIANTS.len() - 1, cases.len());
 
         for ((feature, expected), name) in cases.into_iter().zip(WriterFeatures::VARIANTS) {
             assert_eq!(*name, expected);

diff --git a/kernel/src/utils.rs b/kernel/src/utils.rs
@@ -22,11 +22,9 @@ pub(crate) mod test_utils {
     use tempfile::TempDir;
     use test_utils::delta_path_for_version;
 
-    use crate::{
-        actions::{Add, Cdc, CommitInfo, Metadata, Protocol, Remove},
-        engine::arrow_data::ArrowEngineData,
-        EngineData,
-    };
+    use crate::actions::{Add, Cdc, CommitInfo, Metadata, Protocol, Remove};
+    use crate::engine::arrow_data::ArrowEngineData;
+    use crate::EngineData;
 
     #[derive(Serialize)]
     pub(crate) enum Action {