diff --git a/src/uu/factor/BENCHMARKING.md b/src/uu/factor/BENCHMARKING.md index 6bf9cbf90..0f9afaff0 100644 --- a/src/uu/factor/BENCHMARKING.md +++ b/src/uu/factor/BENCHMARKING.md @@ -44,7 +44,8 @@ on Daniel Lemire's [*Microbenchmarking calls for idealized conditions*][lemire], which I recommend reading if you want to add benchmarks to `factor`. 1. Select a small, self-contained, deterministic component - `gcd` and `table::factor` are good example of such: + (`gcd` and `table::factor` are good examples): + - no I/O or access to external data structures ; - no call into other components ; - behavior is deterministic: no RNG, no concurrency, ... ; @@ -53,16 +54,19 @@ which I recommend reading if you want to add benchmarks to `factor`. maximizing the numbers of samples we can take in a given time. 2. Benchmarks are immutable (once merged in `uutils`) + Modifying a benchmark means previously-collected values cannot meaningfully be compared, silently giving nonsensical results. If you must modify an existing benchmark, rename it. 3. Test common cases + We are interested in overall performance, rather than specific edge-cases; use **reproducibly-randomized inputs**, sampling from either all possible input values or some subset of interest. 4. Use [`criterion`], `criterion::black_box`, ... + `criterion` isn't perfect, but it is also much better than ad-hoc solutions in each benchmark. diff --git a/src/uu/factor/src/numeric/gcd.rs b/src/uu/factor/src/numeric/gcd.rs index 004ef1515..78197c722 100644 --- a/src/uu/factor/src/numeric/gcd.rs +++ b/src/uu/factor/src/numeric/gcd.rs @@ -52,7 +52,7 @@ pub fn gcd(mut u: u64, mut v: u64) -> u64 { #[cfg(test)] mod tests { use super::*; - use quickcheck::quickcheck; + use quickcheck::{quickcheck, TestResult}; quickcheck! { fn euclidean(a: u64, b: u64) -> bool { @@ -76,13 +76,12 @@ mod tests { gcd(0, a) == a } - fn divisor(a: u64, b: u64) -> () { + fn divisor(a: u64, b: u64) -> TestResult { // Test that gcd(a, b) divides a and b, unless a == b == 0 - if a == 0 && b == 0 { return; } + if a == 0 && b == 0 { return TestResult::discard(); } // restrict test domain to !(a == b == 0) let g = gcd(a, b); - assert_eq!(a % g, 0); - assert_eq!(b % g, 0); + TestResult::from_bool( g != 0 && a % g == 0 && b % g == 0 ) } fn commutative(a: u64, b: u64) -> bool {