Remove unused locks and simplify tests.

The OutputBuffer construct was entirely unnecessary, given the presence of UI::with_sinks. Signed-off-by: Josh Black <raskchanky@gmail.com>
habitat-sh · Sep 4, 2019 · 03ff50c · 03ff50c
1 parent 6829884
commit 03ff50c
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 161 deletions.
diff --git a/components/common/src/lib.rs b/components/common/src/lib.rs
@@ -32,6 +32,7 @@ pub mod locked_env_var;
 pub mod output;
 pub mod package_graph;
 pub mod templating;
+pub mod test_helpers;
 pub mod types;
 pub mod ui;
 pub mod util;

diff --git a/components/common/src/templating/test_helpers.rs b/components/common/src/templating/test_helpers.rs
@@ -1,11 +1,10 @@
+use crate::json;
+use serde_json;
 use std::{fs::File,
           io::{Read,
                Write},
           path::{Path,
                  PathBuf}};
-
-use crate::json;
-use serde_json;
 use valico::json_schema;
 
 pub fn create_with_content<P>(path: P, content: &str)

diff --git a/components/hab/src/command/bldr/job/promote.rs b/components/hab/src/command/bldr/job/promote.rs
@@ -142,13 +142,6 @@ pub fn start(ui: &mut UI,
 
 #[cfg(test)]
 mod test {
-    use std::{io::{self,
-                   Cursor,
-                   Write},
-              sync::{Arc,
-                     RwLock}};
-    use termcolor::ColorChoice;
-
     use super::get_ident_list;
     use crate::{api_client::{Project,
                              SchedulerResponse},
@@ -169,49 +162,9 @@ mod test {
         vec![project1, project2]
     }
 
-    fn ui() -> (UI, OutputBuffer, OutputBuffer) {
-        let stdout_buf = OutputBuffer::new();
-        let stderr_buf = OutputBuffer::new();
-
-        let ui = UI::with_streams(Box::new(io::empty()),
-                                  || Box::new(stdout_buf.clone()),
-                                  || Box::new(stderr_buf.clone()),
-                                  ColorChoice::Never,
-                                  false);
-
-        (ui, stdout_buf, stderr_buf)
-    }
-
-    #[derive(Clone)]
-    pub struct OutputBuffer {
-        pub cursor: Arc<RwLock<Cursor<Vec<u8>>>>,
-    }
-
-    impl OutputBuffer {
-        fn new() -> Self {
-            OutputBuffer { cursor: Arc::new(RwLock::new(Cursor::new(Vec::new()))), }
-        }
-    }
-
-    impl Write for OutputBuffer {
-        fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
-            self.cursor
-                .write()
-                .expect("Cursor lock is poisoned")
-                .write(buf)
-        }
-
-        fn flush(&mut self) -> io::Result<()> {
-            self.cursor
-                .write()
-                .expect("Cursor lock is poisoned")
-                .flush()
-        }
-    }
-
     #[test]
     fn test_get_ident_list() {
-        let (mut ui, _stdout, _stderr) = ui();
+        let mut ui = UI::with_sinks();
         let group_status = SchedulerResponse { id:           "12345678".to_string(),
                                                state:        "Finished".to_string(),
                                                projects:     sample_project_list(),

diff --git a/components/hab/src/command/pkg/binlink.rs b/components/hab/src/command/pkg/binlink.rs
@@ -242,30 +242,23 @@ impl Binlink {
 
 #[cfg(test)]
 mod test {
+    use super::{binlink_all_in_pkg,
+                start,
+                Binlink};
+    use crate::{common::ui::UI,
+                hcore::{self,
+                        package::{PackageIdent,
+                                  PackageTarget}}};
     use std::{collections::HashMap,
               env,
               fs::{self,
                    File},
-              io::{self,
-                   Cursor,
-                   Write},
+              io::Write,
               path::Path,
               str::{self,
-                    FromStr},
-              sync::{Arc,
-                     RwLock}};
-    use termcolor::ColorChoice;
-
-    use crate::{common::ui::UI,
-                hcore::{self,
-                        package::{PackageIdent,
-                                  PackageTarget}}};
+                    FromStr}};
     use tempfile::TempDir;
 
-    use super::{binlink_all_in_pkg,
-                start,
-                Binlink};
-
     #[test]
     fn start_symlinks_binaries() {
         let rootfs = TempDir::new().unwrap();
@@ -282,7 +275,7 @@ mod test {
         let rootfs_bin_dir = rootfs.path().join("opt/bin");
         let force = true;
 
-        let (mut ui, _stdout, _stderr) = ui();
+        let mut ui = UI::with_sinks();
 
         #[cfg(target_os = "linux")]
         let magicate_link = "magicate.exe";
@@ -348,7 +341,7 @@ mod test {
         #[cfg(target_os = "windows")]
         let securitize_link = "securitize.bat";
 
-        let (mut ui, _stdout, _stderr) = ui();
+        let mut ui = UI::with_sinks();
         binlink_all_in_pkg(&mut ui, &ident, &dst_path, rootfs.path(), force).unwrap();
 
         assert_eq!(rootfs_src_dir.join("bin/magicate.exe"),
@@ -378,7 +371,7 @@ mod test {
         let rootfs_bin_dir = rootfs.path().join("opt/bin");
         let force = true;
 
-        let (mut ui, _stdout, _stderr) = ui();
+        let mut ui = UI::with_sinks();
         binlink_all_in_pkg(&mut ui, &ident, &dst_path, rootfs.path(), force).unwrap();
 
         assert_eq!(rootfs_src_dir.join("bin/magicate.exe"),
@@ -420,7 +413,7 @@ mod test {
         #[cfg(target_os = "windows")]
         let bonus_round_link = "bonus-round.bat";
 
-        let (mut ui, _stdout, _stderr) = ui();
+        let mut ui = UI::with_sinks();
         binlink_all_in_pkg(&mut ui, &ident, &dst_path, rootfs.path(), force).unwrap();
 
         assert_eq!(rootfs_src_dir.join("bin/magicate.exe"),
@@ -431,19 +424,6 @@ mod test {
                                                                              .target);
     }
 
-    fn ui() -> (UI, OutputBuffer, OutputBuffer) {
-        let stdout_buf = OutputBuffer::new();
-        let stderr_buf = OutputBuffer::new();
-
-        let ui = UI::with_streams(Box::new(io::empty()),
-                                  || Box::new(stdout_buf.clone()),
-                                  || Box::new(stderr_buf.clone()),
-                                  ColorChoice::Never,
-                                  false);
-
-        (ui, stdout_buf, stderr_buf)
-    }
-
     fn fake_bin_pkg_install<P>(ident: &str,
                                binaries: HashMap<&str, Vec<&str>>,
                                rootfs: P)
@@ -484,31 +464,4 @@ mod test {
         f.write_all(content.as_bytes())
          .expect("Bytes not written to file");
     }
-
-    #[derive(Clone)]
-    pub struct OutputBuffer {
-        pub cursor: Arc<RwLock<Cursor<Vec<u8>>>>,
-    }
-
-    impl OutputBuffer {
-        fn new() -> Self {
-            OutputBuffer { cursor: Arc::new(RwLock::new(Cursor::new(Vec::new()))), }
-        }
-    }
-
-    impl Write for OutputBuffer {
-        fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
-            self.cursor
-                .write()
-                .expect("Cursor lock is poisoned")
-                .write(buf)
-        }
-
-        fn flush(&mut self) -> io::Result<()> {
-            self.cursor
-                .write()
-                .expect("Cursor lock is poisoned")
-                .flush()
-        }
-    }
 }
diff --git a/components/pkg-export-docker/src/build.rs b/components/pkg-export-docker/src/build.rs
@@ -854,18 +854,12 @@ mod test {
         use super::{super::*,
                     *};
         use crate::common::ui::UI;
-        use std::{io::{self,
-                       Cursor,
-                       Write},
-                  sync::{Arc,
-                         RwLock}};
         use tempfile::TempDir;
-        use termcolor::ColorChoice;
 
         #[test]
         fn artifact_cache_symlink() {
             let rootfs = TempDir::new().unwrap();
-            let (mut ui, ..) = ui();
+            let mut ui = UI::with_sinks();
             build_spec().create_symlink_to_artifact_cache(&mut ui, rootfs.path())
                         .unwrap();
             let link = rootfs.path().join(CACHE_ARTIFACT_PATH);
@@ -877,7 +871,7 @@ mod test {
         #[test]
         fn key_cache_symlink() {
             let rootfs = TempDir::new().unwrap();
-            let (mut ui, ..) = ui();
+            let mut ui = UI::with_sinks();
             build_spec().create_symlink_to_key_cache(&mut ui, rootfs.path())
                         .unwrap();
             let link = rootfs.path().join(CACHE_KEY_PATH);
@@ -889,7 +883,7 @@ mod test {
         #[test]
         fn link_binaries() {
             let rootfs = TempDir::new().unwrap();
-            let (mut ui, ..) = ui();
+            let mut ui = UI::with_sinks();
             let base_pkgs = base_pkgs(rootfs.path());
             build_spec().link_binaries(&mut ui, rootfs.path(), &base_pkgs)
                         .unwrap();
@@ -913,7 +907,7 @@ mod test {
         #[test]
         fn link_cacerts() {
             let rootfs = TempDir::new().unwrap();
-            let (mut ui, ..) = ui();
+            let mut ui = UI::with_sinks();
             let base_pkgs = base_pkgs(rootfs.path());
             build_spec().link_cacerts(&mut ui, rootfs.path(), &base_pkgs)
                         .unwrap();
@@ -923,19 +917,6 @@ mod test {
                        "cacerts are symlinked into /etc/ssl");
         }
 
-        fn ui() -> (UI, OutputBuffer, OutputBuffer) {
-            let stdout_buf = OutputBuffer::new();
-            let stderr_buf = OutputBuffer::new();
-
-            let ui = UI::with_streams(Box::new(io::empty()),
-                                      || Box::new(stdout_buf.clone()),
-                                      || Box::new(stderr_buf.clone()),
-                                      ColorChoice::Never,
-                                      false);
-
-            (ui, stdout_buf, stderr_buf)
-        }
-
         #[cfg(not(windows))]
         fn base_pkgs<P: AsRef<Path>>(rootfs: P) -> BasePkgIdents {
             BasePkgIdents { hab:      fake_hab_install(&rootfs),
@@ -978,33 +959,6 @@ mod test {
             util::write_file(prefix.join("ssl/cacert.pem"), "").unwrap();
             ident
         }
-
-        #[derive(Clone)]
-        pub struct OutputBuffer {
-            pub cursor: Arc<RwLock<Cursor<Vec<u8>>>>,
-        }
-
-        impl OutputBuffer {
-            fn new() -> Self {
-                OutputBuffer { cursor: Arc::new(RwLock::new(Cursor::new(Vec::new()))), }
-            }
-        }
-
-        impl Write for OutputBuffer {
-            fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
-                self.cursor
-                    .write()
-                    .expect("Cursor lock is poisoned")
-                    .write(buf)
-            }
-
-            fn flush(&mut self) -> io::Result<()> {
-                self.cursor
-                    .write()
-                    .expect("Cursor lock is poisoned")
-                    .flush()
-            }
-        }
     }
 
     mod build_root_context {