Re: [PATCH v5 proxmox-backup 06/16] datastore: test FixedIndexWriter

[Christian Ebner <c.ebner@proxmox.com>]

On 1/30/26 5:45 PM, Robert Obkircher wrote:
> Write fixed and dynamically sized fidx files to a temporary directory.
> Compare the resulting files directly (ignoring uuid and ctime bytes)
> and also read them back using the reader.
> 
> The chunk hashes are just dummy values that don't actually exist in a
> chunk store.
> 
> Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
> ---
>   pbs-datastore/src/fixed_index.rs | 170 +++++++++++++++++++++++++++++++
>   pbs-datastore/src/lib.rs         |   1 -
>   2 files changed, 170 insertions(+), 1 deletion(-)
> 
> diff --git a/pbs-datastore/src/fixed_index.rs b/pbs-datastore/src/fixed_index.rs
> index bcb0cf94..0f87bf15 100644
> --- a/pbs-datastore/src/fixed_index.rs
> +++ b/pbs-datastore/src/fixed_index.rs
> @@ -534,3 +534,173 @@ impl FixedIndexWriter {
>           Ok(())
>       }
>   }
> +
> +#[cfg(test)]
> +mod tests {
> +    use std::fs;
> +
> +    use super::*;
> +    use crate::temp_test_dir::TempTestDir;
> +
> +    const CS: usize = 4096;
> +
> +    #[test]
> +    fn test_empty() {
> +        let dir = TempTestDir::new();
> +        let path = dir.join("test_empty");
> +        let mut w = FixedIndexWriter::create(&path, None, CS).unwrap();
> +
> +        assert!(w.add_digest(0, &[1u8; 32]).is_err(), "out of bounds");
> +
> +        assert_eq!(0, w.size);
> +        assert_eq!(0, w.index_length(), "returns length, not capacity");
> +        assert_eq!(FixedIndexWriter::INITIAL_CAPACITY, w.index_capacity);
> +
> +        assert!(w.close().is_err(), "should refuse to create empty file");
> +
> +        drop(w);
> +        assert!(!fs::exists(path).unwrap());
> +    }
> +
> +    #[test]
> +    fn test_single_partial_chunk() {
> +        let dir = TempTestDir::new();
> +        let path = dir.join("test_single_partial_chunk");
> +        let mut w = FixedIndexWriter::create(&path, None, CS).unwrap();
> +
> +        let size = CS - 1;
> +        let expected = test_data(size);
> +        w.grow_to_size(size).unwrap();
> +        expected[0].add_to(&mut w);
> +
> +        w.close().unwrap();
> +        drop(w);
> +
> +        check_with_reader(&path, size, &expected);
> +        compare_to_known_size_writer(&path, size, &expected);
> +    }
> +
> +    #[test]
> +    fn test_grow_to_multiples_of_chunk_size() {
> +        let dir = TempTestDir::new();
> +        let path = dir.join("test_grow_to_multiples_of_chunk_size");
> +        let mut w = FixedIndexWriter::create(&path, None, CS).unwrap();
> +
> +        let initial = FixedIndexWriter::INITIAL_CAPACITY;
> +        let steps = [1, 2, initial, initial + 1, 5 * initial, 10 * initial + 1];
> +        let expected = test_data(steps.last().unwrap() * CS);
> +
> +        let mut begin = 0;
> +        for chunk_count in steps {
> +            let last = &expected[chunk_count - 1];
> +            w.grow_to_size(last.end).unwrap();
> +            assert_eq!(last.index + 1, w.index_length());
> +            assert!(w.add_digest(last.index + 1, &[1u8; 32]).is_err());
> +
> +            for c in expected[begin..chunk_count].iter().rev() {
> +                c.add_to(&mut w);
> +            }
> +            begin = chunk_count;
> +        }
> +        w.close().unwrap();
> +        drop(w);
> +
> +        let size = expected.len() * CS;
> +        check_with_reader(&path, size, &expected);
> +        compare_to_known_size_writer(&path, size, &expected);
> +    }
> +
> +    #[test]
> +    fn test_grow_to_misaligned_size() {
> +        let dir = TempTestDir::new();
> +        let path = dir.join("test_grow_to_misaligned_size");
> +        let mut w = FixedIndexWriter::create(&path, None, CS).unwrap();
> +
> +        let size = (FixedIndexWriter::INITIAL_CAPACITY + 42) * CS - 1; // last is not full
> +        let expected = test_data(size);
> +
> +        w.grow_to_size(size).unwrap();
> +        assert!(w.grow_to_size(size + 1).is_err(), "size must be fixed now");
> +        assert_eq!(expected.len(), w.index_length());
> +        assert!(w.add_digest(expected.len(), &[1u8; 32]).is_err());
> +
> +        for c in expected.iter().rev() {
> +            c.add_to(&mut w);
> +        }
> +
> +        w.close().unwrap();
> +        drop(w);
> +
> +        check_with_reader(&path, size, &expected);
> +        compare_to_known_size_writer(&path, size, &expected);
> +    }
> +
> +    struct TestChunk {
> +        digest: [u8; 32],
> +        index: usize,
> +        size: usize,
> +        end: usize,
> +    }
> +
> +    impl TestChunk {
> +        fn add_to(&self, w: &mut FixedIndexWriter) {
> +            assert_eq!(
> +                self.index,
> +                w.check_chunk_alignment(self.end, self.size).unwrap()
> +            );
> +            w.add_digest(self.index, &self.digest).unwrap();
> +        }
> +    }
> +
> +    fn test_data(size: usize) -> Vec<TestChunk> {
> +        (0..size.div_ceil(CS))
> +            .map(|index| {
> +                let mut digest = [0u8; 32];
> +                let i = &(index as u64).to_le_bytes();
> +                for c in digest.chunks_mut(i.len()) {
> +                    c.copy_from_slice(i);
> +                }
> +                let size = if ((index + 1) * CS) <= size {
> +                    CS
> +                } else {
> +                    size % CS
> +                };
> +                TestChunk {
> +                    digest,
> +                    index,
> +                    size,
> +                    end: index * CS + size,
> +                }
> +            })
> +            .collect()
> +    }
> +
> +    fn check_with_reader(path: &Path, size: usize, chunks: &[TestChunk]) {
> +        let reader = FixedIndexReader::open(path).unwrap();
> +        assert_eq!(size as u64, reader.index_bytes());
> +        assert_eq!(chunks.len(), reader.index_count());
> +        for c in chunks {
> +            assert_eq!(&c.digest, reader.index_digest(c.index).unwrap());
> +        }
> +    }
> +
> +    fn compare_to_known_size_writer(file: &Path, size: usize, chunks: &[TestChunk]) {
> +        let mut path = file.to_path_buf();
> +        path.set_extension("reference");
> +        let mut w = FixedIndexWriter::create(&path, Some(size), CS).unwrap();
> +        for c in chunks {
> +            c.add_to(&mut w);
> +        }
> +        w.close().unwrap();
> +        drop(w);
> +
> +        let mut reference = fs::read(file).unwrap();
> +        let mut tested = fs::read(path).unwrap();
> +
> +        // ignore uuid and ctime
> +        reference[8..32].fill(0);
> +        tested[8..32].fill(0);
> +
> +        assert_eq!(reference, tested);
> +    }
> +}
> diff --git a/pbs-datastore/src/lib.rs b/pbs-datastore/src/lib.rs
> index b48ec93c..23a17d96 100644
> --- a/pbs-datastore/src/lib.rs
> +++ b/pbs-datastore/src/lib.rs
> @@ -236,4 +236,3 @@ pub use local_datastore_lru_cache::LocalDatastoreLruCache;
>   
>   #[cfg(test)]
>   mod temp_test_dir;
> -

nit: unrelated hunk introduced by patch 0003, which should be fixed up 
there.

Apart from that, test look good to me.

Reviewed-by: Christian Ebner <c.ebner@proxmox.com>