From 554a921161d049fb56a64e55c212e44985efb468 Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Tue, 31 Mar 2026 17:30:18 +0100 Subject: [PATCH 1/3] Assert_arrays_eq also executes the array and compares the results against scalar_at Signed-off-by: Robert Kruszewski --- vortex-array/src/arrays/assertions.rs | 56 ++++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/vortex-array/src/arrays/assertions.rs b/vortex-array/src/arrays/assertions.rs index ef409f2af23..cf85837903a 100644 --- a/vortex-array/src/arrays/assertions.rs +++ b/vortex-array/src/arrays/assertions.rs @@ -71,16 +71,60 @@ macro_rules! assert_arrays_eq { ) } + let executed = { + use $crate::IntoArray; + use $crate::VortexSessionExecute; + let mut ctx = $crate::LEGACY_SESSION.create_execution_ctx(); + // Allow deprecated to_array() as it's the only method that works uniformly + // for all input types (ArrayRef, concrete arrays, and &dyn DynArray). + #[allow(deprecated)] + let arr = left.to_array(); + arr.execute::<$crate::RecursiveCanonical>(&mut ctx) + .expect("assert_arrays_eq: failed to execute left array to recursive canonical form") + .0 + .into_array() + }; + let n = left.len(); - let mismatched_indices = (0..n) + let left_right_mismatched: Vec = (0..n) .filter(|i| left.scalar_at(*i).unwrap() != right.scalar_at(*i).unwrap()) - .collect::>(); - if mismatched_indices.len() != 0 { + .collect(); + let left_executed_mismatched: Vec = (0..n) + .filter(|i| left.scalar_at(*i).unwrap() != executed.scalar_at(*i).unwrap()) + .collect(); + let right_executed_mismatched: Vec = (0..n) + .filter(|i| right.scalar_at(*i).unwrap() != executed.scalar_at(*i).unwrap()) + .collect(); + + if !left_right_mismatched.is_empty() + || !left_executed_mismatched.is_empty() + || !right_executed_mismatched.is_empty() + { + let mut msg = String::new(); + if !left_right_mismatched.is_empty() { + msg.push_str(&format!( + "\n left != right at indices: {}", + $crate::arrays::format_indices(left_right_mismatched) + )); + } + if !left_executed_mismatched.is_empty() { + msg.push_str(&format!( + "\n left != executed at indices: {}", + $crate::arrays::format_indices(left_executed_mismatched) + )); + } + if !right_executed_mismatched.is_empty() { + msg.push_str(&format!( + "\n right != executed at indices: {}", + $crate::arrays::format_indices(right_executed_mismatched) + )); + } panic!( - "assertion left == right failed: arrays do not match at indices: {}.\n left: {}\n right: {}", - $crate::arrays::format_indices(mismatched_indices), + "assertion failed: arrays do not match:{}\n left: {}\n right: {}\n executed: {}", + msg, left.display_values(), - right.display_values() + right.display_values(), + executed.display_values() ) } }}; From 65017d9582d9861d005131d878fee810ff2e3b76 Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Tue, 31 Mar 2026 18:07:41 +0100 Subject: [PATCH 2/3] asserts --- vortex-array/src/arrays/assertions.rs | 156 +++++++++++++------------- vortex-array/src/arrays/mod.rs | 2 +- 2 files changed, 82 insertions(+), 76 deletions(-) diff --git a/vortex-array/src/arrays/assertions.rs b/vortex-array/src/arrays/assertions.rs index cf85837903a..fae6e5e29fc 100644 --- a/vortex-array/src/arrays/assertions.rs +++ b/vortex-array/src/arrays/assertions.rs @@ -5,10 +5,35 @@ use std::fmt::Display; use itertools::Itertools; -pub fn format_indices>(indices: I) -> impl Display { +use crate::ArrayRef; +use crate::DynArray; +use crate::ExecutionCtx; +use crate::IntoArray; +use crate::LEGACY_SESSION; +use crate::RecursiveCanonical; +use crate::VortexSessionExecute; + +fn format_indices>(indices: I) -> impl Display { indices.into_iter().format(",") } +/// Executes an array to recursive canonical form with the given execution context. +fn execute_to_canonical(array: ArrayRef, ctx: &mut ExecutionCtx) -> ArrayRef { + array + .execute::(ctx) + .expect("failed to execute array to recursive canonical form") + .0 + .into_array() +} + +/// Finds indices where two arrays differ based on `scalar_at` comparison. +fn find_mismatched_indices(left: &ArrayRef, right: &ArrayRef) -> Vec { + assert_eq!(left.len(), right.len()); + (0..left.len()) + .filter(|i| left.scalar_at(*i).unwrap() != right.scalar_at(*i).unwrap()) + .collect() +} + /// Asserts that the scalar at position `$n` in array `$arr` equals `$expected`. /// /// This is a convenience macro for testing that avoids verbose scalar comparison code. @@ -49,83 +74,64 @@ macro_rules! assert_nth_scalar_is_null { #[macro_export] macro_rules! assert_arrays_eq { ($left:expr, $right:expr) => {{ - let left = $left.clone(); - let right = $right.clone(); - if left.dtype() != right.dtype() { - panic!( - "assertion left == right failed: arrays differ in type: {} != {}.\n left: {}\n right: {}", - left.dtype(), - right.dtype(), - left.display_values(), - right.display_values() - ) - } + assert_eq!( + left.dtype(), + right.dtype(), + "assertion left == right failed: arrays differ in type: {} != {}.\n left: {}\n right: {}", + left.dtype(), + right.dtype(), + left.display_values(), + right.display_values() + ); - if left.len() != right.len() { - panic!( - "assertion left == right failed: arrays differ in length: {} != {}.\n left: {}\n right: {}", - left.len(), - right.len(), - left.display_values(), - right.display_values() - ) - } + assert_eq!( + left.len(), + right.len(), + "assertion left == right failed: arrays differ in length: {} != {}.\n left: {}\n right: {}", + left.len(), + right.len(), + left.display_values(), + right.display_values() + ); - let executed = { - use $crate::IntoArray; - use $crate::VortexSessionExecute; - let mut ctx = $crate::LEGACY_SESSION.create_execution_ctx(); - // Allow deprecated to_array() as it's the only method that works uniformly - // for all input types (ArrayRef, concrete arrays, and &dyn DynArray). - #[allow(deprecated)] - let arr = left.to_array(); - arr.execute::<$crate::RecursiveCanonical>(&mut ctx) - .expect("assert_arrays_eq: failed to execute left array to recursive canonical form") - .0 - .into_array() - }; + #[allow(deprecated)] + let left = $left.to_array(); + #[allow(deprecated)] + let right = $right.to_array(); + $crate::arrays::assert_arrays_eq_impl(&left, &right); + }}; +} - let n = left.len(); - let left_right_mismatched: Vec = (0..n) - .filter(|i| left.scalar_at(*i).unwrap() != right.scalar_at(*i).unwrap()) - .collect(); - let left_executed_mismatched: Vec = (0..n) - .filter(|i| left.scalar_at(*i).unwrap() != executed.scalar_at(*i).unwrap()) - .collect(); - let right_executed_mismatched: Vec = (0..n) - .filter(|i| right.scalar_at(*i).unwrap() != executed.scalar_at(*i).unwrap()) - .collect(); +/// Implementation of `assert_arrays_eq!` — called by the macro after converting inputs to +/// `ArrayRef`. +#[track_caller] +#[allow(clippy::panic)] +pub fn assert_arrays_eq_impl(left: &ArrayRef, right: &ArrayRef) { + let executed = execute_to_canonical(left.clone(), &mut LEGACY_SESSION.create_execution_ctx()); - if !left_right_mismatched.is_empty() - || !left_executed_mismatched.is_empty() - || !right_executed_mismatched.is_empty() - { - let mut msg = String::new(); - if !left_right_mismatched.is_empty() { - msg.push_str(&format!( - "\n left != right at indices: {}", - $crate::arrays::format_indices(left_right_mismatched) - )); - } - if !left_executed_mismatched.is_empty() { - msg.push_str(&format!( - "\n left != executed at indices: {}", - $crate::arrays::format_indices(left_executed_mismatched) - )); - } - if !right_executed_mismatched.is_empty() { - msg.push_str(&format!( - "\n right != executed at indices: {}", - $crate::arrays::format_indices(right_executed_mismatched) - )); - } - panic!( - "assertion failed: arrays do not match:{}\n left: {}\n right: {}\n executed: {}", - msg, - left.display_values(), - right.display_values(), - executed.display_values() - ) + let left_right = find_mismatched_indices(left, right); + let executed_right = find_mismatched_indices(&executed, right); + + if !left_right.is_empty() || !executed_right.is_empty() { + let mut msg = String::new(); + if !left_right.is_empty() { + msg.push_str(&format!( + "\n left != right at indices: {}", + format_indices(left_right) + )); } - }}; + if !executed_right.is_empty() { + msg.push_str(&format!( + "\n executed != right at indices: {}", + format_indices(executed_right) + )); + } + panic!( + "assertion failed: arrays do not match:{}\n left: {}\n right: {}\n executed: {}", + msg, + left.display_values(), + right.display_values(), + executed.display_values() + ) + } } diff --git a/vortex-array/src/arrays/mod.rs b/vortex-array/src/arrays/mod.rs index 43f8a84d49e..947116a5f73 100644 --- a/vortex-array/src/arrays/mod.rs +++ b/vortex-array/src/arrays/mod.rs @@ -7,7 +7,7 @@ mod assertions; #[cfg(any(test, feature = "_test-harness"))] -pub use assertions::format_indices; +pub use assertions::assert_arrays_eq_impl; #[cfg(test)] mod validation_tests; From 7e43d02b4e3ae8f5ce3d6c9b314a8f8df3044d5c Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Tue, 31 Mar 2026 18:14:26 +0100 Subject: [PATCH 3/3] simpler Signed-off-by: Robert Kruszewski --- vortex-array/src/arrays/assertions.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/vortex-array/src/arrays/assertions.rs b/vortex-array/src/arrays/assertions.rs index fae6e5e29fc..9be7a8c16c6 100644 --- a/vortex-array/src/arrays/assertions.rs +++ b/vortex-array/src/arrays/assertions.rs @@ -4,6 +4,7 @@ use std::fmt::Display; use itertools::Itertools; +use vortex_error::VortexExpect; use crate::ArrayRef; use crate::DynArray; @@ -21,12 +22,13 @@ fn format_indices>(indices: I) -> impl Display { fn execute_to_canonical(array: ArrayRef, ctx: &mut ExecutionCtx) -> ArrayRef { array .execute::(ctx) - .expect("failed to execute array to recursive canonical form") + .vortex_expect("failed to execute array to recursive canonical form") .0 .into_array() } /// Finds indices where two arrays differ based on `scalar_at` comparison. +#[expect(clippy::unwrap_used)] fn find_mismatched_indices(left: &ArrayRef, right: &ArrayRef) -> Vec { assert_eq!(left.len(), right.len()); (0..left.len()) @@ -74,6 +76,8 @@ macro_rules! assert_nth_scalar_is_null { #[macro_export] macro_rules! assert_arrays_eq { ($left:expr, $right:expr) => {{ + let left = $left.clone(); + let right = $right.clone(); assert_eq!( left.dtype(), right.dtype(), @@ -95,9 +99,9 @@ macro_rules! assert_arrays_eq { ); #[allow(deprecated)] - let left = $left.to_array(); + let left = left.to_array(); #[allow(deprecated)] - let right = $right.to_array(); + let right = right.to_array(); $crate::arrays::assert_arrays_eq_impl(&left, &right); }}; }