Refactor tests

Make use of the 'is' more to test the entire output.

Add test for no_color mode.

Replace several calls to format_string with calls that build a string
directly. I feel format_string is becoming unweildly and this simplfies
the tests

src/display.rs

@@ -9,6 +9,7 @@ static UNITS: [char; 4] = ['T', 'G', 'M', 'K'];
 pub struct DisplayData {
     pub short_paths: bool,
     pub is_reversed: bool,
+    pub colors_on: bool,
 }
 
 impl DisplayData {
@@ -76,7 +77,7 @@ pub fn draw_it(
     permissions: bool,
     use_full_path: bool,
     is_reversed: bool,
-    colors_on: bool,
+    no_colors: bool,
     root_node: Node,
 ) {
     if !permissions {
@@ -85,23 +86,16 @@ pub fn draw_it(
     let display_data = DisplayData {
         short_paths: !use_full_path,
         is_reversed,
+        colors_on: !no_colors,
     };
 
     for c in display_data.get_children_from_node(root_node) {
         let first_tree_chars = display_data.get_first_chars();
-        display_node(c, true, colors_on, first_tree_chars, &display_data)
+        display_node(c, true, first_tree_chars, &display_data)
     }
 }
 
-fn display_node(
+fn display_node(node: Node, is_biggest: bool, indent: &str, display_data: &DisplayData) {
-    node: Node,
-    is_biggest: bool,
-    colors_on: bool,
-    indent: &str,
-    display_data: &DisplayData,
-) {
-    let short = display_data.short_paths;
-
     let mut num_siblings = node.children.len() as u64;
     let max_sibling = num_siblings;
     let new_indent = clean_indentation_string(indent);
@@ -109,7 +103,7 @@ fn display_node(
     let size = node.size;
 
     if !display_data.is_reversed {
-        print_this_node(&*name, size, is_biggest, short, colors_on, indent);
+        print_this_node(&*name, size, is_biggest, display_data, indent);
     }
 
     for c in display_data.get_children_from_node(node) {
@@ -117,11 +111,11 @@ fn display_node(
         let chars = display_data.get_tree_chars(num_siblings, max_sibling, !c.children.is_empty());
         let is_biggest = display_data.is_biggest(num_siblings, max_sibling);
         let full_indent = new_indent.clone() + chars;
-        display_node(c, is_biggest, colors_on, &*full_indent, display_data)
+        display_node(c, is_biggest, &*full_indent, display_data)
     }
 
     if display_data.is_reversed {
-        print_this_node(&*name, size, is_biggest, short, colors_on, indent);
+        print_this_node(&*name, size, is_biggest, display_data, indent);
     }
 }
 
@@ -146,34 +140,25 @@ fn print_this_node(
     name: &str,
     size: u64,
     is_biggest: bool,
-    short_paths: bool,
+    display_data: &DisplayData,
-    colors_on: bool,
     indentation: &str,
 ) {
     let pretty_size = format!("{:>5}", human_readable_number(size),);
     println!(
         "{}",
-        format_string(
+        format_string(name, is_biggest, display_data, &*pretty_size, indentation)
-            name,
-            is_biggest,
-            short_paths,
-            colors_on,
-            &*pretty_size,
-            indentation
-        )
     )
 }
 
 pub fn format_string(
     dir_name: &str,
     is_biggest: bool,
-    short_paths: bool,
+    display_data: &DisplayData,
-    colors_on: bool,
     size: &str,
     indentation: &str,
 ) -> String {
     let printable_name = {
-        if short_paths {
+        if display_data.short_paths {
             dir_name.split('/').last().unwrap_or(dir_name)
         } else {
             dir_name
@@ -181,7 +166,7 @@ pub fn format_string(
     };
     format!(
         "{} {} {}",
-        if is_biggest && colors_on {
+        if is_biggest && display_data.colors_on {
             Fixed(196).paint(size)
         } else {
             Style::new().paint(size)

src/main.rs

@@ -101,7 +101,6 @@ fn main() {
     }
 
     let use_apparent_size = options.is_present("display_apparent_size");
-    let use_full_path = options.is_present("display_full_paths");
 
     let simplified_dirs = simplify_dir_names(target_dirs);
     let (permissions, nodes) = get_dir_tree(&simplified_dirs, use_apparent_size, threads);
@@ -116,9 +115,9 @@ fn main() {
 
     draw_it(
         permissions,
-        use_full_path,
+        options.is_present("display_full_paths"),
         options.is_present("reverse"),
-        !options.is_present("no_colors"),
+        options.is_present("no_colors"),
         tree,
     );
 }

src/tests.rs

@@ -1,4 +1,5 @@
 use super::*;
+use crate::display::DisplayData;
 use display::format_string;
 use std::fs::File;
 use std::io::Write;
@@ -37,67 +38,83 @@ pub fn test_main_multi_arg() {
 
 #[cfg(target_os = "macos")]
 fn main_output(short_paths: bool) -> String {
+    let d = DisplayData {
+        short_paths,
+        is_reversed: false,
+        colors_on: true,
+    };
     format!(
         "{}
 {}
 {}
 {}",
-        format_string("src/test_dir", true, short_paths, " 4.0K", "─┬"),
+        format_string("src/test_dir", true, &d, " 4.0K", "─┬"),
-        format_string("src/test_dir/many", true, short_paths, " 4.0K", " └─┬",),
+        format_string("src/test_dir/many", true, &d, " 4.0K", " └─┬",),
-        format_string(
+        format_string("src/test_dir/many/hello_file", true, &d, " 4.0K", "   ├──",),
-            "src/test_dir/many/hello_file",
+        format_string("src/test_dir/many/a_file", false, &d, "   0B", "   └──",),
-            true,
-            short_paths,
-            " 4.0K",
-            "   ├──",
-        ),
-        format_string(
-            "src/test_dir/many/a_file",
-            false,
-            short_paths,
-            "   0B",
-            "   └──",
-        ),
     )
 }
 
 #[cfg(target_os = "linux")]
 fn main_output(short_paths: bool) -> String {
+    let d = DisplayData {
+        short_paths,
+        is_reversed: false,
+        colors_on: true,
+    };
     format!(
         "{}
 {}
 {}
 {}",
-        format_string("src/test_dir", true, short_paths, "  12K", "─┬"),
+        format_string("src/test_dir", true, &d, "  12K", "─┬"),
-        format_string("src/test_dir/many", true, short_paths, " 8.0K", " └─┬",),
+        format_string("src/test_dir/many", true, &d, " 8.0K", " └─┬",),
-        format_string(
+        format_string("src/test_dir/many/hello_file", true, &d, " 4.0K", "   ├──",),
-            "src/test_dir/many/hello_file",
+        format_string("src/test_dir/many/a_file", false, &d, "   0B", "   └──",),
-            true,
-            short_paths,
-            " 4.0K",
-            "   ├──",
-        ),
-        format_string(
-            "src/test_dir/many/a_file",
-            false,
-            short_paths,
-            "   0B",
-            "   └──",
-        ),
     )
 }
 
+#[test]
+pub fn test_no_color_flag() {
+    assert_cli::Assert::main_binary()
+        .with_args(&["-c", "src/test_dir/"])
+        .stdout()
+        .is(no_color_flag_output())
+        .unwrap();
+}
+
+#[cfg(target_os = "macos")]
+fn no_color_flag_output() -> String {
+    "
+ 4.0K ─┬ test_dir
+ 4.0K  └─┬ many
+ 4.0K    ├── hello_file
+   0B    └── a_file
+    "
+    .to_string()
+}
+
+#[cfg(target_os = "linux")]
+fn no_color_flag_output() -> String {
+    "
+  12K ─┬ test_dir
+ 8.0K  └─┬ many
+ 4.0K    ├── hello_file
+   0B    └── a_file
+    "
+    .to_string()
+}
+
 #[test]
 pub fn test_apparent_size() {
+    let d = DisplayData {
+        short_paths: true,
+        is_reversed: false,
+        colors_on: true,
+    };
     let r = format!(
         "{}",
-        format_string(
+        format_string("src/test_dir/many/hello_file", true, &d, "   6B", "   ├──",),
-            "src/test_dir/many/hello_file",
-            true,
-            true,
-            "   6B",
-            "   ├──",
-        ),
     );
 
     assert_cli::Assert::main_binary()
@@ -161,41 +178,21 @@ pub fn test_soft_sym_link() {
         .output();
     assert!(c.is_ok());
 
-    let r = soft_sym_link_output(dir_s, file_path_s, link_name_s);
+    let a = format!(" ─┬ {}", dir_s);
+    let b = format!("  ├── {}", file_path_s);
+    let c = format!("  └── {}", link_name_s);
 
-    // We cannot guarantee which version will appear first.
-    // TODO: Consider adding predictable iteration order (sort file entries by name?)
     assert_cli::Assert::main_binary()
-        .with_args(&[dir_s])
+        .with_args(&["-p", &dir_s])
         .stdout()
-        .contains(r)
+        .contains(a)
+        .stdout()
+        .contains(b)
+        .stdout()
+        .contains(c)
         .unwrap();
 }
 
-#[cfg(target_os = "macos")]
-fn soft_sym_link_output(dir: &str, file_path: &str, link_name: &str) -> String {
-    format!(
-        "{}
-{}
-{}",
-        format_string(dir, true, true, " 8.0K", "─┬"),
-        format_string(file_path, true, true, " 4.0K", " ├──",),
-        format_string(link_name, false, true, " 4.0K", " └──",),
-    )
-}
-
-#[cfg(target_os = "linux")]
-fn soft_sym_link_output(dir: &str, file_path: &str, link_name: &str) -> String {
-    format!(
-        "{}
-{}
-{}",
-        format_string(dir, true, true, " 8.0K", "─┬"),
-        format_string(file_path, true, true, " 4.0K", " ├──",),
-        format_string(link_name, false, true, "   0B", " └──",),
-    )
-}
-
 // Hard links are ignored as the inode is the same as the file
 #[test]
 pub fn test_hard_sym_link() {
@@ -212,61 +209,32 @@ pub fn test_hard_sym_link() {
         .output();
     assert!(c.is_ok());
 
-    let (r, r2) = hard_link_output(dir_s, file_path_s, link_name_s);
+    let a = format!(" ─┬ {}", dir_s);
+    let b = format!("  └── {}", link_name_s);
+    let b2 = format!("  └── {}", file_path_s);
 
     // Because this is a hard link the file and hard link look identical. Therefore
     // we cannot guarantee which version will appear first.
-    // TODO: Consider adding predictable iteration order (sort file entries by name?)
     let result = panic::catch_unwind(|| {
         assert_cli::Assert::main_binary()
-            .with_args(&[dir_s])
+            .with_args(&["-p", dir_s])
             .stdout()
-            .contains(r)
+            .contains(a.clone())
+            .stdout()
+            .contains(b)
             .unwrap();
     });
     if result.is_err() {
         assert_cli::Assert::main_binary()
-            .with_args(&[dir_s])
+            .with_args(&["-p", dir_s])
             .stdout()
-            .contains(r2)
+            .contains(a)
+            .stdout()
+            .contains(b2)
             .unwrap();
     }
 }
 
-#[cfg(target_os = "macos")]
-fn hard_link_output(dir_s: &str, file_path_s: &str, link_name_s: &str) -> (String, String) {
-    let r = format!(
-        "{}
-{}",
-        format_string(dir_s, true, true, " 4.0K", "─┬"),
-        format_string(file_path_s, true, true, " 4.0K", " └──")
-    );
-    let r2 = format!(
-        "{}
-{}",
-        format_string(dir_s, true, true, " 4.0K", "─┬"),
-        format_string(link_name_s, true, true, " 4.0K", " └──")
-    );
-    (r, r2)
-}
-
-#[cfg(target_os = "linux")]
-fn hard_link_output(dir_s: &str, file_path_s: &str, link_name_s: &str) -> (String, String) {
-    let r = format!(
-        "{}
-{}",
-        format_string(dir_s, true, true, " 8.0K", "─┬"),
-        format_string(file_path_s, true, true, " 4.0K", " └──")
-    );
-    let r2 = format!(
-        "{}
-{}",
-        format_string(dir_s, true, true, " 8.0K", "─┬"),
-        format_string(link_name_s, true, true, " 4.0K", " └──")
-    );
-    (r, r2)
-}
-
 // Check we don't recurse down an infinite symlink tree
 #[test]
 pub fn test_recursive_sym_link() {
@@ -283,70 +251,50 @@ pub fn test_recursive_sym_link() {
         .output();
     assert!(c.is_ok());
 
-    assert_cli::Assert::main_binary()
+    let a = format!(" ─┬ {}", dir_s);
-        .with_args(&[dir_s])
+    let b = format!("  └── {}", link_name_s);
-        .stdout()
-        .contains(recursive_sym_link_output(dir_s, link_name_s))
-        .unwrap();
-}
 
-#[cfg(target_os = "macos")]
-fn recursive_sym_link_output(dir: &str, link_name: &str) -> String {
-    format!(
-        "{}
-{}",
-        format_string(dir, true, true, " 4.0K", "─┬"),
-        format_string(link_name, true, true, " 4.0K", " └──",),
-    )
-}
-#[cfg(target_os = "linux")]
-fn recursive_sym_link_output(dir: &str, link_name: &str) -> String {
-    format!(
-        "{}
-{}",
-        format_string(dir, true, true, " 4.0K", "─┬"),
-        format_string(link_name, true, true, "   0B", " └──",),
-    )
-}
-
-// Check against directories and files whos names are substrings of each other
-#[test]
-#[cfg(target_os = "macos")]
-pub fn test_substring_of_names() {
     assert_cli::Assert::main_binary()
-        .with_args(&["src/test_dir2"])
+        .with_args(&["-p", dir_s])
         .stdout()
-        .contains(" ─┬ test_dir2")
+        .contains(a)
         .stdout()
-        .contains("  ├─┬ dir")
+        .contains(b)
-        .stdout()
-        .contains("  │ └── hello")
-        .stdout()
-        .contains("  ├── dir_name_clash")
-        .stdout()
-        .contains("  └─┬ dir_substring")
-        .stdout()
-        .contains("    └── hello")
         .unwrap();
 }
 
 // Check against directories and files whos names are substrings of each other
 #[test]
-#[cfg(target_os = "linux")]
 pub fn test_substring_of_names() {
     assert_cli::Assert::main_binary()
-        .with_args(&["src/test_dir2"])
+        .with_args(&["-c", "src/test_dir2"])
         .stdout()
-        .contains(" ─┬ test_dir2")
+        .is(no_substring_of_names_output())
-        .stdout()
-        .contains("  ├─┬ dir")
-        .stdout()
-        .contains("  │ └── hello")
-        .stdout()
-        .contains("  ├─┬ dir_substring")
-        .stdout()
-        .contains("  │ └── hello")
-        .stdout()
-        .contains("  └── dir_name_clash")
         .unwrap();
 }
+
+#[cfg(target_os = "linux")]
+fn no_substring_of_names_output() -> String {
+    "
+  24K ─┬ test_dir2
+ 8.0K  ├─┬ dir
+ 4.0K  │ └── hello
+ 8.0K  ├─┬ dir_substring
+ 4.0K  │ └── hello
+ 4.0K  └── dir_name_clash
+    "
+    .into()
+}
+
+#[cfg(target_os = "macos")]
+fn no_substring_of_names_output() -> String {
+    "
+  12K ─┬ test_dir2
+ 4.0K  ├─┬ dir
+ 4.0K  │ └── hello
+ 4.0K  ├── dir_name_clash
+ 4.0K  └─┬ dir_substring
+ 4.0K    └── hello
+    "
+    .into()
+}