From d08fd375db0014f097ee3fbdc1c595b632669f6a Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Tue, 6 Nov 2018 19:50:19 -0500 Subject: [PATCH 1/5] the more recently added conflicts are more likely to still be relevant This fixes most of the cases fount in #6258, in practice if not in theory. --- src/cargo/core/resolver/conflict_cache.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cargo/core/resolver/conflict_cache.rs b/src/cargo/core/resolver/conflict_cache.rs index b35813bc35a..77f131a2581 100644 --- a/src/cargo/core/resolver/conflict_cache.rs +++ b/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>>, - // `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>, } @@ -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)) } From 90a54dd9ca269a63ca474a1ead5e729998fcce3b Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Tue, 6 Nov 2018 20:18:55 -0500 Subject: [PATCH 2/5] document that fuss tests are flaky, and how to report. --- tests/testsuite/resolve.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs index 9ce7ccc4e42..392c6929c0e 100644 --- a/tests/testsuite/resolve.rs +++ b/tests/testsuite/resolve.rs @@ -16,6 +16,13 @@ use proptest::collection::vec; use proptest::prelude::*; proptest! { + /// 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_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 @@ -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), From 221908d2aebfaa0b48c890f79721bd31f14ab99d Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Tue, 6 Nov 2018 20:40:18 -0500 Subject: [PATCH 3/5] document that meta_fuss tests are flaky, and to ignore. --- tests/testsuite/support/resolver.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/testsuite/support/resolver.rs b/tests/testsuite/support/resolver.rs index ae89eaf71f9..4c7ad3288ae 100644 --- a/tests/testsuite/support/resolver.rs +++ b/tests/testsuite/support/resolver.rs @@ -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 ); } From 8cfa1dd3db490d9f277cf27aee89844128e6ba6c Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Tue, 6 Nov 2018 20:47:06 -0500 Subject: [PATCH 4/5] remove the hard to read start of names --- tests/testsuite/support/resolver.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testsuite/support/resolver.rs b/tests/testsuite/support/resolver.rs index 4c7ad3288ae..fd2a8d6f0ce 100644 --- a/tests/testsuite/support/resolver.rs +++ b/tests/testsuite/support/resolver.rs @@ -355,7 +355,7 @@ pub fn registry_strategy( max_versions: usize, shrinkage: usize, ) -> impl Strategy { - 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]); From 7ca46cccbd8ccc1d2d9b31d6e3a9d87d121c4828 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Wed, 7 Nov 2018 10:41:37 -0500 Subject: [PATCH 5/5] move note to let it build --- tests/testsuite/resolve.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs index 392c6929c0e..99a3ccf0e6e 100644 --- a/tests/testsuite/resolve.rs +++ b/tests/testsuite/resolve.rs @@ -15,14 +15,14 @@ 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! { - /// 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_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