<NME> lib.rs
<BEF> #[cfg(feature = "min")]
#[cfg_attr(feature = "dbus_enabled", allow(dead_code))]
#[macro_use]
extern crate nix;

#[macro_use]
extern crate serde_derive;
#[macro_use]
extern crate log;

#[cfg(test)]
#[macro_use]
extern crate proptest;

#[cfg(test)]
#[macro_use]
extern crate matches;

#[macro_use]
extern crate lazy_static;

#[macro_use]
#[macro_use]
extern crate log;

#[cfg(test)]
extern crate quickcheck;

#[cfg(feature = "dbus_enabled")]
pub mod dbus_api;

pub mod stratis;

#[cfg(feature = "min")]
pub mod jsonrpc;

#[cfg(feature = "systemd_compat")]
pub mod systemd;

<MSG> Better handle panics in unit tests

Before we execute the unit test we will do a sanity check on the
test environment which will panic and end that unit test if bad.  If
the environment is good we will run the test capturing the result
and then clean up capturing its result.  We will then ensure that
the test and the post clean ups were successful as well.  This
approach removes the potential of a panic during a panic and allowing
us to know when we encounter an issue during unit tests.

Note: This change incorporates a number of suggestions/improvements
from Mulhern.

Signed-off-by: Tony Asleson <1ef2d381aa56d512126ddaacc29e68136e4fdf55@redhat.com>
Signed-off-by: mulhern <7b51bcf507bcd7afb72bf8663752c0ddbeb517f6@redhat.com>

<DFF> @@ -23,6 +23,10 @@ extern crate serde_json;
 #[macro_use]
 extern crate log;
 
+#[cfg(test)]
+#[macro_use]
+extern crate error_chain;
+
 #[cfg(test)]
 extern crate quickcheck;
 

{'commit': 'diff --git a/Cargo.lock b/Cargo.lock\nindex 1547a4a4..bbef8a60 100644\n--- a/Cargo.lock\n+++ b/Cargo.lock\n@@ -12,6 +12,7 @@ dependencies = [\n  "devicemapper 0.13.0 (registry+https://github.com/rust-lang/crates.io-index)",\n  "enum_derive 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",\n  "env_logger 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)",\n+ "error-chain 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",\n  "libc 0.2.31 (registry+https://github.com/rust-lang/crates.io-index)",\n  "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",\n  "loopdev 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",\ndiff --git a/Cargo.toml b/Cargo.toml\nindex 0a5a2f29..b3b26db9 100644\n--- a/Cargo.toml\n+++ b/Cargo.toml\n@@ -24,6 +24,7 @@ env_logger="0.3.5"\n libc = "0.2"\n clippy = {version = "*", optional = true}\n mnt = "0.3.1"\n+error-chain = "0.11.0"\n \n [dependencies.uuid]\n version = "0.5"\ndiff --git a/src/engine/strat_engine/tests/loopbacked.rs b/src/engine/strat_engine/tests/loopbacked.rs\nindex 19312e73..129c4cad 100644\n--- a/src/engine/strat_engine/tests/loopbacked.rs\n+++ b/src/engine/strat_engine/tests/loopbacked.rs\n@@ -6,6 +6,7 @@ extern crate loopdev;\n \n use std::fs::OpenOptions;\n use std::io::{Seek, SeekFrom, Write};\n+use std::panic;\n use std::path::{Path, PathBuf};\n \n use devicemapper::{Bytes, IEC, Sectors};\n@@ -35,7 +36,6 @@ impl LoopTestDev {\n     /// Create a new loopbacked device.\n     /// Create its backing store of 1 GiB wiping the first 1 MiB.\n     pub fn new(lc: &LoopControl, path: &Path) -> LoopTestDev {\n-        clean_up();\n         let mut f = OpenOptions::new()\n             .read(true)\n             .write(true)\n@@ -63,7 +63,6 @@ impl LoopTestDev {\n \n impl Drop for LoopTestDev {\n     fn drop(&mut self) {\n-        clean_up();\n         self.ld.detach().unwrap()\n     }\n }\n@@ -94,7 +93,7 @@ fn get_devices(count: usize, dir: &TempDir) -> Vec<LoopTestDev> {\n \n /// Run the designated tests according to the specification.\n pub fn test_with_spec<F>(limits: DeviceLimits, test: F) -> ()\n-    where F: Fn(&[&Path]) -> ()\n+    where F: Fn(&[&Path]) -> () + panic::RefUnwindSafe\n {\n     let counts = get_device_counts(limits);\n \n@@ -108,6 +107,13 @@ pub fn test_with_spec<F>(limits: DeviceLimits, test: F) -> ()\n             .map(|x| x.ld.get_path().unwrap())\n             .collect();\n         let device_paths: Vec<&Path> = device_paths.iter().map(|x| x.as_path()).collect();\n-        test(&device_paths);\n+\n+        clean_up().unwrap();\n+\n+        let result = panic::catch_unwind(|| { test(&device_paths); });\n+        let tear_down = clean_up();\n+\n+        result.unwrap();\n+        tear_down.unwrap();\n     }\n }\ndiff --git a/src/engine/strat_engine/tests/real.rs b/src/engine/strat_engine/tests/real.rs\nindex 4289f97c..ab6e3080 100644\n--- a/src/engine/strat_engine/tests/real.rs\n+++ b/src/engine/strat_engine/tests/real.rs\n@@ -6,6 +6,7 @@ extern crate serde_json;\n \n use std::cmp;\n use std::fs::OpenOptions;\n+use std::panic;\n use std::path::{Path, PathBuf};\n \n use serde_json::{Value, from_reader};\n@@ -25,7 +26,6 @@ impl RealTestDev {\n     /// Construct a new test device for the given path.\n     /// Wipe initial MiB to clear metadata.\n     pub fn new(path: &Path) -> RealTestDev {\n-        clean_up();\n         wipe_sectors(path, Sectors(0), Bytes(IEC::Mi).sectors()).unwrap();\n         RealTestDev { path: PathBuf::from(path) }\n     }\n@@ -38,7 +38,6 @@ impl RealTestDev {\n \n impl Drop for RealTestDev {\n     fn drop(&mut self) {\n-        clean_up();\n         wipe_sectors(&self.path, Sectors(0), Bytes(IEC::Mi).sectors()).unwrap();\n     }\n }\n@@ -90,7 +89,7 @@ fn get_device_counts(limits: DeviceLimits, avail: usize) -> Vec<usize> {\n /// in multiple invocations of the test, with differing numbers of block\n /// devices.\n pub fn test_with_spec<F>(limits: DeviceLimits, test: F) -> ()\n-    where F: Fn(&[&Path]) -> ()\n+    where F: Fn(&[&Path]) -> () + panic::RefUnwindSafe\n {\n     let file = OpenOptions::new()\n         .read(true)\n@@ -118,6 +117,14 @@ pub fn test_with_spec<F>(limits: DeviceLimits, test: F) -> ()\n \n     for run_paths in runs {\n         let devices: Vec<_> = run_paths.iter().map(|x| RealTestDev::new(x)).collect();\n-        test(&devices.iter().map(|x| x.as_path()).collect::<Vec<_>>());\n+\n+        clean_up().unwrap();\n+\n+        let result =\n+            panic::catch_unwind(|| test(&devices.iter().map(|x| x.as_path()).collect::<Vec<_>>()));\n+        let tear_down = clean_up();\n+\n+        result.unwrap();\n+        tear_down.unwrap();\n     }\n }\ndiff --git a/src/engine/strat_engine/tests/util.rs b/src/engine/strat_engine/tests/util.rs\nindex bee7fe23..edc20ce7 100644\n--- a/src/engine/strat_engine/tests/util.rs\n+++ b/src/engine/strat_engine/tests/util.rs\n@@ -4,48 +4,90 @@\n \n use std::path::PathBuf;\n \n-use devicemapper::{DM, DmFlags, DevId};\n+use devicemapper::{DM, DmFlags, DmResult, DevId};\n \n use nix::mount::{MNT_DETACH, umount2};\n use mnt::get_submounts;\n \n+\n+mod cleanup_errors {\n+    use mnt;\n+    use nix;\n+\n+    error_chain!{\n+        foreign_links {\n+            Mnt(mnt::ParseError);\n+            Nix(nix::Error);\n+        }\n+    }\n+}\n+\n+use self::cleanup_errors::{Error, Result, ResultExt};\n+\n+\n /// Attempt to remove all device mapper devices which match the stratis naming convention.\n /// FIXME: Current implementation complicated by https://bugzilla.redhat.com/show_bug.cgi?id=1506287\n-fn dm_stratis_devices_remove() {\n+fn dm_stratis_devices_remove() -> Result<()> {\n \n-    let dm = DM::new().unwrap();\n-\n-    loop {\n+    /// One iteration of removing devicemapper devices\n+    fn one_iteration(dm: &DM) -> DmResult<(bool, Vec<String>)> {\n         let mut progress_made = false;\n-        for d in dm.list_devices()\n-                .unwrap()\n+        let mut remain = Vec::new();\n+\n+        for d in dm.list_devices()?\n                 .iter()\n                 .filter(|d| format!("{}", d.0.as_ref()).starts_with("stratis-1")) {\n-            progress_made |= dm.device_remove(&DevId::Name(&d.0), DmFlags::empty())\n-                .is_ok();\n+\n+            match dm.device_remove(&DevId::Name(&d.0), DmFlags::empty()) {\n+                Ok(_) => progress_made = true,\n+                Err(_) => remain.push(format!("{}", d.0.as_ref())),\n+            }\n         }\n+        Ok((progress_made, remain))\n+    }\n+\n+    let dm = DM::new().chain_err(|| "Unable to initialize DM")?;\n+\n+    loop {\n+        let (progress_made, remain) = one_iteration(&dm)\n+            .map_err(|e| {\n+                Error::with_chain(e,\n+                                  "Error while attempting to remove stratis device mapper devices")}\n+                )?;\n \n         if !progress_made {\n+            if remain.len() != 0 {\n+                bail!("We were unable to remove all stratis device mapper devices {:?}",\n+                      remain);\n+            }\n             break;\n         }\n     }\n+\n+    Ok(())\n }\n \n-/// Try and un-mount any filesystems that have the name stratis in the mount point.\n-fn stratis_filesystems_unmount() {\n-    for m in get_submounts(&PathBuf::from("/"))\n-            .unwrap()\n-            .iter()\n-            .filter(|m| m.file.to_str().map_or(false, |s| s.contains("stratis"))) {\n-        umount2(&m.file, MNT_DETACH).unwrap();\n-    }\n+/// Try and un-mount any filesystems that have the name stratis in the mount point, returning\n+/// immediately on the first one we are unable to unmount.\n+fn stratis_filesystems_unmount() -> Result<()> {\n+    || -> Result<()> {\n+        let mounts = get_submounts(&PathBuf::from("/"))?;\n+        for m in mounts\n+                .iter()\n+                .filter(|m| m.file.to_str().map_or(false, |s| s.contains("stratis"))) {\n+            umount2(&m.file, MNT_DETACH)?;\n+        }\n+        Ok(())\n+    }()\n+            .map_err(|e| Error::with_chain(e, "unable to unmount all stratis filesystems"))\n }\n \n /// When a unit test panics we can leave the system in an inconsistent state.  This function\n /// tries to clean up by un-mounting any mounted file systems which contain the string\n /// "stratis_testing" and then it tries to remove any device mapper tables which are also stratis\n /// created.\n-pub fn clean_up() -> () {\n-    stratis_filesystems_unmount();\n-    dm_stratis_devices_remove();\n+pub fn clean_up() -> Result<()> {\n+    stratis_filesystems_unmount()?;\n+    dm_stratis_devices_remove()?;\n+    Ok(())\n }\ndiff --git a/src/lib.rs b/src/lib.rs\nindex 98b45977..b54d9c27 100644\n--- a/src/lib.rs\n+++ b/src/lib.rs\n@@ -23,6 +23,10 @@ extern crate serde_json;\n #[macro_use]\n extern crate log;\n \n+#[cfg(test)]\n+#[macro_use]\n+extern crate error_chain;\n+\n #[cfg(test)]\n extern crate quickcheck;\n \n', 'commit_subject': 'Better handle panics in unit tests', 'repo_name': 'stratis-storage/stratisd', 'committer': {'name': 'Tony Asleson', 'email': '1ef2d381aa56d512126ddaacc29e68136e4fdf55@redhat.com', 'time_sec': '1511300977', 'tz_offset': '-360', 'date': {'seconds': '1511300977'}}, 'commit_tree': '957cacbab1f28cc99418bea3f5e9f4148d7a94c7', 'commit_message': 'Better handle panics in unit tests\n\nBefore we execute the unit test we will do a sanity check on the\ntest environment which will panic and end that unit test if bad.  If\nthe environment is good we will run the test capturing the result\nand then clean up capturing its result.  We will then ensure that\nthe test and the post clean ups were successful as well.  This\napproach removes the potential of a panic during a panic and allowing\nus to know when we encounter an issue during unit tests.\n\nNote: This change incorporates a number of suggestions/improvements\nfrom Mulhern.\n\nSigned-off-by: Tony Asleson <1ef2d381aa56d512126ddaacc29e68136e4fdf55@redhat.com>\nSigned-off-by: mulhern <7b51bcf507bcd7afb72bf8663752c0ddbeb517f6@redhat.com>\n', 'language_name': 'Python', 'license': 'mpl-2.0', 'hunks': '@@ -23,6 +23,10 @@ extern crate serde_json;\n #[macro_use]\n extern crate log;\n \n+#[cfg(test)]\n+#[macro_use]\n+extern crate error_chain;\n+\n #[cfg(test)]\n extern crate quickcheck;\n \n', 'addition_count': 4, 'deletion_count': 0, 'src_file': 'a/src/lib.rs', 'tgt_file': 'b/src/lib.rs', 'file_extension': '.rs', 'before_file': '#[cfg(feature = "min")]\n#[cfg_attr(feature = "dbus_enabled", allow(dead_code))]\n#[macro_use]\nextern crate nix;\n\n#[macro_use]\nextern crate serde_derive;\n#[macro_use]\nextern crate log;\n\n#[cfg(test)]\n#[macro_use]\nextern crate proptest;\n\n#[cfg(test)]\n#[macro_use]\nextern crate matches;\n\n#[macro_use]\nextern crate lazy_static;\n\n#[macro_use]\n#[macro_use]\nextern crate log;\n\n#[cfg(test)]\nextern crate quickcheck;\n\n#[cfg(feature = "dbus_enabled")]\npub mod dbus_api;\n\npub mod stratis;\n\n#[cfg(feature = "min")]\npub mod jsonrpc;\n\n#[cfg(feature = "systemd_compat")]\npub mod systemd;\n', 'message': 'Better handle panics in unit tests\n\nBefore we execute the unit test we will do a sanity check on the\ntest environment which will panic and end that unit test if bad.  If\nthe environment is good we will run the test capturing the result\nand then clean up capturing its result.  We will then ensure that\nthe test and the post clean ups were successful as well.  This\napproach removes the potential of a panic during a panic and allowing\nus to know when we encounter an issue during unit tests.\n\nNote: This change incorporates a number of suggestions/improvements\nfrom Mulhern.\n\nSigned-off-by: Tony Asleson <1ef2d381aa56d512126ddaacc29e68136e4fdf55@redhat.com>\nSigned-off-by: mulhern <7b51bcf507bcd7afb72bf8663752c0ddbeb517f6@redhat.com>\n', 'author_name': 'Tony Asleson', 'lang': 'rs'}