Skip to content

Commit 08b8fbc

Browse files
authored
test: add API conformance tests (#346)
This PR enhances test coverage by adding API conformance tests for key data structures such as `Vec`, `MinHeap`, `BTreeSet`, and `BTreeMap` while renaming the misleading `iter_upper_bound` method to `iter_from_prev_key`.
1 parent 3f6c0c7 commit 08b8fbc

File tree

6 files changed

+356
-112
lines changed

6 files changed

+356
-112
lines changed

src/btreemap.rs

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1188,16 +1188,33 @@ where
11881188
self.range_internal(key_range).into()
11891189
}
11901190

1191-
/// Returns an iterator pointing to the first element below the given bound.
1192-
/// Returns an empty iterator if there are no keys below the given bound.
1193-
pub fn iter_upper_bound(&self, bound: &K) -> Iter<K, V, M> {
1191+
/// Returns an iterator starting just before the given key.
1192+
///
1193+
/// Finds the largest key strictly less than `bound` and starts from it.
1194+
/// Useful when `range(bound..)` skips the previous element.
1195+
///
1196+
/// Returns an empty iterator if no smaller key exists.
1197+
pub fn iter_from_prev_key(&self, bound: &K) -> Iter<K, V, M> {
11941198
if let Some((start_key, _)) = self.range(..bound).next_back() {
11951199
IterInternal::new_in_range(self, (Bound::Included(start_key), Bound::Unbounded)).into()
11961200
} else {
11971201
IterInternal::null(self).into()
11981202
}
11991203
}
12001204

1205+
/// **Deprecated**: use [`iter_from_prev_key`] instead.
1206+
///
1207+
/// The name `iter_upper_bound` was misleading — it suggested an inclusive
1208+
/// upper bound. In reality, it starts from the largest key strictly less
1209+
/// than the given bound.
1210+
///
1211+
/// The new name, [`iter_from_prev_key`], better reflects this behavior and
1212+
/// improves code clarity.
1213+
#[deprecated(note = "use `iter_from_prev_key` instead")]
1214+
pub fn iter_upper_bound(&self, bound: &K) -> Iter<K, V, M> {
1215+
self.iter_from_prev_key(bound)
1216+
}
1217+
12011218
/// Returns an iterator over the keys of the map.
12021219
pub fn keys(&self) -> KeysIter<K, V, M> {
12031220
self.iter_internal().into()
@@ -2955,28 +2972,28 @@ mod test {
29552972
}
29562973
btree_test!(test_bruteforce_range_search, bruteforce_range_search);
29572974

2958-
fn test_iter_upper_bound<K: TestKey, V: TestValue>() {
2975+
fn test_iter_from_prev_key<K: TestKey, V: TestValue>() {
29592976
let (key, value) = (K::build, V::build);
29602977
run_btree_test(|mut btree| {
29612978
for j in 0..100 {
29622979
btree.insert(key(j), value(j));
29632980
for i in 0..=j {
29642981
assert_eq!(
2965-
btree.iter_upper_bound(&key(i + 1)).next(),
2982+
btree.iter_from_prev_key(&key(i + 1)).next(),
29662983
Some((key(i), value(i))),
29672984
"failed to get an upper bound for key({})",
29682985
i + 1
29692986
);
29702987
}
29712988
assert_eq!(
2972-
btree.iter_upper_bound(&key(0)).next(),
2989+
btree.iter_from_prev_key(&key(0)).next(),
29732990
None,
29742991
"key(0) must not have an upper bound"
29752992
);
29762993
}
29772994
});
29782995
}
2979-
btree_test!(test_test_iter_upper_bound, test_iter_upper_bound);
2996+
btree_test!(test_test_iter_from_prev_key, test_iter_from_prev_key);
29802997

29812998
// A buggy implementation of storable where the max_size is smaller than the serialized size.
29822999
#[derive(Clone, Ord, PartialOrd, Eq, PartialEq)]

src/btreemap/proptests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,12 +141,12 @@ fn map_min_max(#[strategy(pvec(any::<u64>(), 10..100))] keys: Vec<u64>) {
141141
}
142142

143143
#[proptest]
144-
fn map_upper_bound_iter(#[strategy(pvec(0u64..u64::MAX -1 , 10..100))] keys: Vec<u64>) {
144+
fn map_iter_from_prev_key(#[strategy(pvec(0u64..u64::MAX -1 , 10..100))] keys: Vec<u64>) {
145145
run_btree_test(|mut map| {
146146
for k in keys.iter() {
147147
map.insert(*k, ());
148148

149-
prop_assert_eq!(Some((*k, ())), map.iter_upper_bound(&(k + 1)).next());
149+
prop_assert_eq!(Some((*k, ())), map.iter_from_prev_key(&(k + 1)).next());
150150
}
151151

152152
Ok(())

src/btreeset.rs

Lines changed: 0 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -510,31 +510,6 @@ where
510510
Iter::new(self.map.range(key_range))
511511
}
512512

513-
/// Returns an iterator pointing to the first element strictly below the given bound.
514-
/// Returns an empty iterator if there are no keys strictly below the given bound.
515-
///
516-
/// # Complexity
517-
/// O(log n) for creating the iterator, where n is the number of elements in the set.
518-
///
519-
/// # Example
520-
///
521-
/// ```rust
522-
/// use ic_stable_structures::{BTreeSet, DefaultMemoryImpl};
523-
/// use ic_stable_structures::memory_manager::{MemoryId, MemoryManager};
524-
///
525-
/// let mem_mgr = MemoryManager::init(DefaultMemoryImpl::default());
526-
/// let mut set: BTreeSet<u64, _> = BTreeSet::new(mem_mgr.get(MemoryId::new(0)));
527-
/// set.insert(1);
528-
/// set.insert(2);
529-
/// set.insert(3);
530-
///
531-
/// let upper_bound: Option<u64> = set.iter_upper_bound(&3).next();
532-
/// assert_eq!(upper_bound, Some(2));
533-
/// ```
534-
pub fn iter_upper_bound(&self, bound: &K) -> Iter<K, M> {
535-
Iter::new(self.map.iter_upper_bound(bound))
536-
}
537-
538513
/// Returns an iterator over the union of this set and another.
539514
///
540515
/// The union of two sets is a set containing all elements that are in either set.
@@ -1110,31 +1085,6 @@ mod test {
11101085
assert!(!btreeset.contains(&1u32));
11111086
}
11121087

1113-
#[test]
1114-
fn test_iter_upper_bound() {
1115-
let mem = make_memory();
1116-
let mut btreeset = BTreeSet::new(mem);
1117-
1118-
for i in 0u32..100 {
1119-
btreeset.insert(i);
1120-
1121-
// Test that `iter_upper_bound` returns the largest element strictly below the bound.
1122-
for j in 1u32..=i {
1123-
assert_eq!(
1124-
btreeset.iter_upper_bound(&(j + 1)).next(),
1125-
Some(j),
1126-
"failed to get an upper bound for {}",
1127-
j + 1
1128-
);
1129-
}
1130-
assert_eq!(
1131-
btreeset.iter_upper_bound(&0).next(),
1132-
None,
1133-
"0 must not have an upper bound"
1134-
);
1135-
}
1136-
}
1137-
11381088
#[test]
11391089
fn test_iter() {
11401090
let mem = make_memory();
@@ -1234,20 +1184,6 @@ mod test {
12341184
assert_eq!(elements[999], 999);
12351185
}
12361186

1237-
#[test]
1238-
fn test_iter_upper_bound_large_set() {
1239-
let mem = make_memory();
1240-
let mut btreeset: BTreeSet<u32, _> = BTreeSet::new(mem);
1241-
1242-
for i in 0u32..1000 {
1243-
btreeset.insert(i);
1244-
}
1245-
1246-
assert_eq!(btreeset.iter_upper_bound(&500).next(), Some(499));
1247-
assert_eq!(btreeset.iter_upper_bound(&0).next(), None);
1248-
assert_eq!(btreeset.iter_upper_bound(&1000).next(), Some(999));
1249-
}
1250-
12511187
#[test]
12521188
fn test_range_large_set() {
12531189
let mem = make_memory();
@@ -1304,14 +1240,6 @@ mod test {
13041240
assert_eq!(btreeset.pop_last(), None);
13051241
}
13061242

1307-
#[test]
1308-
fn test_iter_upper_bound_empty() {
1309-
let mem = make_memory();
1310-
let btreeset: BTreeSet<u32, _> = BTreeSet::new(mem);
1311-
1312-
assert_eq!(btreeset.iter_upper_bound(&42u32).next(), None);
1313-
}
1314-
13151243
#[test]
13161244
fn test_range_empty() {
13171245
let mem = make_memory();
@@ -1414,20 +1342,6 @@ mod test {
14141342
assert!(btreeset.is_empty());
14151343
}
14161344

1417-
#[test]
1418-
fn test_iter_upper_bound_edge_cases() {
1419-
let mem = make_memory();
1420-
let mut btreeset: BTreeSet<u32, _> = BTreeSet::new(mem);
1421-
1422-
for i in 1..=10 {
1423-
btreeset.insert(i);
1424-
}
1425-
1426-
assert_eq!(btreeset.iter_upper_bound(&1).next(), None); // No element strictly below 1
1427-
assert_eq!(btreeset.iter_upper_bound(&5).next(), Some(4)); // Largest element below 5
1428-
assert_eq!(btreeset.iter_upper_bound(&11).next(), Some(10)); // Largest element below 11
1429-
}
1430-
14311345
#[test]
14321346
fn test_is_disjoint_with_disjoint_sets() {
14331347
let mem1 = make_memory();

src/btreeset/proptests.rs

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -67,19 +67,6 @@ fn set_min_max(#[strategy(pvec(any::<u64>(), 10..100))] keys: Vec<u64>) {
6767
});
6868
}
6969

70-
#[proptest]
71-
fn set_upper_bound_iter(#[strategy(pvec(0u64..u64::MAX - 1, 10..100))] keys: Vec<u64>) {
72-
crate::btreeset::test::run_btree_test(|mut set| {
73-
for k in keys.iter() {
74-
set.insert(*k);
75-
76-
prop_assert_eq!(Some(*k), set.iter_upper_bound(&(k + 1)).next());
77-
}
78-
79-
Ok(())
80-
});
81-
}
82-
8370
// Given an operation, executes it on the given stable btreeset and standard btreeset, verifying
8471
// that the result of the operation is equal in both btrees.
8572
fn execute_operation<M: Memory>(

src/lib.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@ mod base_vec;
33
pub mod btreemap;
44
pub mod cell;
55
pub use cell::{Cell as StableCell, Cell};
6+
pub mod btreeset;
67
pub mod file_mem;
78
#[cfg(target_arch = "wasm32")]
89
mod ic0_memory; // Memory API for canisters.
910
pub mod log;
10-
pub use log::{Log as StableLog, Log};
11-
pub mod btreeset;
1211
pub mod memory_manager;
1312
pub mod min_heap;
1413
pub mod reader;
@@ -17,20 +16,22 @@ pub mod storable;
1716
mod tests;
1817
mod types;
1918
pub mod vec;
20-
pub use min_heap::{MinHeap, MinHeap as StableMinHeap};
21-
pub use vec::{Vec as StableVec, Vec};
2219
pub mod vec_mem;
2320
pub mod writer;
21+
2422
pub use btreemap::{BTreeMap, BTreeMap as StableBTreeMap};
2523
pub use btreeset::{BTreeSet, BTreeSet as StableBTreeSet};
2624
pub use file_mem::FileMemory;
2725
#[cfg(target_arch = "wasm32")]
2826
pub use ic0_memory::Ic0StableMemory;
27+
pub use log::{Log as StableLog, Log};
28+
pub use min_heap::{MinHeap, MinHeap as StableMinHeap};
2929
use std::error;
3030
use std::fmt::{Display, Formatter};
3131
use std::mem::MaybeUninit;
3232
pub use storable::Storable;
3333
use types::Address;
34+
pub use vec::{Vec as StableVec, Vec};
3435
pub use vec_mem::VectorMemory;
3536

3637
#[cfg(target_arch = "wasm32")]

0 commit comments

Comments
 (0)