Skip to content

Commit 1f894d3

Browse files
authored
feat: compare array metadata against checked in goldenfiles (#1812)
This is part of a general epic (is that word taboo?) around stability. I have a separate changeset that stores an entire Vortex file and checks for round-trip compatibility. However, it's also helpful to know on a per-encoding level that the metadata format is stable. This PR adds [goldenfile](https://crates.io/crates/goldenfile) tests for all encoding metadata. This is a bit simplistic: we don't check for backward/forward compatibility, this is just an attempt to make it clearer on PRs when changes are made to the metadata format. Now when changes are made to Array metadata, you should re-run the test with ``` UPDATE_GOLDENFILES=1 cargo test -p ... ``` and that will update the checked-in metadatas. Roughly half of the changed files are checked-in snapshots, most of the other half are simple test definitions. The only serious change is the addition of the `check_metadata` helper in test-utils.
1 parent 98ac114 commit 1f894d3

File tree

66 files changed

+496
-34
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

66 files changed

+496
-34
lines changed

Cargo.lock

Lines changed: 50 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ futures = { version = "0.3", default-features = false }
8585
futures-executor = "0.3"
8686
futures-util = "0.3"
8787
getrandom = "0.2.14"
88+
goldenfile = "1"
8889
half = { version = "^2", features = ["std", "num-traits"] }
8990
hashbrown = "0.15.0"
9091
homedir = "0.3.3"

encodings/alp/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ vortex-fastlanes = { workspace = true }
2828
vortex-scalar = { workspace = true }
2929

3030
[dev-dependencies]
31-
arrow-array = { workspace = true }
3231
divan = { workspace = true }
3332
rstest = { workspace = true }
33+
vortex-array = { path = "../../vortex-array", features = ["test_util"] }
3434

3535
[[bench]]
3636
name = "alp_compress"
119 Bytes
Binary file not shown.
160 Bytes
Binary file not shown.

encodings/alp/src/alp/array.rs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ impl_encoding!("vortex.alp", ids::ALP, ALP);
2121

2222
#[derive(Debug, Clone, Serialize, Deserialize)]
2323
pub struct ALPMetadata {
24-
exponents: Exponents,
25-
patches: Option<PatchesMetadata>,
24+
pub(crate) exponents: Exponents,
25+
pub(crate) patches: Option<PatchesMetadata>,
2626
}
2727

2828
impl Display for ALPMetadata {
@@ -150,3 +150,27 @@ impl VisitorVTable<ALPArray> for ALPEncoding {
150150
}
151151

152152
impl StatisticsVTable<ALPArray> for ALPEncoding {}
153+
154+
#[cfg(test)]
155+
mod tests {
156+
use vortex_array::patches::PatchesMetadata;
157+
use vortex_array::test_utils::check_metadata;
158+
use vortex_dtype::PType;
159+
160+
use crate::{ALPMetadata, Exponents};
161+
162+
#[cfg_attr(miri, ignore)]
163+
#[test]
164+
fn test_alp_metadata() {
165+
check_metadata(
166+
"alp.metadata",
167+
ALPMetadata {
168+
patches: Some(PatchesMetadata::new(usize::MAX, PType::U64)),
169+
exponents: Exponents {
170+
e: u8::MAX,
171+
f: u8::MAX,
172+
},
173+
},
174+
);
175+
}
176+
}

encodings/alp/src/alp_rd/array.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,27 @@ impl ArrayTrait for ALPRDArray {}
258258
mod test {
259259
use rstest::rstest;
260260
use vortex_array::array::PrimitiveArray;
261+
use vortex_array::patches::PatchesMetadata;
262+
use vortex_array::test_utils::check_metadata;
261263
use vortex_array::{IntoArrayData, IntoCanonical};
264+
use vortex_dtype::PType;
262265

263-
use crate::{alp_rd, ALPRDFloat};
266+
use crate::{alp_rd, ALPRDFloat, ALPRDMetadata};
267+
268+
#[cfg_attr(miri, ignore)]
269+
#[test]
270+
fn test_alprd_metadata() {
271+
check_metadata(
272+
"alprd.metadata",
273+
ALPRDMetadata {
274+
right_bit_width: u8::MAX,
275+
patches: Some(PatchesMetadata::new(usize::MAX, PType::U64)),
276+
dict: [0u16; 8],
277+
left_parts_ptype: PType::U64,
278+
dict_len: 8,
279+
},
280+
);
281+
}
264282

265283
#[rstest]
266284
#[case(vec![0.1f32.next_up(); 1024], 1.123_848_f32)]

encodings/bytebool/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,6 @@ vortex-buffer = { workspace = true }
2525
vortex-dtype = { workspace = true }
2626
vortex-error = { workspace = true }
2727
vortex-scalar = { workspace = true }
28+
29+
[dev-dependencies]
30+
vortex-array = { path = "../../vortex-array", features = ["test_util"] }
29 Bytes
Binary file not shown.

encodings/bytebool/src/array.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,22 @@ impl VisitorVTable<ByteBoolArray> for ByteBoolEncoding {
133133

134134
#[cfg(test)]
135135
mod tests {
136+
use vortex_array::test_utils::check_metadata;
136137
use vortex_array::validity::ArrayValidity;
137138

138139
use super::*;
139140

141+
#[cfg_attr(miri, ignore)]
142+
#[test]
143+
fn test_bytebool_metadata() {
144+
check_metadata(
145+
"bytebool.metadata",
146+
ByteBoolMetadata {
147+
validity: ValidityMetadata::AllValid,
148+
},
149+
);
150+
}
151+
140152
#[test]
141153
fn test_validity_construction() {
142154
let v = vec![true, false];

0 commit comments

Comments
 (0)