refactor: Use test_strategy over proptest macro in replicated state t…

…ests (#3462)

rs/replicated_state/src/canister_state/queues/queue/tests.rs

@@ -218,83 +218,77 @@ fn canister_queue_push_without_reserved_slot_panics() {
 /// Generator for an arbitrary inbound message reference.
 fn arbitrary_message_reference() -> impl Strategy<Value = InboundReference> + Clone {
     prop_oneof![
-        1 => any::<u64>().prop_map(|gen| new_request_reference(gen, Class::GuaranteedResponse)),
-        1 => any::<u64>().prop_map(|gen| new_request_reference(gen, Class::BestEffort)),
-        1 => any::<u64>().prop_map(|gen| new_response_reference(gen, Class::GuaranteedResponse)),
-        1 => any::<u64>().prop_map(|gen| new_response_reference(gen, Class::BestEffort)),
+        any::<u64>().prop_map(|gen| new_request_reference(gen, Class::GuaranteedResponse)),
+        any::<u64>().prop_map(|gen| new_request_reference(gen, Class::BestEffort)),
+        any::<u64>().prop_map(|gen| new_response_reference(gen, Class::GuaranteedResponse)),
+        any::<u64>().prop_map(|gen| new_response_reference(gen, Class::BestEffort)),
     ]
 }
 
-proptest! {
-    #[test]
-    fn canister_queue_push_and_pop(
-        mut references in proptest::collection::vec_deque(
-            arbitrary_message_reference(),
-            10..20,
-        )
-    ) {
-        // Create a queue with large enough capacity.
-        let mut queue = InputQueue::new(20);
-
-        // Push all references onto the queue.
-        for reference in references.iter() {
-            match reference {
-                reference if reference.kind() == Kind::Request => {
-                    queue.push_request(*reference);
-                }
-                reference => {
-                    queue.try_reserve_response_slot().unwrap();
-                    queue.push_response(*reference);
-                }
+#[test_strategy::proptest]
+fn canister_queue_push_and_pop(
+    #[strategy(proptest::collection::vec_deque(arbitrary_message_reference(), 10..20))]
+    mut references: VecDeque<InboundReference>,
+) {
+    // Create a queue with large enough capacity.
+    let mut queue = InputQueue::new(20);
+
+    // Push all references onto the queue.
+    for reference in references.iter() {
+        match reference {
+            reference if reference.kind() == Kind::Request => {
+                queue.push_request(*reference);
+            }
+            reference => {
+                queue.try_reserve_response_slot().unwrap();
+                queue.push_response(*reference);
             }
-            prop_assert_eq!(Ok(()), queue.check_invariants());
-        }
-
-        // Check the contents of the queue via `peek` and `pop`.
-        while let Some(r) = queue.peek() {
-            let reference = references.pop_front();
-            prop_assert_eq!(reference, Some(r));
-            prop_assert_eq!(reference, queue.pop());
         }
+        prop_assert_eq!(Ok(()), queue.check_invariants());
+    }
 
-        // All references should have been consumed.
-        prop_assert!(references.is_empty());
+    // Check the contents of the queue via `peek` and `pop`.
+    while let Some(r) = queue.peek() {
+        let reference = references.pop_front();
+        prop_assert_eq!(reference, Some(r));
+        prop_assert_eq!(reference, queue.pop());
     }
 
-    #[test]
-    fn encode_roundtrip(
-        references in proptest::collection::vec_deque(
-            arbitrary_message_reference(),
-            10..20,
-        ),
-        reserved_slots in 0..3
-    ) {
-        let mut queue = CanisterQueue::new(DEFAULT_QUEUE_CAPACITY);
-
-        // Push all references onto the queue.
-        for reference in references.iter() {
-            match reference {
-                reference if reference.kind() == Kind::Request => {
-                    queue.push_request(*reference);
-                }
-                reference => {
-                    queue.try_reserve_response_slot().unwrap();
-                    queue.push_response(*reference);
-                }
+    // All references should have been consumed.
+    prop_assert!(references.is_empty());
+}
+
+#[test_strategy::proptest]
+fn encode_roundtrip(
+    #[strategy(proptest::collection::vec_deque(arbitrary_message_reference(), 10..20))]
+    references: VecDeque<InboundReference>,
+    #[strategy(0..3)] reserved_slots: i32,
+) {
+    let mut queue = CanisterQueue::new(DEFAULT_QUEUE_CAPACITY);
+
+    // Push all references onto the queue.
+    for reference in references.iter() {
+        match reference {
+            reference if reference.kind() == Kind::Request => {
+                queue.push_request(*reference);
+            }
+            reference => {
+                queue.try_reserve_response_slot().unwrap();
+                queue.push_response(*reference);
             }
-            prop_assert_eq!(Ok(()), queue.check_invariants());
-        }
-        // And make `reserved_slots` additional reservations.
-        for _ in 0..reserved_slots {
-            queue.try_reserve_response_slot().unwrap();
         }
         prop_assert_eq!(Ok(()), queue.check_invariants());
+    }
+    // And make `reserved_slots` additional reservations.
+    for _ in 0..reserved_slots {
+        queue.try_reserve_response_slot().unwrap();
+    }
+    prop_assert_eq!(Ok(()), queue.check_invariants());
 
-        let encoded: pb_queues::CanisterQueue = (&queue).into();
-        let decoded = encoded.try_into().unwrap();
+    let encoded: pb_queues::CanisterQueue = (&queue).into();
+    let decoded = encoded.try_into().unwrap();
 
-        assert_eq!(queue, decoded);
-    }
+    prop_assert_eq!(queue, decoded);
 }
 
 #[test]

rs/replicated_state/src/canister_state/system_state/wasm_chunk_store.rs

@@ -374,23 +374,23 @@ mod tests {
         const MB: usize = 1024 * 1024;
         const MAX_SIZE: NumBytes = NumBytes::new(20 * MB as u64);
 
-        proptest! {
-            #[test]
-            // Try chunks 2x as big as the size limit.
-            // If all inserts below the size limit succeeded, we'd expect 50 *
-            // .5 MiB = 25 MiB total. So set the max size below that to
-            // evenutally hit the size limit.
-            fn insert_result_matches_can_insert(vecs in prop_vec((any::<u8>(), 0..2 * MB), 100)) {
-                let mut store = WasmChunkStore::new_for_testing();
-                for (byte, length) in vecs {
-                    let chunk = vec![byte; length];
-                    let check = store.can_insert_chunk(MAX_SIZE, &chunk);
-                    let hash = store.insert_chunk(MAX_SIZE, &chunk);
-                    if hash.is_ok() {
-                        assert_eq!(check, Ok(()));
-                    } else {
-                        assert_eq!(check.unwrap_err(), hash.unwrap_err());
-                    }
+        #[test_strategy::proptest]
+        // Try chunks 2x as big as the size limit.
+        // If all inserts below the size limit succeeded, we'd expect 50 *
+        // .5 MiB = 25 MiB total. So set the max size below that to
+        // evenutally hit the size limit.
+        fn insert_result_matches_can_insert(
+            #[strategy(prop_vec((any::<u8>(), 0..2 * MB), 100))] vecs: Vec<(u8, usize)>,
+        ) {
+            let mut store = WasmChunkStore::new_for_testing();
+            for (byte, length) in vecs {
+                let chunk = vec![byte; length];
+                let check = store.can_insert_chunk(MAX_SIZE, &chunk);
+                let hash = store.insert_chunk(MAX_SIZE, &chunk);
+                if hash.is_ok() {
+                    prop_assert_eq!(check, Ok(()));
+                } else {
+                    prop_assert_eq!(check.unwrap_err(), hash.unwrap_err());
                 }
             }
         }

rs/replicated_state/src/metadata_state/tests.rs

@@ -2309,69 +2309,65 @@ const BATCH_TIME_RANGE: Range<u64> = (u64::MAX / 2)..(u64::MAX / 2 + MAX_NUM_DAY
 #[allow(dead_code)]
 const NODE_ID_RANGE: Range<u64> = 0..20;
 
-proptest! {
-    /// Checks that `check_soft_invariants()` does not return an error when observing random
-    /// node IDs at random mostly sorted and slightly permuted timestamps.
-    /// Such invariants are checked indirectly at the bottom of `observe()` where
-    /// `check_soft_invariants()` is called. There is an additional call to
-    /// `check_soft_invariants()` at the end of the test to ensure the test doesn't
-    /// silently pass when the production code is changed.
-    /// Querying `metrics_since()` is also checked using completely random time stamps to
-    /// ensure there are no hidden panics.
-    #[test]
-    fn blockmaker_metrics_check_soft_invariants(
-        (mut time_u64, random_time_u64, node_ids_u64) in (0..MAX_NUM_DAYS)
-        .prop_flat_map(|num_elements| {
-            (
-                proptest::collection::vec(BATCH_TIME_RANGE, num_elements),
-                proptest::collection::vec(any::<u64>(), num_elements),
-                proptest::collection::vec(NODE_ID_RANGE, num_elements),
-            )
-        })
-    ) {
-        // Sort timestamps, then slightly permute them by inserting some
-        // duplicates and swapping elements in some places.
-        time_u64.sort();
-        if !time_u64.is_empty() {
-            for index in 0..(time_u64.len() - 1) {
-                if time_u64[index] % 23 == 0 {
-                    time_u64[index + 1] = time_u64[index];
-                }
-                if time_u64[index] % 27 == 0 {
-                    time_u64.swap(index, index + 1);
-                }
+/// Checks that `check_soft_invariants()` does not return an error when observing random
+/// node IDs at random mostly sorted and slightly permuted timestamps.
+/// Such invariants are checked indirectly at the bottom of `observe()` where
+/// `check_soft_invariants()` is called. There is an additional call to
+/// `check_soft_invariants()` at the end of the test to ensure the test doesn't
+/// silently pass when the production code is changed.
+/// Querying `metrics_since()` is also checked using completely random time stamps to
+/// ensure there are no hidden panics.
+#[test_strategy::proptest]
+fn blockmaker_metrics_check_soft_invariants(
+    #[strategy(0..MAX_NUM_DAYS)] _num_elements: usize,
+    #[strategy(proptest::collection::vec(BATCH_TIME_RANGE, #_num_elements))] mut time_u64: Vec<u64>,
+    #[strategy(proptest::collection::vec(any::<u64>(), #_num_elements))] random_time_u64: Vec<u64>,
+    #[strategy(proptest::collection::vec(NODE_ID_RANGE, #_num_elements))] node_ids_u64: Vec<u64>,
+) {
+    // Sort timestamps, then slightly permute them by inserting some
+    // duplicates and swapping elements in some places.
+    time_u64.sort();
+    if !time_u64.is_empty() {
+        for index in 0..(time_u64.len() - 1) {
+            if time_u64[index] % 23 == 0 {
+                time_u64[index + 1] = time_u64[index];
+            }
+            if time_u64[index] % 27 == 0 {
+                time_u64.swap(index, index + 1);
             }
         }
+    }
 
-        let mut metrics = BlockmakerMetricsTimeSeries::default();
-        // Observe a unique node ID first to ensure the pruning process
-        // is triggered once the metrics reach capacity.
+    let mut metrics = BlockmakerMetricsTimeSeries::default();
+    // Observe a unique node ID first to ensure the pruning process
+    // is triggered once the metrics reach capacity.
+    metrics.observe(
+        Time::from_nanos_since_unix_epoch(0),
+        &BlockmakerMetrics {
+            blockmaker: node_test_id(NODE_ID_RANGE.end + 10),
+            failed_blockmakers: vec![],
+        },
+    );
+    // Observe random node IDs at random increasing timestamps; `check_runtime_invariants()`
+    // will be triggered passively each time `observe()` is called.
+    // Additionally, query snapshots at random times and consume the iterator to ensure
+    // there are no hidden panics in `metrics_since()`.
+    for ((batch_time_u64, query_time_u64), node_id_u64) in time_u64
+        .into_iter()
+        .zip(random_time_u64.into_iter())
+        .zip(node_ids_u64.into_iter())
+    {
         metrics.observe(
-            Time::from_nanos_since_unix_epoch(0),
+            Time::from_nanos_since_unix_epoch(batch_time_u64),
             &BlockmakerMetrics {
-                blockmaker: node_test_id(NODE_ID_RANGE.end + 10),
-                failed_blockmakers: vec![],
-            }
+                blockmaker: node_test_id(node_id_u64),
+                failed_blockmakers: vec![node_test_id(node_id_u64 + 1)],
+            },
         );
-        // Observe random node IDs at random increasing timestamps; `check_runtime_invariants()`
-        // will be triggered passively each time `observe()` is called.
-        // Additionally, query snapshots at random times and consume the iterator to ensure
-        // there are no hidden panics in `metrics_since()`.
-        for ((batch_time_u64, query_time_u64), node_id_u64) in time_u64
-            .into_iter()
-            .zip(random_time_u64.into_iter())
-            .zip(node_ids_u64.into_iter())
-        {
-            metrics.observe(
-                Time::from_nanos_since_unix_epoch(batch_time_u64),
-                &BlockmakerMetrics {
-                    blockmaker: node_test_id(node_id_u64),
-                    failed_blockmakers: vec![node_test_id(node_id_u64 + 1)],
-                }
-            );
-            metrics.metrics_since(Time::from_nanos_since_unix_epoch(query_time_u64)).count();
-        }
-
-        prop_assert!(metrics.check_soft_invariants().is_ok());
+        metrics
+            .metrics_since(Time::from_nanos_since_unix_epoch(query_time_u64))
+            .count();
     }
+
+    prop_assert!(metrics.check_soft_invariants().is_ok());
 }

rs/replicated_state/src/page_map/storage/tests.rs

@@ -1566,10 +1566,8 @@ mod proptest_tests {
         prop_vec(instruction_strategy(), 1..20)
     }
 
-    proptest! {
-        #[test]
-        fn random_instructions(instructions in instructions_strategy()) {
-            write_overlays_and_verify(instructions);
-        }
+    #[test_strategy::proptest]
+    fn random_instructions(#[strategy(instructions_strategy())] instructions: Vec<Instruction>) {
+        write_overlays_and_verify(instructions);
     }
 }

rs/replicated_state/tests/metadata_state.rs

@@ -1,26 +1,21 @@
 use ic_protobuf::state::{queues::v1 as pb_queues, system_metadata::v1 as pb_metadata};
+use ic_replicated_state::{metadata_state::SubnetMetrics, Stream};
 use ic_test_utilities_state::{arb_stream, arb_subnet_metrics};
-use proptest::prelude::*;
 use std::convert::TryInto;
 
-proptest! {
-    #[test]
-    fn roundtrip_conversion_stream_proptest(stream in arb_stream(0, 10, 0, 100)) {
-        assert_eq!(
-            stream,
-            pb_queues::Stream::from(&stream)
-                .try_into()
-                .unwrap()
-        );
-    }
+#[test_strategy::proptest]
+fn roundtrip_conversion_stream_proptest(#[strategy(arb_stream(0, 10, 0, 100))] stream: Stream) {
+    assert_eq!(stream, pb_queues::Stream::from(&stream).try_into().unwrap());
+}
 
-    #[test]
-    fn roundtrip_conversion_subnet_metrics(subnet_metrics in arb_subnet_metrics()) {
-        assert_eq!(
-            subnet_metrics,
-            pb_metadata::SubnetMetrics::from(&subnet_metrics)
-                .try_into()
-                .unwrap()
-        );
-    }
+#[test_strategy::proptest]
+fn roundtrip_conversion_subnet_metrics(
+    #[strategy(arb_subnet_metrics())] subnet_metrics: SubnetMetrics,
+) {
+    assert_eq!(
+        subnet_metrics,
+        pb_metadata::SubnetMetrics::from(&subnet_metrics)
+            .try_into()
+            .unwrap()
+    );
 }