Skip to content

Commit

Permalink
Make test 'iter_panic_fuse' deterministic
Browse files Browse the repository at this point in the history
Fixes rayon-rs#667

'panic_fuse' will only stop other threads 'as soon as possible' -
if the other threads are fast enough, they might end up processing
the entire rest of the iterator.

This commit changes the test 'iter_panic_fuse' to properly take this
into account, by creating a custom threadpool with only 1 thread.
This makes the test deterministic - with only one thread, the panic
is guaranmteed to be observed when the next item is processed, causing
the desired early exit.

I've also added the 'more-asserts' crate as a dev dependency, so
that we can print out a more informative error message in
'iter_panic_fuse'
  • Loading branch information
Aaron1011 committed Jul 7, 2019
1 parent b8b97a1 commit 9532eb5
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 21 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,4 @@ rand_xorshift = "0.1"
serde = "1"
serde_derive = "1"
doc-comment = "0.3"
more-asserts = "0.2.1"
48 changes: 27 additions & 21 deletions tests/iter_panic.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
extern crate rayon;
#[macro_use]
extern crate more_asserts;

use rayon::prelude::*;
use std::ops::Range;
Expand All @@ -22,26 +24,30 @@ fn iter_panic() {

#[test]
fn iter_panic_fuse() {
fn count(iter: impl ParallelIterator + UnwindSafe) -> usize {
let count = AtomicUsize::new(0);
let result = panic::catch_unwind(|| {
iter.for_each(|_| {
count.fetch_add(1, Ordering::Relaxed);
// We only use a single thread in order to make the behavior
// of 'panic_fuse' deterministic
rayon::ThreadPoolBuilder::new().num_threads(1).build().unwrap().install(|| {
fn count(iter: impl ParallelIterator + UnwindSafe) -> usize {
let count = AtomicUsize::new(0);
let result = panic::catch_unwind(|| {
iter.for_each(|_| {
count.fetch_add(1, Ordering::Relaxed);
});
});
});
assert!(result.is_err());
count.into_inner()
}

// Without `panic_fuse()`, we'll reach every item except the panicking one.
let expected = ITER.len() - 1;
let iter = ITER.into_par_iter().with_max_len(1);
assert_eq!(count(iter.clone().inspect(check)), expected);

// With `panic_fuse()` anywhere in the chain, we'll reach fewer items.
assert!(count(iter.clone().inspect(check).panic_fuse()) < expected);
assert!(count(iter.clone().panic_fuse().inspect(check)) < expected);

// Try in reverse to be sure we hit the producer case.
assert!(count(iter.clone().panic_fuse().inspect(check).rev()) < expected);
assert!(result.is_err());
count.into_inner()
}

// Without `panic_fuse()`, we'll reach every item except the panicking one.
let expected = ITER.len() - 1;
let iter = ITER.into_par_iter().with_max_len(1);
assert_eq!(count(iter.clone().inspect(check)), expected);

// With `panic_fuse()` anywhere in the chain, we'll reach fewer items.
assert_lt!(count(iter.clone().inspect(check).panic_fuse()), expected);
assert_lt!(count(iter.clone().panic_fuse().inspect(check)), expected);

// Try in reverse to be sure we hit the producer case.
assert_lt!(count(iter.clone().panic_fuse().inspect(check).rev()), expected);
});
}

0 comments on commit 9532eb5

Please sign in to comment.