-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use proptest to fuzz the resolver #5921
Merged
Merged
Changes from 9 commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
56a222c
a start on using proptest to fuzz the resolver
Eh2406 1761886
cache the example url to solve performance problem
Eh2406 ce1772c
get working with minimal-versions
Eh2406 4e619d8
better generation of version numbers
Eh2406 732aa10
small clean up
Eh2406 c26553a
small clean up
Eh2406 b7285e8
handle "bad" slightly better
Eh2406 5339e92
small clean up for the cache of the example url
Eh2406 f270dda
incorporate @AltSysrq suggestions
Eh2406 17fe190
update to the new version of proptest
Eh2406 85b1976
use the new `impl Strategy for Vec<S>` to only generate valid depende…
Eh2406 0ef43cb
use the new `result_cache`
Eh2406 0206cec
double down on `prop_flat_map` to guarantee version requirements are …
Eh2406 1f98871
stronger assert in the core
Eh2406 b0bbb6a
use args for scale
Eh2406 2628ec0
works a LOT better if the simple cases are at the beginning
Eh2406 509806e
small clean up
Eh2406 d6258e9
Merge remote-tracking branch 'origin/master' into proptest
Eh2406 1eaf543
small clean up
Eh2406 2b1c39f
more controllable bad dependencies, and use Index for ranges
Eh2406 05c6ce0
remove a `prop_flat_map` use @AltSysrq suggestion again
Eh2406 856964f
fail fast on CI, don't shrink the input
Eh2406 2b16156
If resolution was successful, then unpublishing a version of a crate …
Eh2406 c497851
If resolution was unsuccessful, then it should stay unsuccessful even…
Eh2406 1b15b15
give intermediate steps names
Eh2406 d8e19fb
pull the has bad dep before `prop_flat_map`
Eh2406 2015191
`edge list` is a much cleaner approach
Eh2406 7ad9f5e
add a test to test the tester
Eh2406 6bd2540
check each registry more thoroughly
Eh2406 40d9de4
Merge remote-tracking branch 'origin/master' into proptest
Eh2406 3e7192e
pretty print the registry made by proptest
Eh2406 6763ede
proptest 0.8.7 has a better `ci_no_shrink`
Eh2406 a4da525
In theory shrinkage should be 2, but in practice we get better trees …
Eh2406 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
use std::collections::{BTreeMap, HashSet}; | ||
use std::cmp::{max, min}; | ||
|
||
use support::hamcrest::{assert_that, contains, is_not}; | ||
|
||
|
@@ -12,6 +13,11 @@ use support::ChannelChanger; | |
use support::{execs, project}; | ||
use support::registry::Package; | ||
|
||
use proptest::collection::{btree_map, btree_set, vec}; | ||
use proptest::prelude::*; | ||
use proptest::sample::subsequence; | ||
use proptest::string::string_regex; | ||
|
||
fn resolve( | ||
pkg: &PackageId, | ||
deps: Vec<Dependency>, | ||
|
@@ -58,9 +64,7 @@ trait ToDep { | |
|
||
impl ToDep for &'static str { | ||
fn to_dep(self) -> Dependency { | ||
let url = "http://example.com".to_url().unwrap(); | ||
let source_id = SourceId::for_registry(&url).unwrap(); | ||
Dependency::parse_no_deprecated(self, Some("1.0.0"), &source_id).unwrap() | ||
Dependency::parse_no_deprecated(self, Some("1.0.0"), ®istry_loc()).unwrap() | ||
} | ||
} | ||
|
||
|
@@ -74,54 +78,55 @@ trait ToPkgId { | |
fn to_pkgid(&self) -> PackageId; | ||
} | ||
|
||
impl<'a> ToPkgId for &'a str { | ||
impl<'a> ToPkgId for PackageId { | ||
fn to_pkgid(&self) -> PackageId { | ||
PackageId::new(*self, "1.0.0", ®istry_loc()).unwrap() | ||
self.clone() | ||
} | ||
} | ||
|
||
impl<'a> ToPkgId for (&'a str, &'a str) { | ||
impl<'a> ToPkgId for &'a str { | ||
fn to_pkgid(&self) -> PackageId { | ||
let (name, vers) = *self; | ||
PackageId::new(name, vers, ®istry_loc()).unwrap() | ||
PackageId::new(*self, "1.0.0", ®istry_loc()).unwrap() | ||
} | ||
} | ||
|
||
impl<'a> ToPkgId for (&'a str, String) { | ||
impl<T: AsRef<str>, U: AsRef<str>> ToPkgId for (T, U) { | ||
fn to_pkgid(&self) -> PackageId { | ||
let (name, ref vers) = *self; | ||
PackageId::new(name, vers, ®istry_loc()).unwrap() | ||
let (name, vers) = self; | ||
PackageId::new(name.as_ref(), vers.as_ref(), ®istry_loc()).unwrap() | ||
} | ||
} | ||
|
||
macro_rules! pkg { | ||
($pkgid:expr => [$($deps:expr),+]) => ({ | ||
let d: Vec<Dependency> = vec![$($deps.to_dep()),+]; | ||
let pkgid = $pkgid.to_pkgid(); | ||
let link = if pkgid.name().ends_with("-sys") {Some(pkgid.name().as_str())} else {None}; | ||
|
||
Summary::new(pkgid, d, &BTreeMap::<String, Vec<String>>::new(), link, false).unwrap() | ||
pkg_dep($pkgid, d) | ||
}); | ||
|
||
($pkgid:expr) => ({ | ||
let pkgid = $pkgid.to_pkgid(); | ||
let link = if pkgid.name().ends_with("-sys") {Some(pkgid.name().as_str())} else {None}; | ||
Summary::new(pkgid, Vec::new(), &BTreeMap::<String, Vec<String>>::new(), link, false).unwrap() | ||
pkg($pkgid) | ||
}) | ||
} | ||
|
||
fn registry_loc() -> SourceId { | ||
let remote = "http://example.com".to_url().unwrap(); | ||
SourceId::for_registry(&remote).unwrap() | ||
lazy_static! { | ||
static ref EXAMPLE_DOT_COM: SourceId = SourceId::for_registry(&"http://example.com".to_url().unwrap()).unwrap(); | ||
} | ||
EXAMPLE_DOT_COM.clone() | ||
} | ||
|
||
fn pkg(name: &str) -> Summary { | ||
let link = if name.ends_with("-sys") { | ||
Some(name) | ||
fn pkg<T: ToPkgId>(name: T) -> Summary { | ||
pkg_dep(name, Vec::new()) | ||
} | ||
|
||
fn pkg_dep<T: ToPkgId>(name: T, dep: Vec<Dependency>) -> Summary { | ||
let pkgid = name.to_pkgid(); | ||
let link = if pkgid.name().ends_with("-sys") { | ||
Some(pkgid.name().as_str()) | ||
} else { | ||
None | ||
}; | ||
Summary::new(pkg_id(name), Vec::new(), &BTreeMap::<String, Vec<String>>::new(), link, false).unwrap() | ||
Summary::new(name.to_pkgid(), dep, &BTreeMap::<String, Vec<String>>::new(), link, false).unwrap() | ||
} | ||
|
||
fn pkg_id(name: &str) -> PackageId { | ||
|
@@ -155,9 +160,7 @@ fn dep(name: &str) -> Dependency { | |
dep_req(name, "1.0.0") | ||
} | ||
fn dep_req(name: &str, req: &str) -> Dependency { | ||
let url = "http://example.com".to_url().unwrap(); | ||
let source_id = SourceId::for_registry(&url).unwrap(); | ||
Dependency::parse_no_deprecated(name, Some(req), &source_id).unwrap() | ||
Dependency::parse_no_deprecated(name, Some(req), ®istry_loc()).unwrap() | ||
} | ||
|
||
fn dep_loc(name: &str, location: &str) -> Dependency { | ||
|
@@ -185,6 +188,90 @@ fn loc_names(names: &[(&'static str, &'static str)]) -> Vec<PackageId> { | |
.collect() | ||
} | ||
|
||
/// This generates a random registry index. | ||
/// Unlike vec((Name, Ver, vec((Name, VerRq), ..), ..) | ||
/// This strategy has a high probability of having valid dependencies | ||
fn registry_strategy() -> impl Strategy<Value=Vec<Summary>> { | ||
const MAX_CRATES: usize = 10; | ||
const MAX_VERSIONS: usize = 10; | ||
|
||
let valid_name_strategy = string_regex("[A-Za-z_-][A-Za-z0-9_-]*").unwrap(); | ||
|
||
fn range(m: usize) -> impl Strategy<Value = (usize, usize)> { | ||
(..m, ..m).prop_map(|(a, b)| (min(a, b), max(a,b))) | ||
} | ||
|
||
fn ver(max: usize) -> impl Strategy<Value=(usize, usize, usize)> { | ||
(..=max, ..=max, ..=max) | ||
} | ||
|
||
btree_map( | ||
valid_name_strategy, | ||
btree_set( | ||
ver(MAX_VERSIONS), | ||
1..=MAX_VERSIONS, | ||
), | ||
1..=MAX_CRATES, | ||
).prop_flat_map(|mut b| { | ||
// root is the name of the thing being compiled | ||
// so it would be confusing to have it in the index | ||
b.remove("root"); | ||
let names = Some("bad".to_owned()) // if randomly pointing to a dependency that does not exist then call it `bad` | ||
.iter() | ||
.chain(b.iter().map(|(name, _)| name)) | ||
.map(|name| pkg_id(name).name()) | ||
.collect::<Vec<_>>(); | ||
let data = b.iter() | ||
.flat_map(|(name, vers)| vers.iter().map(move |(a, b, c)| (name, format!("{}.{}.{}", a, b, c)).to_pkgid())) | ||
.collect::<Vec<_>>(); | ||
let names_len = names.len(); | ||
let data_len = data.len(); | ||
( | ||
Just(data), | ||
vec(subsequence(names, ..names_len), data_len), | ||
vec( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you could lift this out to a top-level strategy by just generating a vector of size |
||
range(MAX_VERSIONS).prop_map(|(b, e)| format!(">={}.0.0, <={}.0.0", b, e)), | ||
names_len*data_len, | ||
), | ||
) | ||
}).prop_map(|(data, deps, vers)| { | ||
let mut i = 0; | ||
data.into_iter() | ||
.zip(deps.into_iter()) | ||
.map(|(pkgid, deps)| { | ||
let d: Vec<Dependency> = deps.into_iter() | ||
.filter(|n| pkgid.name() < *n || n.as_str() == "bad") | ||
.map(|n| { | ||
i += 1; | ||
dep_req(&n, &vers[i - 1]) | ||
}) | ||
.collect(); | ||
pkg_dep(pkgid, d) | ||
}) | ||
.collect() | ||
}) | ||
} | ||
|
||
proptest! { | ||
#[test] | ||
fn doesnt_crash(input in registry_strategy()) { | ||
let reg = registry(input.clone()); | ||
// there is only a small chance that eny one | ||
// crate will be interesting. So we try them all. | ||
for this in input.iter().rev() { | ||
let res = resolve( | ||
&pkg_id("root"), | ||
vec![dep_req(&this.name(), &format!("={}", this.version()))], | ||
®, | ||
); | ||
|
||
if let Ok(r) = res { | ||
prop_assert!(r.len() <= 4) | ||
} | ||
} | ||
} | ||
} | ||
|
||
#[test] | ||
#[should_panic(expected = "assertion failed: !name.is_empty()")] | ||
fn test_dependency_with_empty_name() { | ||
|
@@ -208,14 +295,14 @@ fn assert_same(a: &[PackageId], b: &[PackageId]) { | |
|
||
#[test] | ||
fn test_resolving_only_package() { | ||
let reg = registry(vec![pkg("foo")]); | ||
let reg = registry(vec![pkg!("foo")]); | ||
let res = resolve(&pkg_id("root"), vec![dep("foo")], ®).unwrap(); | ||
assert_same(&res, &names(&["root", "foo"])); | ||
} | ||
|
||
#[test] | ||
fn test_resolving_one_dep() { | ||
let reg = registry(vec![pkg("foo"), pkg("bar")]); | ||
let reg = registry(vec![pkg!("foo"), pkg!("bar")]); | ||
let res = resolve(&pkg_id("root"), vec![dep("foo")], ®).unwrap(); | ||
assert_same(&res, &names(&["root", "foo"])); | ||
} | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this particular issue should be fixed in proptest 0.8.6