Add roundtrip serialization property tests for types in change sets file

icechunk/src/change_set.rs

@@ -716,12 +716,13 @@ mod tests {
     use super::ChangeSet;
 
     use crate::{
-        change_set::{ArrayData, MoveTracker},
+        change_set::{ArrayData, EditChanges, MoveTracker},
         format::{
             ChunkIndices, NodeId, Path,
             manifest::{ChunkInfo, ChunkPayload, ManifestSplits},
             snapshot::ArrayShape,
         },
+        roundtrip_serialization_tests,
     };
 
     #[icechunk_macros::test]
@@ -965,4 +966,40 @@ mod tests {
             &Path::new("/other").unwrap(),
         );
     }
+
+    use crate::strategies::{
+        array_data, bytes, chunk_indices2, gen_move, manifest_extents, node_id, path,
+        split_manifest,
+    };
+    use proptest::collection::{btree_map, hash_map, hash_set, vec};
+    use proptest::prelude::*;
+
+    prop_compose! {
+        fn edit_changes()(num_of_dims in any::<u16>().prop_map(usize::from))(
+            new_groups in hash_map(path(),(node_id(), bytes()), 3..7),
+                new_arrays in hash_map(path(),(node_id(), array_data()), 3..7),
+           updated_arrays in hash_map(node_id(), array_data(), 3..7),
+           updated_groups in hash_map(node_id(), bytes(), 3..7),
+           set_chunks in btree_map(node_id(),
+                hash_map(manifest_extents(num_of_dims), split_manifest(), 3..7),
+            3..7),
+    deleted_chunks_outside_bounds in btree_map(node_id(), hash_set(chunk_indices2(), 3..8), 3..7),
+            deleted_groups in hash_set((path(), node_id()), 3..7),
+            deleted_arrays in hash_set((path(), node_id()), 3..7)
+        ) -> EditChanges {
+            EditChanges{new_groups, updated_groups, updated_arrays, set_chunks, deleted_chunks_outside_bounds, deleted_arrays, deleted_groups, new_arrays}
+        }
+    }
+
+    fn move_tracker() -> impl Strategy<Value = MoveTracker> {
+        vec(gen_move(), 1..5).prop_map(MoveTracker).boxed()
+    }
+
+    fn change_set() -> impl Strategy<Value = ChangeSet> {
+        use ChangeSet::*;
+        prop_oneof![edit_changes().prop_map(Edit), move_tracker().prop_map(Rearrange)]
+            .boxed()
+    }
+
+    roundtrip_serialization_tests!(serialize_and_deserialize_change_sets - change_set);
 }

icechunk/src/config.rs

@@ -632,25 +632,16 @@ pub enum Credentials {
     Azure(AzureCredentials),
 }
 
-#[cfg(test)]
-#[allow(clippy::panic, clippy::unwrap_used, clippy::expect_used)]
-mod tests {
-    use crate::strategies::{
-        azure_credentials, gcs_static_credentials, repository_config,
-        s3_static_credentials,
-    };
-
-    use proptest::prelude::*;
-
-    // This macro is used for creating property tests
-    // which check that serializing and deserializing
-    // an instance of a type T is equivalent to the
-    // identity function
-    // Given pairs of test names and arbitraries to be used
-    // for the tests, e.g., (n1, a1), (n2, a2),... (nx, ax)
-    // the tests can be created by doing
-    // roundtrip_serialization_tests!(n1 - a1, n2 - a2, .... nx - ax)
-    macro_rules! roundtrip_serialization_tests {
+// This macro is used for creating property tests
+// which check that serializing and deserializing
+// an instance of a type T is equivalent to the
+// identity function
+// Given pairs of test names and arbitraries to be used
+// for the tests, e.g., (n1, a1), (n2, a2),... (nx, ax)
+// the tests can be created by doing
+// roundtrip_serialization_tests!(n1 - a1, n2 - a2, .... nx - ax)
+#[macro_export]
+macro_rules! roundtrip_serialization_tests {
         ($($test_name: ident - $generator: ident), +) => {
                             $(
                 proptest!{
@@ -663,6 +654,16 @@ mod tests {
         }
     }
 
+#[cfg(test)]
+#[allow(clippy::panic, clippy::unwrap_used, clippy::expect_used)]
+mod tests {
+    use crate::strategies::{
+        azure_credentials, gcs_static_credentials, repository_config,
+        s3_static_credentials,
+    };
+
+    use proptest::prelude::*;
+
     roundtrip_serialization_tests!(
         test_config_roundtrip - repository_config,
         test_s3_static_credentials_roundtrip - s3_static_credentials,

icechunk/src/format/manifest.rs

@@ -636,7 +636,9 @@ static ROOT_OPTIONS: VerifierOptions = VerifierOptions {
 #[allow(clippy::unwrap_used, clippy::panic)]
 mod tests {
     use super::*;
-    use crate::strategies::{ShapeDim, manifest_extents, shapes_and_dims};
+    use crate::strategies::{
+        ShapeDim, manifest_extents, manifest_extents2, shapes_and_dims,
+    };
     use icechunk_macros;
     use itertools::{all, multizip};
     use proptest::collection::vec;
@@ -683,7 +685,7 @@ mod tests {
 
     #[proptest]
     fn test_property_extents_widths(
-        #[strategy(manifest_extents(4))] extent1: ManifestExtents,
+        #[strategy(manifest_extents2(4))] extent1: ManifestExtents,
         #[strategy(vec(0..100, 4))] delta_left: Vec<i32>,
         #[strategy(vec(0..100, 4))] delta_right: Vec<i32>,
     ) {
@@ -732,6 +734,7 @@ mod tests {
                     ..((extent.end as i32 - width - low) as u32)
             }),
         );
+
         prop_assert_eq!(extent2.overlap_with(&extent1), Overlap::None);
 
         let extent2 = ManifestExtents::from_ranges_iter(

icechunk/src/storage/redirect.rs

@@ -77,6 +77,7 @@ impl RedirectStorage {
                     "Cannot build http client for redirect Storage instance: {e}"
                 )))
             })?;
+
         let req = client.get(self.url.clone()).build().map_err(|e| {
             StorageError::from(StorageErrorKind::BadRedirect(format!(
                 "Cannot build http request for redirect Storage instance: {e}"