Skip to content

Commit

Permalink
Add a quickcheck property to better test the positive case
Browse files Browse the repository at this point in the history
If you just generate two random strings, the odds are very high
that the shorter one won't be a substring of the longer one once
they reach any substantial length. This means that the existing
quickcheck cases were probably just testing the negative cases.
The exception would be the two cases that append the needle
to the haystack, but those only test behavior at the ends. This
patch adds a better quickcheck case that can test a needle anywhere
in the haystack.

See the comments on rust-lang#446
  • Loading branch information
Ethan Pailes committed Mar 7, 2018
1 parent 7f020b8 commit 24ffdb4
Showing 1 changed file with 42 additions and 4 deletions.
46 changes: 42 additions & 4 deletions src/literals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,7 @@ mod tests {
let haystack = vec![91];
let needle = vec![91];

let naive_offset = naive_find(needle, haystack.as_slice()).unwrap();
let naive_offset = naive_find(&needle, &haystack).unwrap();
assert_eq!(0, naive_offset);
}

Expand Down Expand Up @@ -981,12 +981,12 @@ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

use quickcheck::TestResult;

fn naive_find(needle: Vec<u8>, haystack: &[u8]) -> Option<usize> {
fn naive_find(needle: &[u8], haystack: &[u8]) -> Option<usize> {
assert!(needle.len() <= haystack.len());

for i in 0..(haystack.len() - (needle.len() - 1)) {
if haystack[i] == needle[0]
&& &haystack[i..(i+needle.len())] == needle.as_slice() {
&& &haystack[i..(i+needle.len())] == needle {
return Some(i)
}
}
Expand All @@ -1008,7 +1008,7 @@ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

let searcher = BoyerMooreSearch::new(needle.clone());
TestResult::from_bool(
searcher.find(haystack) == naive_find(needle, haystack))
searcher.find(haystack) == naive_find(&needle, haystack))
}

fn qc_bm_equals_single(pile1: Vec<u8>, pile2: Vec<u8>) -> TestResult {
Expand Down Expand Up @@ -1063,6 +1063,44 @@ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
.unwrap_or(false))
}

// qc_equals_* is only testing the negative case as @burntsushi
// pointed out in https://github.com/rust-lang/regex/issues/446.
// This quickcheck prop represents an effort to force testing of
// the positive case. qc_bm_finds_first and qc_bm_finds_trailing_needle
// already check some of the positive cases, but they don't cover
// cases where the needle is in the middle of haystack. This prop
// fills that hole.
fn qc_bm_finds_subslice(
haystack: Vec<u8>,
needle_start: usize,
needle_length: usize
) -> TestResult {
if haystack.len() == 0 {
return TestResult::discard();
}

let needle_start = needle_start % haystack.len();
let needle_length = needle_length % (haystack.len() - needle_start);

if needle_length == 0 {
return TestResult::discard();
}

let needle = &haystack[needle_start..(needle_start + needle_length)];

let bm_searcher = BoyerMooreSearch::new(needle.to_vec());

let start = naive_find(&needle, &haystack);
match start {
None => TestResult::from_bool(false),
Some(nf_start) =>
TestResult::from_bool(
nf_start <= needle_start
&& bm_searcher.find(&haystack) == start
)
}
}

fn qc_bm_finds_first(needle: Vec<u8>) -> TestResult {
if needle.len() == 0 {
return TestResult::discard();
Expand Down

0 comments on commit 24ffdb4

Please sign in to comment.