Repository: camshaft/bolero (c)

Issue:
Shrinker runs once when `shrink_time` is zero.
I tried setting `.with_shrink_time(ZERO)` to fully disable input shrinking for a proptest, but noticed that the shrinker only checks if the shrinking time is exhausted after running its first iteration: [shrink.rs#L100-L103](https://github.com/camshaft/bolero/blob/master/lib/bolero-engine/src/shrink.rs#L100-L103). In my case this causes a rather large delay whenever there is a test failure, which is annoying during test development.

Is there some other feature besides adjusting the shrinking time for fully disabling shrinking?

If not, do you think [this approach](https://github.com/mkforsb/bolero/commit/996274a7ac116c2676dcf82beb11d56997022ff1) could be viable for a PR?

Pull request:
Disable shrinker when `shrink_time` is zero
Fixes #248.

1. Early return `None` in `Shrinker::shrink` for zero shrink time.
2. Avoid a little bit of prep work in `TestEngine` by checking shrink time in `rng_options`.

Patch:
diff --git a/lib/bolero-engine/src/shrink.rs b/lib/bolero-engine/src/shrink.rs
index e45b612..0ca2451 100644
--- a/lib/bolero-engine/src/shrink.rs
+++ b/lib/bolero-engine/src/shrink.rs
@@ -82,6 +82,10 @@ impl<'a, T: Test, I: Input> Shrinker<'a, T, I> {
     }
 
     fn shrink(mut self) -> Option<Failure<T::Value>> {
+        if self.options.shrink_time_or_default().is_zero() {
+            return None;
+        }
+
         panic::set_hook();
         let forward_panic = panic::forward_panic(false);
         let capture_backtrace = panic::capture_backtrace(false);


Test patch:
diff --git a/lib/bolero-engine/src/shrink/tests.rs b/lib/bolero-engine/src/shrink/tests.rs
index 9e0cf52..55cfc90 100644
--- a/lib/bolero-engine/src/shrink/tests.rs
+++ b/lib/bolero-engine/src/shrink/tests.rs
@@ -1,8 +1,24 @@
 use super::*;
 use std::time::Duration;
 
+macro_rules! shrink_test_assert {
+    ($test:ident, $input:ident, $options:ident, (None)) => {
+        assert!(Shrinker::new(&mut $test, $input, None, &$options)
+            .shrink()
+            .is_none());
+    };
+
+    ($test:ident, $input:ident, $options:ident, ($($expected:tt)+)) => {
+        let failure = Shrinker::new(&mut $test, $input, None, &$options)
+            .shrink()
+            .expect("should produce a result");
+
+        assert_eq!(failure.input, $($expected)+);
+    };
+}
+
 macro_rules! shrink_test {
-    ($name:ident, $gen:expr, $input:expr, $expected:expr, $check:expr) => {
+    ($name:ident, $gen:expr, $input:expr, $duration:expr, ($($expected:tt)+), $check:expr) => {
         #[test]
         fn $name() {
             #[allow(unused_imports)]
@@ -14,31 +30,51 @@ macro_rules! shrink_test {
             let mut test = crate::ClonedGeneratorTest::new($check, $gen);
             let input = ($input).to_vec();
 
-            let options = driver::Options::default().with_shrink_time(Duration::from_secs(1));
+            let options = driver::Options::default().with_shrink_time($duration);
 
-            let failure = Shrinker::new(&mut test, input, None, &options)
-                .shrink()
-                .expect("should produce a result");
-
-            assert_eq!(failure.input, $expected);
+            shrink_test_assert!(test, input, options, ($($expected)+));
         }
     };
 }
 
-shrink_test!(u16_shrink_test, gen::<u16>(), [255u8; 2], 1, |value| {
-    assert!(value < 20);
-    assert!(value % 7 == 0);
-});
+shrink_test!(
+    u16_shrink_test_zero_time,
+    gen::<u16>(),
+    [255u8; 2],
+    Duration::ZERO,
+    (None),
+    |_| {}
+);
+
+shrink_test!(
+    u16_shrink_test,
+    gen::<u16>(),
+    [255u8; 2],
+    Duration::from_secs(1),
+    (1),
+    |value| {
+        assert!(value < 20);
+        assert!(value % 7 == 0);
+    }
+);
 
-shrink_test!(u32_shrink_test, gen::<u32>(), [255u8; 4], 20, |value| {
-    assert!(value < 20);
-});
+shrink_test!(
+    u32_shrink_test,
+    gen::<u32>(),
+    [255u8; 4],
+    Duration::from_secs(1),
+    (20),
+    |value| {
+        assert!(value < 20);
+    }
+);
 
 shrink_test!(
     vec_shrink_test,
     gen::<Vec<u32>>().filter_gen(|vec| vec.len() >= 3),
     [255u8; 256],
-    vec![4, 0, 0],
+    Duration::from_secs(1),
+    (vec![4, 0, 0]),
     |value: Vec<u32>| {
         assert!(value[0] < 4);
         assert!(value[1] < 5);
@@ -50,7 +86,8 @@ shrink_test!(
     non_start_vec_shrink_test,
     gen::<Vec<u32>>().filter_gen(|vec| vec.len() >= 3),
     [255u8; 256],
-    vec![0, 5, 0],
+    Duration::from_secs(1),
+    (vec![0, 5, 0]),
     |value: Vec<u32>| {
         assert!(value[1] < 5);
         assert!(value[2] < 6);
@@ -61,7 +98,8 @@ shrink_test!(
     middle_vec_shrink_test,
     gen::<Vec<u8>>().filter_gen(|vec| vec.len() >= 3),
     [255u8; 256],
-    vec![1, 1, 1],
+    Duration::from_secs(1),
+    (vec![1, 1, 1]),
     |value: Vec<u8>| {
         if value[0] > 0 && *value.last().unwrap() > 0 {
             assert_eq!(value[1], 0);
diff --git a/lib/bolero/src/test/mod.rs b/lib/bolero/src/test/mod.rs
index e454286..ce7e2c0 100644
--- a/lib/bolero/src/test/mod.rs
+++ b/lib/bolero/src/test/mod.rs
@@ -239,11 +239,15 @@ impl TestEngine {
                 input::Test::Rng(conf) => {
                     let mut input = conf.input(&mut buffer, &mut cache, rng_options);
                     test.test(&mut input).map_err(|error| {
-                        // reseed the input and buffer the rng for shrinking
-                        let mut input = conf.buffered_input(&mut buffer, rng_options);
-                        let _ = test.generate_value(&mut input);
+                        let shrunken = if rng_options.shrink_time_or_default().is_zero() {
+                            None
+                        } else {
+                            // reseed the input and buffer the rng for shrinking
+                            let mut input = conf.buffered_input(&mut buffer, rng_options);
+                            let _ = test.generate_value(&mut input);
 
-                        let shrunken = test.shrink(buffer.clone(), data.seed(), rng_options);
+                            test.shrink(buffer.clone(), data.seed(), rng_options)
+                        };
 
                         if let Some(shrunken) = shrunken {
                             format!("{:#}", shrunken)
diff --git a/lib/bolero/src/tests.rs b/lib/bolero/src/tests.rs
index f8efb52..ef09391 100644
--- a/lib/bolero/src/tests.rs
+++ b/lib/bolero/src/tests.rs
@@ -80,3 +80,54 @@ fn with_test_time() {
         });
     assert!(num_iters.load(Ordering::Relaxed) > 10);
 }
+
+#[test]
+fn with_shrinking() {
+    use std::sync::atomic::Ordering;
+    let last_seen_value = std::sync::atomic::AtomicU8::new(0);
+
+    std::panic::catch_unwind(|| {
+        check!()
+            .with_generator(gen::<u8>())
+            .with_shrink_time(Duration::from_secs(10))
+            .for_each(|value| {
+                last_seen_value.store(*value, Ordering::Relaxed);
+                assert!(*value == 0)
+            });
+    })
+    .unwrap_err();
+
+    assert_eq!(last_seen_value.load(Ordering::Relaxed), 1);
+}
+
+#[test]
+fn without_shrinking() {
+    use std::sync::atomic::Ordering;
+
+    let max_seen_value = std::sync::atomic::AtomicU8::new(0);
+    let n = 20; // P(false negative) = 1/(256^n) assuming uniform gen::<u8>
+
+    for _ in 0..n {
+        let last_seen_value = std::sync::atomic::AtomicU8::new(0);
+
+        std::panic::catch_unwind(|| {
+            check!()
+                .with_generator(gen::<u8>())
+                .with_shrink_time(Duration::ZERO)
+                .for_each(|value| {
+                    last_seen_value.store(*value, Ordering::Relaxed);
+                    assert!(*value == 0)
+                });
+        })
+        .unwrap_err();
+
+        let last = last_seen_value.load(Ordering::Relaxed);
+        let max = max_seen_value.load(Ordering::Relaxed);
+
+        if last > max {
+            max_seen_value.store(last, Ordering::Relaxed);
+        }
+    }
+
+    assert!(max_seen_value.load(Ordering::Relaxed) > 1);
+}