From 3f3f8e118f3278b2509f48804b7906bd4d957180 Mon Sep 17 00:00:00 2001 From: Luca Bruno Date: Tue, 9 Jul 2019 17:22:58 +0000 Subject: [PATCH] fail: add test-mutex pattern directly to the library (#40) * fail: derive some more Debug * fail: add test-mutex pattern directly to the library This adds a `FailScenario` to the library, exposing the usual idiom of a global testing mutex and individual guards owned by each test. The global mutex is kept as an internal detail and not exposed, consumers can only borrow a guard via the scenario. --- README.md | 7 +- src/lib.rs | 211 +++++++++++++++++++++++++++-------------------------- 2 files changed, 112 insertions(+), 106 deletions(-) diff --git a/README.md b/README.md index e1542c0..87a42e8 100644 --- a/README.md +++ b/README.md @@ -26,8 +26,7 @@ Fail points generation by this macro is disabled by default, and can be enabled As an example, here's a simple program that uses a fail point to simulate an I/O panic: ```rust -#[macro_use] -extern crate fail; +use fail::{fail_point, FailScenario}; fn do_fallible_work() { fail_point!("read-dir"); @@ -36,9 +35,9 @@ fn do_fallible_work() { } fn main() { - fail::setup(); + let scenario = FailScenario::setup(); do_fallible_work(); - fail::teardown(); + scenario.teardown(); println!("done"); } ``` diff --git a/src/lib.rs b/src/lib.rs index b1c3f8a..4d41ac0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -39,8 +39,7 @@ //! I/O panic: //! //! ```rust -//! #[macro_use] -//! extern crate fail; +//! use fail::{fail_point, FailScenario}; //! //! fn do_fallible_work() { //! fail_point!("read-dir"); @@ -49,9 +48,9 @@ //! } //! //! fn main() { -//! fail::setup(); +//! let scenario = FailScenario::setup(); //! do_fallible_work(); -//! fail::teardown(); +//! scenario.teardown(); //! println!("done"); //! } //! ``` @@ -88,21 +87,15 @@ //! //! The previous example triggers a fail point by modifying the `FAILPOINT` //! environment variable. In practice, you'll often want to trigger fail points -//! programmatically, in unit tests. Unfortunately, unit testing with fail -//! points is complicated by concurrency concerns, so requires some careful -//! setup. Fail points are global resources, and Rust tests run in parallel, +//! programmatically, in unit tests. +//! Fail points are global resources, and Rust tests run in parallel, //! so tests that exercise fail points generally need to hold a lock to -//! avoid interfering with each other. +//! avoid interfering with each other. This is accomplished by `FailScenario`. //! //! Here's a basic pattern for writing unit tests tests with fail points: //! -//! ``` -//! #[macro_use] -//! extern crate lazy_static; -//! #[macro_use] -//! extern crate fail; -//! -//! use std::sync::{Mutex, MutexGuard}; +//! ```rust +//! use fail::{fail_point, FailScenario}; //! //! fn do_fallible_work() { //! fail_point!("read-dir"); @@ -110,32 +103,19 @@ //! // ... do some work on the directory ... //! } //! -//! lazy_static! { -//! static ref LOCK: Mutex<()> = Mutex::new(()); -//! } -//! -//! fn setup<'a>() -> MutexGuard<'a, ()> { -//! let guard = LOCK.lock().unwrap_or_else(|e| e.into_inner()); -//! fail::teardown(); -//! fail::setup(); -//! guard -//! } -//! //! #[test] //! #[should_panic] //! fn test_fallible_work() { -//! let _gaurd = setup(); +//! let scenario = FailScenario::setup(); //! fail::cfg("read-dir", "panic").unwrap(); +//! //! do_fallible_work(); +//! +//! scenario.teardown(); //! } //! # fn main() { } //! ``` //! -//! With this arrangement, any test that calls `setup` and holds the resulting -//! guard for the duration will not run in parallel with other tests. It depends -//! on the [`lazy_static`](https://crates.io/crates/lazy_static) crate to -//! initialize a global mutex. -//! //! Even if a test does not itself turn on any fail points, code that it runs //! could trigger a fail point that was configured by another thread. Because of //! this it is a best practice to put all fail point unit tests into their own @@ -165,9 +145,7 @@ //! function we used earlier to return a `Result`: //! //! ```rust -//! #[macro_use] -//! extern crate fail; -//! +//! use fail::{fail_point, FailScenario}; //! use std::io; //! //! fn do_fallible_work() -> io::Result<()> { @@ -178,9 +156,9 @@ //! } //! //! fn main() -> io::Result<()> { -//! fail::setup(); +//! let scenario = FailScenario::setup(); //! do_fallible_work()?; -//! fail::teardown(); +//! scenario.teardown(); //! println!("done"); //! Ok(()) //! } @@ -211,10 +189,9 @@ //! Here's a variation that does so: //! //! ```rust -//! # #[macro_use] extern crate fail; //! # use std::io; //! fn do_fallible_work() -> io::Result<()> { -//! fail_point!("read-dir", |_| { +//! fail::fail_point!("read-dir", |_| { //! Err(io::Error::new(io::ErrorKind::PermissionDenied, "error")) //! }); //! let _dir: Vec<_> = std::fs::read_dir(".")?.collect(); @@ -254,7 +231,7 @@ //! feature. When failpoints are disabled, no code is generated by the macro. //! - Carefully consider complex, concurrent, non-deterministic combinations of //! fail points. Put test cases exercising fail points into their own test -//! crate and protect each test case with a mutex guard. +//! crate. //! - Fail points might have the same name, in which case they take the //! same actions. Be careful about duplicating fail point names, either within //! a single crate, or across multiple crates. @@ -265,7 +242,7 @@ use std::collections::HashMap; use std::env::VarError; use std::str::FromStr; use std::sync::atomic::{AtomicUsize, Ordering}; -use std::sync::{Arc, Condvar, Mutex, RwLock, TryLockError}; +use std::sync::{Arc, Condvar, Mutex, MutexGuard, RwLock, TryLockError}; use std::time::{Duration, Instant}; use std::{env, thread}; @@ -420,6 +397,8 @@ impl FromStr for Action { } } +#[cfg_attr(feature = "cargo-clippy", allow(clippy::mutex_atomic))] +#[derive(Debug)] struct FailPoint { pause: Mutex, pause_notifier: Condvar, @@ -501,82 +480,110 @@ impl FailPoint { } } -#[derive(Default)] +/// Registry with failpoints configuration. +type Registry = HashMap>; + +#[derive(Debug, Default)] struct FailPointRegistry { // TODO: remove rwlock or store *mut FailPoint - registry: RwLock>>, + registry: RwLock, } lazy_static! { static ref REGISTRY: FailPointRegistry = FailPointRegistry::default(); + static ref SCENARIO: Mutex<&'static FailPointRegistry> = Mutex::new(®ISTRY); } -/// Returns whether code generation for failpoints is enabled. -/// -/// This function allows consumers to check (at runtime) whether the library -/// was compiled with the (buildtime) `failpoints` feature, which enables -/// code generation for failpoints. -pub const fn has_failpoints() -> bool { - cfg!(feature = "failpoints") +/// Test scenario with configured fail points. +#[derive(Debug)] +pub struct FailScenario<'a> { + scenario_guard: MutexGuard<'a, &'static FailPointRegistry>, } -/// Set up the fail point system. -/// -/// Configures all fail points specified in the `FAILPOINTS` environment variable. -/// It does not otherwise change any existing fail point configuration -/// -/// The format of `FAILPOINTS` is `failpoint=actions;...`, where -/// `failpoint` is the name of the fail point. For more information -/// about fail point actions see the [`cfg`](fn.cfg.html) function and -/// the [`fail_point`](macro.fail_point.html) macro. -/// -/// `FAILPOINTS` may configure fail points that are not actually defined. In -/// this case the configuration has no effect. -/// -/// This function should generally be called prior to running a test with fail -/// points, and afterward paired with [`teardown`](fn.teardown.html). -/// -/// # Panics -/// -/// Panics if an action is not formatted correctly. -pub fn setup() { - let mut registry = REGISTRY.registry.write().unwrap(); - let failpoints = match env::var("FAILPOINTS") { - Ok(s) => s, - Err(VarError::NotPresent) => return, - Err(e) => panic!("invalid failpoints: {:?}", e), - }; - for mut cfg in failpoints.trim().split(';') { - cfg = cfg.trim(); - if cfg.is_empty() { - continue; - } - let (name, order) = partition(cfg, '='); - match order { - None => panic!("invalid failpoint: {:?}", cfg), - Some(order) => { - if let Err(e) = set(&mut registry, name.to_owned(), order) { - panic!("unable to configure failpoint \"{}\": {}", name, e); +impl<'a> FailScenario<'a> { + /// Set up the system for a fail points scenario. + /// + /// Configures all fail points specified in the `FAILPOINTS` environment variable. + /// It does not otherwise change any existing fail point configuration. + /// + /// The format of `FAILPOINTS` is `failpoint=actions;...`, where + /// `failpoint` is the name of the fail point. For more information + /// about fail point actions see the [`cfg`](fn.cfg.html) function and + /// the [`fail_point`](macro.fail_point.html) macro. + /// + /// `FAILPOINTS` may configure fail points that are not actually defined. In + /// this case the configuration has no effect. + /// + /// This function should generally be called prior to running a test with fail + /// points, and afterward paired with [`teardown`](#method.teardown). + /// + /// # Panics + /// + /// Panics if an action is not formatted correctly. + pub fn setup() -> Self { + // Cleanup first, in case of previous failed/panic'ed test scenarios. + let scenario_guard = SCENARIO.lock().unwrap_or_else(|e| e.into_inner()); + let mut registry = scenario_guard.registry.write().unwrap(); + Self::cleanup(&mut registry); + + let failpoints = match env::var("FAILPOINTS") { + Ok(s) => s, + Err(VarError::NotPresent) => return Self { scenario_guard }, + Err(e) => panic!("invalid failpoints: {:?}", e), + }; + for mut cfg in failpoints.trim().split(';') { + cfg = cfg.trim(); + if cfg.is_empty() { + continue; + } + let (name, order) = partition(cfg, '='); + match order { + None => panic!("invalid failpoint: {:?}", cfg), + Some(order) => { + if let Err(e) = set(&mut registry, name.to_owned(), order) { + panic!("unable to configure failpoint \"{}\": {}", name, e); + } } } } + Self { scenario_guard } + } + + /// Tear down the fail point system. + /// + /// Clears the configuration of all fail points. Any paused fail + /// points will be notified before they are deactivated. + /// + /// This function should generally be called after running a test with fail points. + /// Calling `teardown` without previously calling `setup` results in a no-op. + pub fn teardown(self) { + drop(self) + } + + /// Clean all registered fail points. + fn cleanup(registry: &mut std::sync::RwLockWriteGuard<'a, Registry>) { + for p in registry.values() { + // wake up all pause failpoint. + p.set_actions("", vec![]); + } + registry.clear(); } } -/// Tear down the fail point system. -/// -/// Clears the configuration of all fail points. Any paused fail -/// points will be notified before they are deactivated. -/// -/// This function should generally be called after running a test with fail points. -/// Calling `teardown` without previously calling `setup` results in a no-op. -pub fn teardown() { - let mut registry = REGISTRY.registry.write().unwrap(); - for p in registry.values() { - // wake up all pause failpoint. - p.set_actions("", vec![]); +impl<'a> Drop for FailScenario<'a> { + fn drop(&mut self) { + let mut registry = self.scenario_guard.registry.write().unwrap(); + Self::cleanup(&mut registry) } - registry.clear(); +} + +/// Returns whether code generation for failpoints is enabled. +/// +/// This function allows consumers to check (at runtime) whether the library +/// was compiled with the (buildtime) `failpoints` feature, which enables +/// code generation for failpoints. +pub const fn has_failpoints() -> bool { + cfg!(feature = "failpoints") } /// Get all registered fail points. @@ -955,7 +962,7 @@ mod tests { "FAILPOINTS", "setup_and_teardown1=return;setup_and_teardown2=pause;", ); - setup(); + let scenario = FailScenario::setup(); assert_eq!(f1(), 1); let (tx, rx) = mpsc::channel(); @@ -964,7 +971,7 @@ mod tests { }); assert!(rx.recv_timeout(Duration::from_millis(500)).is_err()); - teardown(); + scenario.teardown(); assert_eq!(rx.recv_timeout(Duration::from_millis(500)).unwrap(), 0); assert_eq!(f1(), 0); }