Make tests pass and improve shrinking for prop_flat_map

proptest-derive/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name          = "proptest-derive"
-version       = "0.3.0"
+version       = "0.3.1"
 authors       = ["Mazdak Farrokhzad <twingoow@gmail.com>"]
 license       = "MIT/Apache-2.0"
 readme        = "README.md"
@@ -22,12 +22,12 @@ edition = "2018"
 proc-macro = true
 
 [dev-dependencies]
-proptest = { version = "1.0.0", path = "../proptest" }
+proptest = { version = "1.1.0", path = "../proptest" }
 # We don't actually run the tests on stable since some of them use nightly
 # features. However, due to
 # https://github.com/laumann/compiletest-rs/issues/166, the default features of
 # compiletest-rs fail to compile, but the stable fallback works fine.
-compiletest_rs = { version = "0.3.19", features = ["tmp", "stable"] }
+compiletest_rs = { version = "0.8.0", features = ["tmp", "stable"] }
 # criterion is used for benchmarks.
 criterion = "0.2"
 

proptest-derive/tests/compile-fail/no-arbitrary.rs

@@ -13,5 +13,5 @@ fn main() {}
 
 struct T0;
 
-#[derive(Debug, Arbitrary)] //~ Arbitrary` is not satisfied [E0277]
-struct T1 { f0: T0, }
+#[derive(Debug, Arbitrary)] //~ ERROR Arbitrary` is not satisfied [E0277]
+struct T1 { f0: T0, }       //~ ERROR Arbitrary` is not satisfied [E0277]

proptest-derive/tests/compiletest.rs

@@ -17,7 +17,7 @@ fn run_mode(src: &'static str, mode: &'static str) {
     config.target_rustcflags =
         Some("-L ../target/debug/deps --edition=2018".to_owned());
     if let Ok(name) = env::var("TESTNAME") {
-        config.filter = Some(name);
+        config.filters = vec![name];
     }
     config.src_base = format!("tests/{}", src).into();
 

proptest/CHANGELOG.md

@@ -1,3 +1,20 @@
+## Unreleased
+
+### Breaking Changes
+
+- `prop_flat_map` can only be used with closures that produce a strategy that
+  implements `Copy`.
+
+- Half-bounded ranges (`x..`, `..x`, and `..=x`) are no longer supported for
+  `f32` and `f64`.
+
+### New Features
+
+- The shrink strategy for `prop_flat_map` now shrinks the "outer" strategy
+  first (the one that produces the values given to `prop_flat_map`) and
+  shrinks the "inner" strategy" (the one produced by `prop_flat_map`) only
+  when the outer strategy cannot be shrunk further.
+
 ## 1.0.0
 
 ### Breaking Changes

proptest/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "proptest"
-version = "1.0.0"
+version = "1.1.0"
 authors = ["Jason Lingle"]
 license = "MIT/Apache-2.0"
 readme = "README.md"

proptest/src/num.rs

@@ -53,7 +53,7 @@ macro_rules! int_any {
     };
 }
 
-macro_rules! numeric_api {
+macro_rules! int_api {
     ($typ:ident, $epsilon:expr) => {
         impl Strategy for ::core::ops::Range<$typ> {
             type Tree = BinarySearch;
@@ -137,6 +137,40 @@ macro_rules! numeric_api {
     };
 }
 
+macro_rules! float_api {
+    ($typ:ident, $epsilon:expr) => {
+        impl Strategy for ::core::ops::Range<$typ> {
+            type Tree = BinarySearch;
+            type Value = $typ;
+
+            fn new_tree(&self, runner: &mut TestRunner) -> NewTree<Self> {
+                Ok(BinarySearch::new_clamped(
+                    self.start,
+                    $crate::num::sample_uniform(runner, self.clone()),
+                    self.end - $epsilon,
+                ))
+            }
+        }
+
+        impl Strategy for ::core::ops::RangeInclusive<$typ> {
+            type Tree = BinarySearch;
+            type Value = $typ;
+
+            fn new_tree(&self, runner: &mut TestRunner) -> NewTree<Self> {
+                Ok(BinarySearch::new_clamped(
+                    *self.start(),
+                    $crate::num::sample_uniform_incl(
+                        runner,
+                        *self.start(),
+                        *self.end(),
+                    ),
+                    *self.end(),
+                ))
+            }
+        }
+    };
+}
+
 macro_rules! signed_integer_bin_search {
     ($typ:ident) => {
         #[allow(missing_docs)]
@@ -234,7 +268,7 @@ macro_rules! signed_integer_bin_search {
                 }
             }
 
-            numeric_api!($typ, 1);
+            int_api!($typ, 1);
         }
     };
 }
@@ -322,7 +356,7 @@ macro_rules! unsigned_integer_bin_search {
                 }
             }
 
-            numeric_api!($typ, 1);
+            int_api!($typ, 1);
         }
     };
 }
@@ -842,7 +876,7 @@ macro_rules! float_bin_search {
                 }
             }
 
-            numeric_api!($typ, 0.0);
+            float_api!($typ, 0.0);
         }
     };
 }
@@ -1032,7 +1066,7 @@ mod test {
     }
 
     mod contract_sanity {
-        macro_rules! contract_sanity {
+        macro_rules! contract_sanity_int {
             ($t:tt) => {
                 mod $t {
                     use crate::strategy::check_strategy_sanity;
@@ -1067,18 +1101,40 @@ mod test {
                 }
             };
         }
-        contract_sanity!(u8);
-        contract_sanity!(i8);
-        contract_sanity!(u16);
-        contract_sanity!(i16);
-        contract_sanity!(u32);
-        contract_sanity!(i32);
-        contract_sanity!(u64);
-        contract_sanity!(i64);
-        contract_sanity!(usize);
-        contract_sanity!(isize);
-        contract_sanity!(f32);
-        contract_sanity!(f64);
+
+        macro_rules! contract_sanity_float {
+            ($t:tt) => {
+                mod $t {
+                    use crate::strategy::check_strategy_sanity;
+
+                    const FOURTY_TWO: $t = 42 as $t;
+                    const FIFTY_SIX: $t = 56 as $t;
+
+                    #[test]
+                    fn range() {
+                        check_strategy_sanity(FOURTY_TWO..FIFTY_SIX, None);
+                    }
+
+                    #[test]
+                    fn range_inclusive() {
+                        check_strategy_sanity(FOURTY_TWO..=FIFTY_SIX, None);
+                    }
+                }
+            };
+        }
+
+        contract_sanity_int!(u8);
+        contract_sanity_int!(i8);
+        contract_sanity_int!(u16);
+        contract_sanity_int!(i16);
+        contract_sanity_int!(u32);
+        contract_sanity_int!(i32);
+        contract_sanity_int!(u64);
+        contract_sanity_int!(i64);
+        contract_sanity_int!(usize);
+        contract_sanity_int!(isize);
+        contract_sanity_float!(f32);
+        contract_sanity_float!(f64);
     }
 
     #[test]

proptest/src/strategy/flatten.rs

@@ -8,11 +8,11 @@
 // except according to those terms.
 
 use crate::std_facade::{fmt, Arc};
-use core::mem;
 
 use crate::strategy::fuse::Fuse;
 use crate::strategy::traits::*;
 use crate::test_runner::*;
+use std::mem;
 
 /// Adaptor that flattens a `Strategy` which produces other `Strategy`s into a
 /// `Strategy` that picks one of those strategies and then picks values from
@@ -33,6 +33,7 @@ impl<S: Strategy> Flatten<S> {
 impl<S: Strategy> Strategy for Flatten<S>
 where
     S::Value: Strategy,
+    <S::Value as Strategy>::Tree: Clone,
 {
     type Tree = FlattenValueTree<S::Tree>;
     type Value = <S::Value as Strategy>::Value;
@@ -50,19 +51,7 @@ where
 {
     meta: Fuse<S>,
     current: Fuse<<S::Value as Strategy>::Tree>,
-    // The final value to produce after successive calls to complicate() on the
-    // underlying objects return false.
-    final_complication: Option<Fuse<<S::Value as Strategy>::Tree>>,
-    // When `simplify()` or `complicate()` causes a new `Strategy` to be
-    // chosen, we need to find a new failing input for that case. To do this,
-    // we implement `complicate()` by regenerating values up to a number of
-    // times corresponding to the maximum number of test cases. A `simplify()`
-    // which does not cause a new strategy to be chosen always resets
-    // `complicate_regen_remaining` to 0.
-    //
-    // This does unfortunately depart from the direct interpretation of
-    // simplify/complicate as binary search, but is still easier to think about
-    // than other implementations of higher-order strategies.
+    last_complication: Option<Fuse<<S::Value as Strategy>::Tree>>,
     runner: TestRunner,
     complicate_regen_remaining: u32,
 }
@@ -77,7 +66,7 @@ where
         FlattenValueTree {
             meta: self.meta.clone(),
             current: self.current.clone(),
-            final_complication: self.final_complication.clone(),
+            last_complication: self.last_complication.clone(),
             runner: self.runner.clone(),
             complicate_regen_remaining: self.complicate_regen_remaining,
         }
@@ -94,7 +83,7 @@ where
         f.debug_struct("FlattenValueTree")
             .field("meta", &self.meta)
             .field("current", &self.current)
-            .field("final_complication", &self.final_complication)
+            .field("last_complication", &self.last_complication)
             .field(
                 "complicate_regen_remaining",
                 &self.complicate_regen_remaining,
@@ -112,7 +101,7 @@ where
         Ok(FlattenValueTree {
             meta: Fuse::new(meta),
             current: Fuse::new(current),
-            final_complication: None,
+            last_complication: None,
             runner: runner.partial_clone(),
             complicate_regen_remaining: 0,
         })
@@ -122,6 +111,7 @@ where
 impl<S: ValueTree> ValueTree for FlattenValueTree<S>
 where
     S::Value: Strategy,
+    <S::Value as Strategy>::Tree: Clone,
 {
     type Value = <S::Value as Strategy>::Value;
 
@@ -130,33 +120,28 @@ where
     }
 
     fn simplify(&mut self) -> bool {
+        self.current.disallow_complicate();
+
+        if self.meta.simplify() {
+            if let Ok(v) = self.meta.current().new_tree(&mut self.runner) {
+                self.last_complication = Some(Fuse::new(v));
+                mem::swap(
+                    self.last_complication.as_mut().unwrap(),
+                    &mut self.current,
+                );
+                self.complicate_regen_remaining = self.runner.config().cases;
+                return true;
+            } else {
+                self.meta.disallow_simplify();
+            }
+        }
+
         self.complicate_regen_remaining = 0;
+        let mut old_current = self.current.clone();
+        old_current.disallow_simplify();
 
         if self.current.simplify() {
-            // Now that we've simplified the derivative value, we can't
-            // re-complicate the meta value unless it gets simplified again.
-            // We also mustn't complicate back to whatever's in
-            // `final_complication` since the new state of `self.current` is
-            // the most complicated state.
-            self.meta.disallow_complicate();
-            self.final_complication = None;
-            true
-        } else if !self.meta.simplify() {
-            false
-        } else if let Ok(v) = self.meta.current().new_tree(&mut self.runner) {
-            // Shift current into final_complication and `v` into
-            // `current`. We also need to prevent that value from
-            // complicating beyond the current point in the future
-            // since we're going to return `true` from `simplify()`
-            // ourselves.
-            self.current.disallow_complicate();
-            self.final_complication = Some(Fuse::new(v));
-            mem::swap(
-                self.final_complication.as_mut().unwrap(),
-                &mut self.current,
-            );
-            // Initially complicate by regenerating the chosen value.
-            self.complicate_regen_remaining = self.runner.config().cases;
+            self.last_complication = Some(old_current);
             true
         } else {
             false
@@ -177,17 +162,20 @@ where
             }
         }
 
-        if self.current.complicate() {
-            return true;
-        } else if self.meta.complicate() {
+        if self.meta.complicate() {
             if let Ok(v) = self.meta.current().new_tree(&mut self.runner) {
-                self.complicate_regen_remaining = self.runner.config().cases;
                 self.current = Fuse::new(v);
+                self.complicate_regen_remaining = self.runner.config().cases;
                 return true;
+            } else {
             }
         }
 
-        if let Some(v) = self.final_complication.take() {
+        if self.current.complicate() {
+            return true;
+        }
+
+        if let Some(v) = self.last_complication.take() {
             self.current = v;
             true
         } else {