Auto merge of #6274 - Eh2406:small_things, r=alexcrichton

Small things to help with fuzz tests

These are some small fixes to make it less likely to have randomly failing tests. This also documents what to do, and not to feel guilty, if the test do fail.

Author: bors

src/cargo/core/resolver/conflict_cache.rs

@@ -35,9 +35,8 @@ pub(super) struct ConflictCache {
     // unconditionally true regardless of our resolution history of how we got
     // here.
     con_from_dep: HashMap<Dependency, Vec<HashMap<PackageId, ConflictReason>>>,
-    // `past_conflict_triggers` is an
-    // of `past_conflicting_activations`.
-    // For every `PackageId` this lists the `Dependency`s that mention it in `past_conflicting_activations`.
+    // `dep_from_pid` is an inverse-index of `con_from_dep`.
+    // For every `PackageId` this lists the `Dependency`s that mention it in `dep_from_pid`.
     dep_from_pid: HashMap<PackageId, HashSet<Dependency>>,
 }
 
@@ -62,6 +61,7 @@ impl ConflictCache {
         self.con_from_dep
             .get(dep)?
             .iter()
+            .rev() // more general cases are normally found letter. So start the search there.
             .filter(filter)
             .find(|conflicting| cx.is_conflicting(None, conflicting))
     }

tests/testsuite/resolve.rs

@@ -15,6 +15,13 @@ use support::resolver::{
 use proptest::collection::vec;
 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,
+/// but if it fails then there really is something wrong. When testing something as
+/// complicated as the resolver, the problems can be very subtle and hard to generate.
+/// We have had a history of these tests only failing on PRs long after a bug is introduced.
+/// If you have one of these test fail please report it on #6258,
+/// 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,
@@ -34,6 +41,7 @@ proptest! {
         .. ProptestConfig::default()
     })]
 
+    /// NOTE: if you think this test has failed spuriously see the note at the top of this macro.
     #[test]
     fn passes_validation(
         PrettyPrintRegistry(input) in registry_strategy(50, 20, 60)
@@ -51,6 +59,7 @@ proptest! {
         }
     }
 
+    /// NOTE: if you think this test has failed spuriously see the note at the top of this macro.
     #[test]
     fn minimum_version_errors_the_same(
             PrettyPrintRegistry(input) in registry_strategy(50, 20, 60)
@@ -100,6 +109,7 @@ proptest! {
         }
     }
 
+    /// 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),

tests/testsuite/support/resolver.rs

@@ -355,7 +355,7 @@ pub fn registry_strategy(
     max_versions: usize,
     shrinkage: usize,
 ) -> impl Strategy<Value = PrettyPrintRegistry> {
-    let name = string_regex("[A-Za-z_-][A-Za-z0-9_-]*(-sys)?").unwrap();
+    let name = string_regex("[A-Za-z][A-Za-z0-9_-]*(-sys)?").unwrap();
 
     let raw_version = [..max_versions; 3];
     let version_from_raw = |v: &[usize; 3]| format!("{}.{}.{}", v[0], v[1], v[2]);
@@ -460,12 +460,16 @@ pub fn registry_strategy(
 
 /// This test is to test the generator to ensure
 /// that it makes registries with large dependency trees
+///
+/// This is a form of randomized testing, if you are unlucky it can fail.
+/// A failure on it's own is not a big dael. If you did not change the
+/// `registry_strategy` then feel free to retry without concern.
 #[test]
 fn meta_test_deep_trees_from_strategy() {
     let mut dis = [0; 21];
 
     let strategy = registry_strategy(50, 20, 60);
-    for _ in 0..64 {
+    for _ in 0..128 {
         let PrettyPrintRegistry(input) = strategy
             .new_tree(&mut TestRunner::default())
             .unwrap()
@@ -488,19 +492,23 @@ fn meta_test_deep_trees_from_strategy() {
     }
 
     panic!(
-        "In 640 tries we did not see a wide enough distribution of dependency trees! dis: {:?}",
+        "In 1280 tries we did not see a wide enough distribution of dependency trees! dis: {:?}",
         dis
     );
 }
 
 /// This test is to test the generator to ensure
 /// that it makes registries that include multiple versions of the same library
+///
+/// This is a form of randomized testing, if you are unlucky it can fail.
+/// A failure on its own is not a big deal. If you did not change the
+/// `registry_strategy` then feel free to retry without concern.
 #[test]
 fn meta_test_multiple_versions_strategy() {
     let mut dis = [0; 10];
 
     let strategy = registry_strategy(50, 20, 60);
-    for _ in 0..64 {
+    for _ in 0..128 {
         let PrettyPrintRegistry(input) = strategy
             .new_tree(&mut TestRunner::default())
             .unwrap()
@@ -524,7 +532,7 @@ fn meta_test_multiple_versions_strategy() {
         }
     }
     panic!(
-        "In 640 tries we did not see a wide enough distribution of multiple versions of the same library! dis: {:?}",
+        "In 1280 tries we did not see a wide enough distribution of multiple versions of the same library! dis: {:?}",
         dis
     );
 }