Copilot commented on code in PR #123:
URL: https://github.com/apache/datasketches-rust/pull/123#discussion_r3218082892
##########
datasketches/src/hll/array4.rs:
##########
@@ -503,4 +503,27 @@ mod tests {
"kxq1 should be small (1/2^40 is tiny)"
);
}
+
+ #[test]
+ fn test_shift_cur_min_rebuilds_aux_entry() {
+ let mut arr = Array4::new(4); // 16 buckets
Review Comment:
This test hard-codes the bucket count (`16`) separately from
`Array4::new(4)`, which can drift if the test setup changes. Consider deriving
the slot count from the same `lg_k` (e.g., `let lg_k = 4; let mut arr =
Array4::new(lg_k); let slots = 1usize << lg_k;`) and iterating `1..slots` to
keep the test self-consistent.
##########
datasketches/src/hll/array4.rs:
##########
@@ -503,4 +503,27 @@ mod tests {
"kxq1 should be small (1/2^40 is tiny)"
);
}
+
+ #[test]
+ fn test_shift_cur_min_rebuilds_aux_entry() {
+ let mut arr = Array4::new(4); // 16 buckets
+
+ arr.update(Coupon::pack(0, 15));
+ assert_eq!(arr.get_raw(0), AUX_TOKEN);
+ assert_eq!(arr.aux_map.as_ref().and_then(|aux| aux.get(0)), Some(15));
+
+ for slot in 1..16 {
+ arr.update(Coupon::pack(slot, 1));
+ }
+
+ assert_eq!(arr.cur_min, 1);
+ assert_eq!(arr.num_at_cur_min, 15);
+ assert_eq!(arr.get_raw(0), 14);
+ assert_eq!(arr.get(0), 15);
+ assert!(arr.aux_map.is_none());
+
+ for slot in 1..16 {
+ assert_eq!(arr.get(slot), 1);
+ }
Review Comment:
This test hard-codes the bucket count (`16`) separately from
`Array4::new(4)`, which can drift if the test setup changes. Consider deriving
the slot count from the same `lg_k` (e.g., `let lg_k = 4; let mut arr =
Array4::new(lg_k); let slots = 1usize << lg_k;`) and iterating `1..slots` to
keep the test self-consistent.
##########
datasketches/src/hll/array4.rs:
##########
@@ -503,4 +503,27 @@ mod tests {
"kxq1 should be small (1/2^40 is tiny)"
);
}
+
+ #[test]
+ fn test_shift_cur_min_rebuilds_aux_entry() {
+ let mut arr = Array4::new(4); // 16 buckets
+
+ arr.update(Coupon::pack(0, 15));
+ assert_eq!(arr.get_raw(0), AUX_TOKEN);
+ assert_eq!(arr.aux_map.as_ref().and_then(|aux| aux.get(0)), Some(15));
+
+ for slot in 1..16 {
+ arr.update(Coupon::pack(slot, 1));
+ }
Review Comment:
This test hard-codes the bucket count (`16`) separately from
`Array4::new(4)`, which can drift if the test setup changes. Consider deriving
the slot count from the same `lg_k` (e.g., `let lg_k = 4; let mut arr =
Array4::new(lg_k); let slots = 1usize << lg_k;`) and iterating `1..slots` to
keep the test self-consistent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
