From e5359354b7ce2468d4544932b244c635ecda22c7 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 14:22:42 -0500 Subject: [PATCH] format: equivalence harness only accepts the known whole-file fallbacks The harness counted ContiguousStorageRequired from any parser as an allowed difference, so a converted module that wrongly fell back to the whole file would still pass. It now accepts the error only from the three sites that are not converted yet (dense attribute storage, a SOHM B-tree index, huge fractal-heap objects, all found through a v2 B-tree) and only in the checks that can reach them; anything else fails with the check and the site named. Checked by making LocalHeap::parse_in return the error first: the fixture and h5py runs fail ("local heap fell back to the whole file"). Co-Authored-By: Claude Opus 5.5 (1M context) --- .../tests/storage_equivalence.rs | 48 +++++++++++++++++-- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/crates/clawhdf5-format/tests/storage_equivalence.rs b/crates/clawhdf5-format/tests/storage_equivalence.rs index ada6274..2f31a8b 100644 --- a/crates/clawhdf5-format/tests/storage_equivalence.rs +++ b/crates/clawhdf5-format/tests/storage_equivalence.rs @@ -11,9 +11,11 @@ //! be identical, value for value and error for error. //! //! The one allowed difference is [`FormatError::ContiguousStorageRequired`] -//! from the storage path: the structures still indexed by a v2 B-tree (dense -//! attributes, a SOHM B-tree index, huge fractal-heap objects), which fail -//! cleanly instead of reading the whole file. Those are counted. +//! from the storage path, and only from the structures still indexed by a v2 +//! B-tree (dense attributes, a SOHM B-tree index, huge fractal-heap objects; +//! see `CONTIGUOUS_REQUIRED`), which fail cleanly instead of reading the +//! whole file. Those are counted; the error from any other site or check +//! fails the harness. //! //! Milestones M2/M3 extend `check_object` with the raw-data and group //! parsers as they are converted. @@ -71,6 +73,38 @@ use clawhdf5_format::symbol_table::{SymbolTableMessage, SymbolTableNode}; const MAX_OBJECTS: usize = 1500; const MAX_HEAP_IDS: usize = 200; +/// The structures that still need the whole file in memory, because they +/// are found through a version-2 B-tree (not converted yet), and the checks +/// that can reach each of them. Anything else answering +/// [`FormatError::ContiguousStorageRequired`] is a converted parser falling +/// back to the whole file, and fails the harness. +const CONTIGUOUS_REQUIRED: &[(&str, &[&str])] = &[ + ( + "dense attribute storage (a v2 B-tree)", + &["attributes", "attributes (tolerant)"], + ), + ( + "a shared-message B-tree index", + &[ + "SOHM B-tree", + "shared message", + "fill value", + "attributes", + "attributes (tolerant)", + ], + ), + ( + "a huge fractal-heap object's B-tree", + &["heap object", "attributes", "attributes (tolerant)"], + ), +]; + +fn may_require_contiguous(check: &str, site: &str) -> bool { + CONTIGUOUS_REQUIRED + .iter() + .any(|(s, checks)| *s == site && checks.contains(&check)) +} + #[derive(Default, Debug)] struct Tally { files: usize, @@ -98,7 +132,13 @@ impl Walk<'_> { got: &Result, ) { self.tally.checks += 1; - if let Err(FormatError::ContiguousStorageRequired(_)) = got { + if let Err(FormatError::ContiguousStorageRequired(site)) = got { + assert!( + may_require_contiguous(what, site), + "{}: {what} fell back to the whole file ({site}), which only the \ + v2-B-tree-indexed structures may do", + self.name + ); self.tally.contiguous_required += 1; return; }