From da13975a4f0054d76cb8fac68bb968589f8d1b29 Mon Sep 17 00:00:00 2001 From: Yan Soares Couto Date: Fri, 1 Oct 2021 03:36:36 -0700 Subject: [PATCH] Fix quickcheck update breakages Summary: D31115820 (https://github.com/facebookexperimental/eden/commit/ae87b82eaff5ca10680398bf4d22cb1f0f81cb88) updated quickcheck, but there's some stuff we need to fix forward. This diff fixes the remaining failures I could find. Reviewed By: farnz Differential Revision: D31305392 fbshipit-source-id: a6684d47833bc0fd933751c13cdd71392cb1833b --- .../bookmarks/bookmarks_types/Cargo.toml | 1 - .../bookmarks/bookmarks_types/src/lib.rs | 2 +- .../mononoke/bookmarks/dbbookmarks/Cargo.toml | 1 - .../mononoke/bookmarks/dbbookmarks/src/lib.rs | 6 ++- .../bookmarks/dbbookmarks/tests/main.rs | 40 +++++++++++++++++-- eden/mononoke/filestore/src/chunk.rs | 4 +- .../filestore/src/test/test_invariants.rs | 6 +-- eden/mononoke/revset/src/quickchecks.rs | 6 +-- eden/scm/lib/checkout/Cargo.toml | 2 - eden/scm/lib/checkout/src/lib.rs | 7 +--- eden/scm/lib/radixbuf/src/key.rs | 2 +- 11 files changed, 52 insertions(+), 25 deletions(-) diff --git a/eden/mononoke/bookmarks/bookmarks_types/Cargo.toml b/eden/mononoke/bookmarks/bookmarks_types/Cargo.toml index 3e52d91738..165ad6c22f 100644 --- a/eden/mononoke/bookmarks/bookmarks_types/Cargo.toml +++ b/eden/mononoke/bookmarks/bookmarks_types/Cargo.toml @@ -12,7 +12,6 @@ anyhow = "1.0" ascii = "1.0" ascii_ext = { version = "0.1.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "master" } quickcheck = "1.0" -rand = { version = "0.8", features = ["small_rng"] } sql = { version = "0.1.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "master" } [patch.crates-io] diff --git a/eden/mononoke/bookmarks/bookmarks_types/src/lib.rs b/eden/mononoke/bookmarks/bookmarks_types/src/lib.rs index 177594051f..a4ed553e10 100644 --- a/eden/mononoke/bookmarks/bookmarks_types/src/lib.rs +++ b/eden/mononoke/bookmarks/bookmarks_types/src/lib.rs @@ -35,7 +35,7 @@ impl Arbitrary for Freshness { fn arbitrary(g: &mut Gen) -> Self { use Freshness::*; - match u32::arbitrary(g) & 2 { + match u32::arbitrary(g) % 2 { 0 => MostRecent, 1 => MaybeStale, _ => unreachable!(), diff --git a/eden/mononoke/bookmarks/dbbookmarks/Cargo.toml b/eden/mononoke/bookmarks/dbbookmarks/Cargo.toml index 71aad6dcdc..1c1378168f 100644 --- a/eden/mononoke/bookmarks/dbbookmarks/Cargo.toml +++ b/eden/mononoke/bookmarks/dbbookmarks/Cargo.toml @@ -38,7 +38,6 @@ futures-old = { package = "futures", version = "0.1.31" } maplit = "1.0" mononoke_types-mocks = { version = "0.1.0", path = "../../mononoke_types/mocks" } quickcheck = "1.0" -quickcheck_derive = "0.3" tokio = { version = "1.10", features = ["full", "test-util", "tracing"] } [patch.crates-io] diff --git a/eden/mononoke/bookmarks/dbbookmarks/src/lib.rs b/eden/mononoke/bookmarks/dbbookmarks/src/lib.rs index 67c923a8e2..7c1610f562 100644 --- a/eden/mononoke/bookmarks/dbbookmarks/src/lib.rs +++ b/eden/mononoke/bookmarks/dbbookmarks/src/lib.rs @@ -223,7 +223,7 @@ mod test { Some(name) => BookmarkPagination::After(name), None => BookmarkPagination::FromStart, }; - let have = insert_then_query( + let mut have = insert_then_query( fb, &bookmarks, freshness, @@ -232,13 +232,15 @@ mod test { &pagination, limit, ); - let want = mock_bookmarks_response( + let mut want = mock_bookmarks_response( &bookmarks, &prefix, kinds.as_slice(), &pagination, limit, ); + have.sort_by_key(|(_, csid)| *csid); + want.sort_by_key(|(_, csid)| *csid); have == want } } diff --git a/eden/mononoke/bookmarks/dbbookmarks/tests/main.rs b/eden/mononoke/bookmarks/dbbookmarks/tests/main.rs index 72cb714851..bed0f4c5f5 100644 --- a/eden/mononoke/bookmarks/dbbookmarks/tests/main.rs +++ b/eden/mononoke/bookmarks/dbbookmarks/tests/main.rs @@ -24,7 +24,7 @@ use mononoke_types_mocks::changesetid::{ FIVES_CSID, FOURS_CSID, ONES_CSID, SIXES_CSID, THREES_CSID, TWOS_CSID, }; use mononoke_types_mocks::repo::{REPO_ONE, REPO_TWO, REPO_ZERO}; -use quickcheck_derive::Arbitrary; +use quickcheck::{Arbitrary, Gen}; use sql::mysql_async::{prelude::ConvIr, Value}; use sql_construct::SqlConstruct; use std::collections::HashMap; @@ -1433,13 +1433,23 @@ async fn bookmark_subscription_updates(fb: FacebookInit) -> Result<()> { Ok(()) } -#[derive(Arbitrary, Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug)] enum TestBookmark { Book1, Book2, } -#[derive(Arbitrary, Clone, Copy, Debug)] +impl Arbitrary for TestBookmark { + fn arbitrary(g: &mut Gen) -> Self { + if bool::arbitrary(g) { + Self::Book1 + } else { + Self::Book2 + } + } +} + +#[derive(Clone, Copy, Debug)] enum BookmarkOp { /// Set this bookmark. Set(ChangesetId), @@ -1449,8 +1459,19 @@ enum BookmarkOp { Delete, } +impl Arbitrary for BookmarkOp { + fn arbitrary(g: &mut Gen) -> Self { + match u32::arbitrary(g) % 3 { + 0 => Self::Set(Arbitrary::arbitrary(g)), + 1 => Self::ForceSet(Arbitrary::arbitrary(g)), + 2 => Self::Delete, + _ => unreachable!(), + } + } +} + /// Use Quickcheck to produce a test scenario of bookmark updates. -#[derive(Arbitrary, Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug)] enum TestOp { /// Update one of our test bookmarks Bookmark(TestBookmark, BookmarkOp), @@ -1460,6 +1481,17 @@ enum TestOp { Refresh, } +impl Arbitrary for TestOp { + fn arbitrary(g: &mut Gen) -> Self { + match u32::arbitrary(g) % 3 { + 0 => Self::Bookmark(Arbitrary::arbitrary(g), Arbitrary::arbitrary(g)), + 1 => Self::Noop, + 2 => Self::Refresh, + _ => unreachable!(), + } + } +} + /// Verify bookmark subscriptions using Quickcheck. We create a test scenario and verify that the /// bookmark subscriptions returns the same data it would return if it was freshly created now (we /// test that this satisfies our assumptions in separate tests for the bookmarks subscription). diff --git a/eden/mononoke/filestore/src/chunk.rs b/eden/mononoke/filestore/src/chunk.rs index 888b96a191..1605c6a3a5 100644 --- a/eden/mononoke/filestore/src/chunk.rs +++ b/eden/mononoke/filestore/src/chunk.rs @@ -431,8 +431,8 @@ mod test { } quickcheck! { - fn check_chunk_stream(in_chunks: Vec>, size: usize) -> bool { - let size = size + 1; // Don't allow 0 as the size. + fn check_chunk_stream(in_chunks: Vec>, size: u8) -> bool { + let size = (size as usize) + 1; // Don't allow 0 as the size. let rt = Runtime::new().unwrap(); rt.block_on(do_check_chunk_stream(in_chunks, size)) } diff --git a/eden/mononoke/filestore/src/test/test_invariants.rs b/eden/mononoke/filestore/src/test/test_invariants.rs index c40ec74e0d..a460a0bb33 100644 --- a/eden/mononoke/filestore/src/test/test_invariants.rs +++ b/eden/mononoke/filestore/src/test/test_invariants.rs @@ -15,7 +15,7 @@ use futures::{ future::{self, TryFutureExt}, stream, }; -use quickcheck::{Arbitrary, StdGen}; +use quickcheck::{Arbitrary, Gen}; use std::collections::HashSet; use std::sync::Arc; @@ -87,7 +87,7 @@ fn test_invariants(fb: FacebookInit) -> Result<()> { // right for a store() call to succeed (all the chunks need to be saved, then we need to write // 3 aliases, and then the content). let rt = tokio::runtime::Runtime::new()?; - let mut gen = StdGen::new(rand::thread_rng(), 128); + let mut gen = Gen::new(128); let memblob = Arc::new(memblob::Memblob::default()); let blob = FailingBlobstore::new(memblob.clone(), 0.75, 0.75); @@ -127,7 +127,7 @@ fn test_invariants(fb: FacebookInit) -> Result<()> { #[fbinit::test] async fn test_store_bytes_consistency(fb: FacebookInit) -> Result<(), Error> { - let mut gen = StdGen::new(rand::thread_rng(), 128); + let mut gen = Gen::new(128); let memblob = Arc::new(memblob::Memblob::default()); let ctx = CoreContext::test_mock(fb); diff --git a/eden/mononoke/revset/src/quickchecks.rs b/eden/mononoke/revset/src/quickchecks.rs index 14b31f7eec..980e08c9bf 100644 --- a/eden/mononoke/revset/src/quickchecks.rs +++ b/eden/mononoke/revset/src/quickchecks.rs @@ -203,10 +203,10 @@ mod test { // Can't add a set operator if we have don't have at least one node revset.push(RevsetEntry::SingleNode(None)); } else { - let input_count = g.gen_range(0..revspecs_in_set) + 1; + let input_count = (usize::arbitrary(g) % revspecs_in_set) + 1; revset.push( // Bias towards SingleNode if we only have 1 rev - match g.gen_range(0..4) { + match u32::arbitrary(g) % 4 { 0 => RevsetEntry::SingleNode(None), 1 => { if revspecs_in_set >= 2 { @@ -233,7 +233,7 @@ mod test { assert!(revspecs_in_set > 0, "Did not produce enough revs"); if revspecs_in_set > 1 { - revset.push(match g.gen_range(0..2) { + revset.push(match u32::arbitrary(g) % 2 { 0 => RevsetEntry::Intersect(revspecs_in_set), 1 => RevsetEntry::Union(revspecs_in_set), _ => panic!("Range returned too wide a variation"), diff --git a/eden/scm/lib/checkout/Cargo.toml b/eden/scm/lib/checkout/Cargo.toml index 6ef79b4246..58d686237c 100644 --- a/eden/scm/lib/checkout/Cargo.toml +++ b/eden/scm/lib/checkout/Cargo.toml @@ -24,7 +24,5 @@ vfs = { path = "../vfs" } [dev-dependencies] manifest-tree = { path = "../manifest-tree", features = ["for-tests"] } quickcheck = "1.0" -rand = { version = "0.8", features = ["small_rng"] } -rand_chacha = "0.3" tempfile = "3.1" walkdir = "2.2.9" diff --git a/eden/scm/lib/checkout/src/lib.rs b/eden/scm/lib/checkout/src/lib.rs index 023495cbc6..8ff36f6d38 100644 --- a/eden/scm/lib/checkout/src/lib.rs +++ b/eden/scm/lib/checkout/src/lib.rs @@ -813,9 +813,7 @@ mod test { use manifest_tree::testutil::{make_tree_manifest_from_meta, TestStore}; use manifest_tree::Diff; use pathmatcher::AlwaysMatcher; - use quickcheck::{Arbitrary, StdGen}; - use rand::SeedableRng; - use rand_chacha::ChaChaRng; + use quickcheck::{Arbitrary, Gen}; use std::collections::HashMap; use std::fs::create_dir; use std::path::Path; @@ -889,8 +887,7 @@ mod test { fn generate_trees(tree_size: usize, count: usize) -> Vec> { let mut result = vec![]; - let rng = ChaChaRng::from_seed([0u8; 32]); - let mut gen = StdGen::new(rng, 5); + let mut gen = Gen::new(5); let paths = generate_repo_paths(tree_size * count, &mut gen); for i in 0..count { diff --git a/eden/scm/lib/radixbuf/src/key.rs b/eden/scm/lib/radixbuf/src/key.rs index 27eaaf8b03..631181a058 100644 --- a/eden/scm/lib/radixbuf/src/key.rs +++ b/eden/scm/lib/radixbuf/src/key.rs @@ -139,7 +139,7 @@ mod tests { let mut buf = Vec::::new(); let keys: Vec> = (0..1000usize) .map(|i| { - let mut rng = thread_rng(); + let rng = thread_rng(); rng.sample_iter(&Alphanumeric).take(i % 40).collect() }) .collect();