Auto merge of #6599 - Eh2406:more-tests, r=dwijnand

Tweeks to proptests

This is some small tweaks to the prop test configuration and adds an additional test.

Most of the diff is whitespace.

Author: bors

tests/testsuite/resolve.rs

@@ -8,13 +8,11 @@ use crate::support::project;
 use crate::support::registry::Package;
 use crate::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_and_validated,
+    pkg_id, pkg_loc, registry, registry_strategy, remove_dep, resolve, resolve_and_validated,
     resolve_with_config, PrettyPrintRegistry, ToDep, ToPkgId,
 };
 
-use proptest::collection::vec;
-use proptest::prelude::*;
-use proptest::*;
+use proptest::{prelude::*, *};
 
 /// NOTE: proptest is a form of fuzz testing. It generates random input and makes shore that
 /// certain universal truths are upheld. Therefore, it can pass when there is a problem,
@@ -25,20 +23,15 @@ use proptest::*;
 /// and if you did not change the resolver then feel free to retry without concern.
 proptest! {
     #![proptest_config(ProptestConfig {
-        // Note that this is a little low in terms of cases we'd like to test,
-        // but this number affects how long this function takes. It can be
-        // increased locally to execute more tests and try to find more bugs,
-        // but for now it's semi-low to run in a small-ish amount of time on CI
-        // and locally.
-        cases: 256,
         max_shrink_iters:
             if env::var("CI").is_ok() || !atty::is(atty::Stream::Stderr) {
                 // This attempts to make sure that CI will fail fast,
                 0
             } else {
                 // but that local builds will give a small clear test case.
-                ProptestConfig::default().max_shrink_iters
+                std::u32::MAX
             },
+        result_cache: prop::test_runner::basic_result_cache,
         .. ProptestConfig::default()
     })]
 
@@ -110,11 +103,53 @@ proptest! {
         }
     }
 
+    /// NOTE: if you think this test has failed spuriously see the note at the top of this macro.
+    #[test]
+    fn removing_a_dep_cant_brake(
+            PrettyPrintRegistry(input) in registry_strategy(50, 20, 60),
+            indexs_to_remove in collection::vec((any::<prop::sample::Index>(), any::<prop::sample::Index>()), ..10)
+    ) {
+        let reg = registry(input.clone());
+        let mut removed_input = input.clone();
+        for (summery_idx, dep_idx) in indexs_to_remove {
+            if removed_input.len() > 0 {
+                let summery_idx = summery_idx.index(removed_input.len());
+                let deps = removed_input[summery_idx].dependencies();
+                if deps.len() > 0 {
+                    let new = remove_dep(&removed_input[summery_idx], dep_idx.index(deps.len()));
+                    removed_input[summery_idx] = new;
+                }
+            }
+        }
+        let removed_reg = registry(removed_input);
+        // 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(10) {
+            if resolve(
+                &pkg_id("root"),
+                vec![dep_req(&this.name(), &format!("={}", this.version()))],
+                &reg,
+            ).is_ok() {
+                prop_assert!(
+                    resolve(
+                        &pkg_id("root"),
+                        vec![dep_req(&this.name(), &format!("={}", this.version()))],
+                        &removed_reg,
+                    ).is_ok(),
+                    "full index worked for `{} = \"={}\"` but removing some deps broke it!",
+                    this.name(),
+                    this.version(),
+                )
+            }
+        }
+    }
+
     /// NOTE: if you think this test has failed spuriously see the note at the top of this macro.
     #[test]
     fn limited_independence_of_irrelevant_alternatives(
         PrettyPrintRegistry(input) in registry_strategy(50, 20, 60),
-        indexs_to_unpublish in vec(any::<prop::sample::Index>(), 10)
+        indexs_to_unpublish in collection::vec(any::<prop::sample::Index>(), ..10)
     )  {
         let reg = registry(input.clone());
         // there is only a small chance that eny one

tests/testsuite/support/resolver.rs

@@ -230,6 +230,20 @@ pub fn pkg_loc(name: &str, loc: &str) -> Summary {
     .unwrap()
 }
 
+pub fn remove_dep(sum: &Summary, ind: usize) -> Summary {
+    let mut deps = sum.dependencies().to_vec();
+    deps.remove(ind);
+    // note: more things will need to be copied over in the future, but it works for now.
+    Summary::new(
+        sum.package_id(),
+        deps,
+        &BTreeMap::<String, Vec<String>>::new(),
+        sum.links().map(|a| a.as_str()),
+        sum.namespaced_features(),
+    )
+    .unwrap()
+}
+
 pub fn dep(name: &str) -> Dependency {
     dep_req(name, "*")
 }
@@ -400,67 +414,78 @@ pub fn registry_strategy(
 
     let list_of_raw_dependency = vec(raw_dependency, ..=max_deps);
 
-    (list_of_crates_with_versions, list_of_raw_dependency).prop_map(
-        |(crate_vers_by_name, raw_dependencies)| {
-            let list_of_pkgid: Vec<_> = crate_vers_by_name
-                .iter()
-                .flat_map(|(name, vers)| vers.iter().map(move |x| ((name.as_str(), &x.0), x.1)))
-                .collect();
-            let len_all_pkgid = list_of_pkgid.len();
-            let mut dependency_by_pkgid = vec![vec![]; len_all_pkgid];
-            for (a, b, (c, d), k) in raw_dependencies {
-                let (a, b) = order_index(a, b, len_all_pkgid);
-                let ((dep_name, _), _) = list_of_pkgid[a];
-                if (list_of_pkgid[b].0).0 == dep_name {
-                    continue;
-                }
-                let s = &crate_vers_by_name[dep_name];
-                let s_last_index = s.len() - 1;
-                let (c, d) = order_index(c, d, s.len());
-
-                dependency_by_pkgid[b].push(dep_req_kind(
-                    &dep_name,
-                    &if c == 0 && d == s_last_index {
-                        "*".to_string()
-                    } else if c == 0 {
-                        format!("<={}", s[d].0)
-                    } else if d == s_last_index {
-                        format!(">={}", s[c].0)
-                    } else if c == d {
-                        format!("={}", s[c].0)
-                    } else {
-                        format!(">={}, <={}", s[c].0, s[d].0)
-                    },
-                    match k {
-                        0 => Kind::Normal,
-                        1 => Kind::Build,
-                        // => Kind::Development, // Development has not impact so don't gen
-                        _ => panic!("bad index for Kind"),
-                    },
-                ))
-            }
+    // By default a package depends only on other packages that have a smaller name,
+    // this helps make sure that all things in the resulting index are DAGs.
+    // If this is true then the DAG is maintained with grater instead.
+    let reverse_alphabetical = any::<bool>().no_shrink();
 
-            PrettyPrintRegistry(
-                list_of_pkgid
-                    .into_iter()
-                    .zip(dependency_by_pkgid.into_iter())
-                    .map(|(((name, ver), allow_deps), deps)| {
-                        pkg_dep(
-                            (name, ver).to_pkgid(),
-                            if !allow_deps {
-                                vec![dep_req("bad", "*")]
-                            } else {
-                                let mut deps = deps;
-                                deps.sort_by_key(|d| d.name_in_toml());
-                                deps.dedup_by_key(|d| d.name_in_toml());
-                                deps
-                            },
-                        )
-                    })
-                    .collect(),
-            )
-        },
+    (
+        list_of_crates_with_versions,
+        list_of_raw_dependency,
+        reverse_alphabetical,
     )
+        .prop_map(
+            |(crate_vers_by_name, raw_dependencies, reverse_alphabetical)| {
+                let list_of_pkgid: Vec<_> = crate_vers_by_name
+                    .iter()
+                    .flat_map(|(name, vers)| vers.iter().map(move |x| ((name.as_str(), &x.0), x.1)))
+                    .collect();
+                let len_all_pkgid = list_of_pkgid.len();
+                let mut dependency_by_pkgid = vec![vec![]; len_all_pkgid];
+                for (a, b, (c, d), k) in raw_dependencies {
+                    let (a, b) = order_index(a, b, len_all_pkgid);
+                    let (a, b) = if reverse_alphabetical { (b, a) } else { (a, b) };
+                    let ((dep_name, _), _) = list_of_pkgid[a];
+                    if (list_of_pkgid[b].0).0 == dep_name {
+                        continue;
+                    }
+                    let s = &crate_vers_by_name[dep_name];
+                    let s_last_index = s.len() - 1;
+                    let (c, d) = order_index(c, d, s.len());
+
+                    dependency_by_pkgid[b].push(dep_req_kind(
+                        &dep_name,
+                        &if c == 0 && d == s_last_index {
+                            "*".to_string()
+                        } else if c == 0 {
+                            format!("<={}", s[d].0)
+                        } else if d == s_last_index {
+                            format!(">={}", s[c].0)
+                        } else if c == d {
+                            format!("={}", s[c].0)
+                        } else {
+                            format!(">={}, <={}", s[c].0, s[d].0)
+                        },
+                        match k {
+                            0 => Kind::Normal,
+                            1 => Kind::Build,
+                            // => Kind::Development, // Development has not impact so don't gen
+                            _ => panic!("bad index for Kind"),
+                        },
+                    ))
+                }
+
+                PrettyPrintRegistry(
+                    list_of_pkgid
+                        .into_iter()
+                        .zip(dependency_by_pkgid.into_iter())
+                        .map(|(((name, ver), allow_deps), deps)| {
+                            pkg_dep(
+                                (name, ver).to_pkgid(),
+                                if !allow_deps {
+                                    vec![dep_req("bad", "*")]
+                                } else {
+                                    let mut deps = deps;
+                                    deps.sort_by_key(|d| d.name_in_toml());
+                                    deps.dedup_by_key(|d| d.name_in_toml());
+                                    deps
+                                },
+                            )
+                        })
+                        .collect(),
+                )
+            },
+        )
 }
 
 /// This test is to test the generator to ensure