Auto merge of #6149 - Eh2406:proptest, r=alexcrichton

proptest basic validation This adds a function for testing that the output of the resolver is basically reasonable. This function has the same signature as the function for running the resolver in a test. So it is easy to switch back and forth, depending on the thoroughness vs speed tradeoff. This also adds a proptest/fuzz that runs this validation against arbitrary registry. cc #6120 Sorry about the cargo fmt.
rust-lang · Oct 8, 2018 · aa06d7c · aa06d7c
2 parents eac8f51 + 1719e84
commit aa06d7c
Show file tree

Hide file tree

Showing 4 changed files with 118 additions and 27 deletions.
diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs
@@ -1,5 +1,7 @@
+use std::borrow::Borrow;
 use std::collections::{HashMap, HashSet};
 use std::fmt;
+use std::hash::Hash;
 use std::iter::FromIterator;
 
 use url::Url;
@@ -161,6 +163,14 @@ unable to verify that `{0}` is the same as when the lockfile was generated
         Ok(())
     }
 
+    pub fn contains<Q: ?Sized>(&self, k: &Q) -> bool
+    where
+        PackageId: Borrow<Q>,
+        Q: Hash + Eq,
+    {
+        self.graph.contains(k)
+    }
+
     pub fn iter(&self) -> impl Iterator<Item = &PackageId> {
         self.graph.iter()
     }

diff --git a/src/cargo/util/graph.rs b/src/cargo/util/graph.rs
@@ -1,3 +1,4 @@
+use std::borrow::Borrow;
 use std::collections::hash_map::HashMap;
 use std::fmt;
 use std::hash::Hash;
@@ -25,6 +26,14 @@ impl<N: Eq + Hash + Clone, E: Default> Graph<N, E> {
             .or_insert_with(Default::default)
     }
 
+    pub fn contains<Q: ?Sized>(&self, k: &Q) -> bool
+    where
+        N: Borrow<Q>,
+        Q: Hash + Eq,
+    {
+        self.nodes.contains_key(k)
+    }
+
     pub fn edge(&self, from: &N, to: &N) -> Option<&E> {
         self.nodes.get(from)?.get(to)
     }

diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs
@@ -8,8 +8,8 @@ use support::project;
 use support::registry::Package;
 use support::resolver::{
     assert_contains, assert_same, dep, dep_kind, dep_loc, dep_req, loc_names, names, pkg, pkg_dep,
-    pkg_id, pkg_loc, registry, registry_strategy, resolve, resolve_with_config,
-    PrettyPrintRegistry, ToDep, ToPkgId,
+    pkg_id, pkg_loc, registry, registry_strategy, resolve, resolve_and_validated,
+    resolve_with_config, PrettyPrintRegistry, ToDep, ToPkgId,
 };
 
 use proptest::collection::vec;
@@ -34,6 +34,22 @@ proptest! {
         .. ProptestConfig::default()
     })]
     #[test]
+    fn passes_validation(
+        PrettyPrintRegistry(input) in registry_strategy(50, 10, 50)
+    )  {
+        let reg = registry(input.clone());
+        // there is only a small chance that eny one
+        // crate will be interesting.
+        // So we try some of the most complicated.
+        for this in input.iter().rev().take(20) {
+            let _ = resolve_and_validated(
+                &pkg_id("root"),
+                vec![dep_req(&this.name(), &format!("={}", this.version()))],
+                &reg,
+            );
+        }
+    }
+    #[test]
     fn limited_independence_of_irrelevant_alternatives(
         PrettyPrintRegistry(input) in registry_strategy(50, 10, 50),
         indexs_to_unpublish in vec(any::<prop::sample::Index>(), 10)
@@ -190,7 +206,8 @@ fn test_resolving_with_same_name() {
             dep_loc("bar", "http://second.example.com"),
         ],
         &reg,
-    ).unwrap();
+    )
+    .unwrap();
 
     let mut names = loc_names(&[
         ("foo", "http://first.example.com"),
@@ -214,7 +231,8 @@ fn test_resolving_with_dev_deps() {
         &pkg_id("root"),
         vec![dep("foo"), dep_kind("baz", Development)],
         &reg,
-    ).unwrap();
+    )
+    .unwrap();
 
     assert_contains(&res, &names(&["root", "foo", "bar", "baz"]));
 }
@@ -251,7 +269,8 @@ fn test_resolving_maximum_version_with_transitive_deps() {
         &pkg_id("root"),
         vec![dep_req("foo", "1.0.0"), dep_req("bar", "1.0.0")],
         &reg,
-    ).unwrap();
+    )
+    .unwrap();
 
     assert_contains(
         &res,
@@ -291,14 +310,16 @@ fn test_resolving_minimum_version_with_transitive_deps() {
             false,
             &None,
             &["minimal-versions".to_string()],
-        ).unwrap();
+        )
+        .unwrap();
 
     let res = resolve_with_config(
         &pkg_id("root"),
         vec![dep_req("foo", "1.0.0"), dep_req("bar", "1.0.0")],
         &reg,
         Some(&config),
-    ).unwrap();
+    )
+    .unwrap();
 
     assert_contains(
         &res,
@@ -332,7 +353,8 @@ fn minimal_version_cli() {
             [dependencies]
             dep = "1.0"
         "#,
-        ).file("src/main.rs", "fn main() {}")
+        )
+        .file("src/main.rs", "fn main() {}")
         .build();
 
     p.cargo("generate-lockfile -Zminimal-versions")
@@ -357,7 +379,8 @@ fn resolving_incompat_versions() {
             &pkg_id("root"),
             vec![dep_req("foo", "=1.0.1"), dep("bar")],
             &reg
-        ).is_err()
+        )
+        .is_err()
     );
 }
 
@@ -498,7 +521,8 @@ fn resolving_with_sys_crates() {
         &pkg_id("root"),
         vec![dep_req("d", "1"), dep_req("r", "1")],
         &reg,
-    ).unwrap();
+    )
+    .unwrap();
 
     assert_contains(
         &res,
@@ -609,7 +633,8 @@ fn resolving_with_many_equivalent_backtracking() {
         &pkg_id("root"),
         vec![dep_req("level0", "*"), dep_req("constrained", "*")],
         &reg,
-    ).unwrap();
+    )
+    .unwrap();
 
     assert_contains(
         &res,
@@ -626,7 +651,8 @@ fn resolving_with_many_equivalent_backtracking() {
         &pkg_id("root"),
         vec![dep_req("level0", "1.0.1"), dep_req("constrained", "*")],
         &reg,
-    ).unwrap();
+    )
+    .unwrap();
 
     assert_contains(
         &res,
@@ -776,7 +802,8 @@ fn resolving_with_constrained_cousins_backtrack() {
         &pkg_id("root"),
         vec![dep_req("level0", "*"), dep_req("constrained", "2.0.0")],
         &reg,
-    ).unwrap();
+    )
+    .unwrap();
 
     assert_contains(
         &res,
@@ -1079,7 +1106,8 @@ fn hard_equality() {
         &pkg_id("root"),
         vec![dep_req("bar", "1"), dep_req("foo", "=1.0.0")],
         &reg,
-    ).unwrap();
+    )
+    .unwrap();
 
     assert_contains(
         &res,

diff --git a/tests/testsuite/support/resolver.rs b/tests/testsuite/support/resolver.rs
@@ -7,10 +7,11 @@ use std::time::{Duration, Instant};
 use cargo::core::dependency::Kind;
 use cargo::core::resolver::{self, Method};
 use cargo::core::source::{GitReference, SourceId};
+use cargo::core::Resolve;
 use cargo::core::{Dependency, PackageId, Registry, Summary};
 use cargo::util::{CargoResult, Config, ToUrl};
 
-use proptest::collection::{btree_map, btree_set, vec};
+use proptest::collection::{btree_map, vec};
 use proptest::prelude::*;
 use proptest::sample::Index;
 use proptest::strategy::ValueTree;
@@ -25,12 +26,52 @@ pub fn resolve(
     resolve_with_config(pkg, deps, registry, None)
 }
 
+pub fn resolve_and_validated(
+    pkg: &PackageId,
+    deps: Vec<Dependency>,
+    registry: &[Summary],
+) -> CargoResult<Vec<PackageId>> {
+    let resolve = resolve_with_config_raw(pkg, deps, registry, None)?;
+    let mut stack = vec![pkg.clone()];
+    let mut used = HashSet::new();
+    let mut links = HashSet::new();
+    while let Some(p) = stack.pop() {
+        assert!(resolve.contains(&p));
+        if used.insert(p.clone()) {
+            // in the tests all `links` crates end in `-sys`
+            if p.name().ends_with("-sys") {
+                assert!(links.insert(p.name()));
+            }
+            stack.extend(resolve.deps(&p).map(|(dp, deps)| {
+                for d in deps {
+                    assert!(d.matches_id(dp));
+                }
+                dp.clone()
+            }));
+        }
+    }
+    let out: Vec<PackageId> = resolve.iter().cloned().collect();
+    assert_eq!(out.len(), used.len());
+    Ok(out)
+}
+
 pub fn resolve_with_config(
     pkg: &PackageId,
     deps: Vec<Dependency>,
     registry: &[Summary],
     config: Option<&Config>,
 ) -> CargoResult<Vec<PackageId>> {
+    let resolve = resolve_with_config_raw(pkg, deps, registry, config)?;
+    let out: Vec<PackageId> = resolve.iter().cloned().collect();
+    Ok(out)
+}
+
+pub fn resolve_with_config_raw(
+    pkg: &PackageId,
+    deps: Vec<Dependency>,
+    registry: &[Summary],
+    config: Option<&Config>,
+) -> CargoResult<Resolve> {
     struct MyRegistry<'a>(&'a [Summary]);
     impl<'a> Registry for MyRegistry<'a> {
         fn query(
@@ -62,7 +103,8 @@ pub fn resolve_with_config(
         &BTreeMap::<String, Vec<String>>::new(),
         None::<String>,
         false,
-    ).unwrap();
+    )
+    .unwrap();
     let method = Method::Everything;
     let start = Instant::now();
     let resolve = resolver::resolve(
@@ -72,13 +114,12 @@ pub fn resolve_with_config(
         &HashSet::new(),
         config,
         false,
-    )?;
+    );
 
     // The largest test in our suite takes less then 30 sec.
     // So lets fail the test if we have ben running for two long.
     assert!(start.elapsed() < Duration::from_secs(60));
-    let res = resolve.iter().cloned().collect();
-    Ok(res)
+    resolve
 }
 
 pub trait ToDep {
@@ -156,7 +197,8 @@ pub fn pkg_dep<T: ToPkgId>(name: T, dep: Vec<Dependency>) -> Summary {
         &BTreeMap::<String, Vec<String>>::new(),
         link,
         false,
-    ).unwrap()
+    )
+    .unwrap()
 }
 
 pub fn pkg_id(name: &str) -> PackageId {
@@ -183,7 +225,8 @@ pub fn pkg_loc(name: &str, loc: &str) -> Summary {
         &BTreeMap::<String, Vec<String>>::new(),
         link,
         false,
-    ).unwrap()
+    )
+    .unwrap()
 }
 
 pub fn dep(name: &str) -> Dependency {
@@ -293,8 +336,8 @@ pub fn registry_strategy(
     let allow_deps = prop::bool::weighted(0.99);
 
     let list_of_versions =
-        btree_set((raw_version, allow_deps), 1..=max_versions).prop_map(move |ver| {
-            ver.iter()
+        btree_map(raw_version, allow_deps, 1..=max_versions).prop_map(move |ver| {
+            ver.into_iter()
                 .map(|a| (version_from_raw(&a.0), a.1))
                 .collect::<Vec<_>>()
         });
@@ -366,7 +409,8 @@ pub fn registry_strategy(
                                 deps
                             },
                         )
-                    }).collect(),
+                    })
+                    .collect(),
             )
         },
     )
@@ -392,9 +436,9 @@ fn meta_test_deep_trees_from_strategy() {
                 &reg,
             );
             dis[res
-                    .as_ref()
-                    .map(|x| min(x.len(), dis.len()) - 1)
-                    .unwrap_or(0)] += 1;
+                .as_ref()
+                .map(|x| min(x.len(), dis.len()) - 1)
+                .unwrap_or(0)] += 1;
             if dis.iter().all(|&x| x > 0) {
                 return;
             }