Merge pull request #1654 from sylvestre/ls-exit

ls - improve the error management + add tests

src/uu/ls/src/ls.rs

@@ -168,11 +168,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         )
         .parse(args);
 
-    list(matches);
-    0
+    list(matches)
 }
 
-fn list(options: getopts::Matches) {
+fn list(options: getopts::Matches) -> i32 {
     let locs: Vec<String> = if options.free.is_empty() {
         vec![String::from(".")]
     } else {
@@ -181,8 +180,16 @@ fn list(options: getopts::Matches) {
 
     let mut files = Vec::<PathBuf>::new();
     let mut dirs = Vec::<PathBuf>::new();
+    let mut has_failed = false;
     for loc in locs {
         let p = PathBuf::from(&loc);
+        if !p.exists() {
+            show_error!("'{}': {}", &loc, "No such file or directory");
+            // We found an error, the return code of ls should not be 0
+            // And no need to continue the execution
+            has_failed = true;
+            continue;
+        }
         let mut dir = false;
 
         if p.is_dir() && !options.opt_present("d") {
@@ -211,6 +218,11 @@ fn list(options: getopts::Matches) {
         }
         enter_directory(&dir, &options);
     }
+    if has_failed {
+        1
+    } else {
+        0
+    }
 }
 
 #[cfg(any(unix, target_os = "redox"))]
@@ -355,7 +367,7 @@ fn display_items(items: &[PathBuf], strip: Option<&Path>, options: &getopts::Mat
                 match md {
                     Err(e) => {
                         let filename = get_file_name(i, strip);
-                        show_error!("{}: {}", filename, e);
+                        show_error!("'{}': {}", filename, e);
                         None
                     }
                     Ok(md) => Some(display_file_name(&i, strip, &md, options)),

tests/by-util/test_ls.rs

@@ -11,6 +11,70 @@ fn test_ls_ls_i() {
     new_ucmd!().arg("-il").succeeds();
 }
 
+#[test]
+fn test_ls_non_existing() {
+    new_ucmd!().arg("doesntexist").fails();
+}
+
+#[test]
+fn test_ls_files_dirs() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.mkdir("a");
+    at.mkdir("a/b");
+    at.mkdir("a/b/c");
+    at.mkdir("z");
+    at.touch(&at.plus_as_string("a/a"));
+    at.touch(&at.plus_as_string("a/b/b"));
+
+    scene.ucmd().arg("a").succeeds();
+    scene.ucmd().arg("a/a").succeeds();
+    scene.ucmd().arg("a").arg("z").succeeds();
+
+    let result = scene.ucmd().arg("doesntexist").fails();
+    // Doesn't exist
+    assert!(result
+        .stderr
+        .contains("error: 'doesntexist': No such file or directory"));
+
+    let result = scene.ucmd().arg("a").arg("doesntexist").fails();
+    // One exists, the other doesn't
+    assert!(result
+        .stderr
+        .contains("error: 'doesntexist': No such file or directory"));
+    assert!(result.stdout.contains("a:"));
+}
+
+#[test]
+fn test_ls_recursive() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.mkdir("a");
+    at.mkdir("a/b");
+    at.mkdir("a/b/c");
+    at.mkdir("z");
+    at.touch(&at.plus_as_string("a/a"));
+    at.touch(&at.plus_as_string("a/b/b"));
+
+    scene.ucmd().arg("a").succeeds();
+    scene.ucmd().arg("a/a").succeeds();
+    let result = scene
+        .ucmd()
+        .arg("--color=never")
+        .arg("-R")
+        .arg("a")
+        .arg("z")
+        .succeeds();
+
+    println!("stderr = {:?}", result.stderr);
+    println!("stdout = {:?}", result.stdout);
+    if cfg!(target_os = "windows") {
+        assert!(result.stdout.contains("a\\b:\nb"));
+    } else {
+        assert!(result.stdout.contains("a/b:\nb"));
+    }
+}
+
 #[test]
 fn test_ls_ls_color() {
     new_ucmd!().arg("--color").succeeds();