Merge pull request #9245 from cakebaker/test_ot_nt_with_non_existing_file

test: existing file is newer than non-existing file

Author: sylvestre

Cargo.lock

@@ -19,10 +19,13 @@ path = "src/test.rs"
 
 [dependencies]
 clap = { workspace = true }
+fluent = { workspace = true }
 libc = { workspace = true }
-uucore = { workspace = true, features = ["process"] }
 thiserror = { workspace = true }
-fluent = { workspace = true }
+uucore = { workspace = true, features = ["process"] }
+
+[dev-dependencies]
+tempfile = { workspace = true }
 
 [[bin]]
 name = "test"

src/uu/test/Cargo.toml

@@ -205,24 +205,26 @@ fn integers(a: &OsStr, b: &OsStr, op: &OsStr) -> ParseResult<bool> {
 
 /// Operations to compare files metadata
 /// `a` is the left hand side
-/// `b` is the left hand side
+/// `b` is the right hand side
 /// `op` the operation (ex: -ef, -nt, etc)
 fn files(a: &OsStr, b: &OsStr, op: &OsStr) -> ParseResult<bool> {
-    // Don't manage the error. GNU doesn't show error when doing
-    // test foo -nt bar
-    let (Ok(f_a), Ok(f_b)) = (fs::metadata(a), fs::metadata(b)) else {
-        return Ok(false);
-    };
+    let f_a = fs::metadata(a);
+    let f_b = fs::metadata(b);
 
-    Ok(match op.to_str() {
+    let result = match (op.to_str(), f_a, f_b) {
         #[cfg(unix)]
-        Some("-ef") => f_a.ino() == f_b.ino() && f_a.dev() == f_b.dev(),
+        (Some("-ef"), Ok(f_a), Ok(f_b)) => f_a.ino() == f_b.ino() && f_a.dev() == f_b.dev(),
         #[cfg(not(unix))]
-        Some("-ef") => unimplemented!(),
-        Some("-nt") => f_a.modified().unwrap() > f_b.modified().unwrap(),
-        Some("-ot") => f_a.modified().unwrap() < f_b.modified().unwrap(),
-        _ => return Err(ParseError::UnknownOperator(op.quote().to_string())),
-    })
+        (Some("-ef"), Ok(_), Ok(_)) => unimplemented!(),
+        (Some("-nt"), Ok(f_a), Ok(f_b)) => f_a.modified().unwrap() > f_b.modified().unwrap(),
+        (Some("-nt"), Ok(_), _) => true,
+        (Some("-ot"), Ok(f_a), Ok(f_b)) => f_a.modified().unwrap() < f_b.modified().unwrap(),
+        (Some("-ot"), _, Ok(_)) => true,
+        (Some("-ef" | "-nt" | "-ot"), _, _) => false,
+        (_, _, _) => return Err(ParseError::UnknownOperator(op.quote().to_string())),
+    };
+
+    Ok(result)
 }
 
 fn isatty(fd: &OsStr) -> ParseResult<bool> {
@@ -347,8 +349,81 @@ fn path(path: &OsStr, condition: &PathCondition) -> bool {
 
 #[cfg(test)]
 mod tests {
-    use super::integers;
-    use std::ffi::OsStr;
+    use super::*;
+    use std::{ffi::OsStr, time::UNIX_EPOCH};
+    use tempfile::NamedTempFile;
+
+    #[test]
+    fn test_files_with_unknown_op() {
+        let a = NamedTempFile::new().unwrap();
+        let b = NamedTempFile::new().unwrap();
+        let a = OsStr::new(a.path());
+        let b = OsStr::new(b.path());
+        let op = OsStr::new("unknown_op");
+
+        assert!(files(a, b, op).is_err());
+    }
+
+    #[test]
+    #[cfg(unix)]
+    fn test_files_with_ef_op() {
+        let a = NamedTempFile::new().unwrap();
+        let b = NamedTempFile::new().unwrap();
+        let a = OsStr::new(a.path());
+        let b = OsStr::new(b.path());
+        let op = OsStr::new("-ef");
+
+        assert!(files(a, a, op).unwrap());
+        assert!(!files(a, b, op).unwrap());
+        assert!(!files(b, a, op).unwrap());
+
+        let existing_file = a;
+        let non_existing_file = OsStr::new("non_existing_file");
+
+        assert!(!files(existing_file, non_existing_file, op).unwrap());
+        assert!(!files(non_existing_file, existing_file, op).unwrap());
+        assert!(!files(non_existing_file, non_existing_file, op).unwrap());
+    }
+
+    #[test]
+    fn test_files_with_nt_op() {
+        let older_file = NamedTempFile::new().unwrap();
+        older_file.as_file().set_modified(UNIX_EPOCH).unwrap();
+        let older_file = OsStr::new(older_file.path());
+        let newer_file = NamedTempFile::new().unwrap();
+        let newer_file = OsStr::new(newer_file.path());
+        let op = OsStr::new("-nt");
+
+        assert!(files(newer_file, older_file, op).unwrap());
+        assert!(!files(older_file, newer_file, op).unwrap());
+
+        let existing_file = newer_file;
+        let non_existing_file = OsStr::new("non_existing_file");
+
+        assert!(files(existing_file, non_existing_file, op).unwrap());
+        assert!(!files(non_existing_file, existing_file, op).unwrap());
+        assert!(!files(non_existing_file, non_existing_file, op).unwrap());
+    }
+
+    #[test]
+    fn test_files_with_ot_op() {
+        let older_file = NamedTempFile::new().unwrap();
+        older_file.as_file().set_modified(UNIX_EPOCH).unwrap();
+        let older_file = OsStr::new(older_file.path());
+        let newer_file = NamedTempFile::new().unwrap();
+        let newer_file = OsStr::new(newer_file.path());
+        let op = OsStr::new("-ot");
+
+        assert!(!files(newer_file, older_file, op).unwrap());
+        assert!(files(older_file, newer_file, op).unwrap());
+
+        let existing_file = newer_file;
+        let non_existing_file = OsStr::new("non_existing_file");
+
+        assert!(!files(existing_file, non_existing_file, op).unwrap());
+        assert!(files(non_existing_file, existing_file, op).unwrap());
+        assert!(!files(non_existing_file, non_existing_file, op).unwrap());
+    }
 
     #[test]
     fn test_integer_op() {

src/uu/test/src/test.rs

@@ -5,10 +5,8 @@
 
 // spell-checker:ignore (words) egid euid pseudofloat
 
-use uutests::at_and_ucmd;
-use uutests::new_ucmd;
 use uutests::util::TestScenario;
-use uutests::util_name;
+use uutests::{at_and_ucmd, new_ucmd, util_name};
 
 #[test]
 fn test_empty_test_equivalent_to_false() {
@@ -337,14 +335,26 @@ fn test_file_is_newer_than_and_older_than_itself() {
 }
 
 #[test]
-fn test_non_existing_files() {
-    let scenario = TestScenario::new(util_name!());
+fn test_file_is_newer_than_non_existing_file() {
+    new_ucmd!()
+        .args(&["non_existing_file", "-nt", "regular_file"])
+        .fails_with_code(1)
+        .no_output();
 
-    let result = scenario
-        .ucmd()
-        .args(&["newer_file", "-nt", "regular_file"])
-        .fails_with_code(1);
-    assert!(result.stderr().is_empty());
+    new_ucmd!()
+        .args(&["regular_file", "-nt", "non_existing_file"])
+        .succeeds()
+        .no_output();
+
+    new_ucmd!()
+        .args(&["non_existing_file", "-ot", "regular_file"])
+        .succeeds()
+        .no_output();
+
+    new_ucmd!()
+        .args(&["regular_file", "-ot", "non_existing_file"])
+        .fails_with_code(1)
+        .no_output();
 }
 
 #[test]