relpath: move from getops to clap (#1939) (#1990)

* relpath: add tests

* relpath: move from getopts to clap

src/uu/relpath/Cargo.toml

@@ -15,7 +15,7 @@ edition = "2018"
 path = "src/relpath.rs"
 
 [dependencies]
-getopts = "0.2.18"
+clap = "2.33.3"
 uucore = { version=">=0.0.7", package="uucore", path="../../uucore", features=["fs"] }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }
 

src/uu/relpath/src/relpath.rs

@@ -10,62 +10,60 @@
 #[macro_use]
 extern crate uucore;
 
+use clap::{App, Arg};
 use std::env;
 use std::path::{Path, PathBuf};
 use uucore::fs::{canonicalize, CanonicalizeMode};
 
-static NAME: &str = "relpath";
 static VERSION: &str = env!("CARGO_PKG_VERSION");
+static ABOUT: &str = "Convert TO destination to the relative path from the FROM dir.
+If FROM path is omitted, current working dir will be used.";
+
+mod options {
+    pub const DIR: &str = "DIR";
+    pub const TO: &str = "TO";
+    pub const FROM: &str = "FROM";
+}
+
+fn get_usage() -> String {
+    format!("{} [-d DIR] TO [FROM]", executable!())
+}
 
 pub fn uumain(args: impl uucore::Args) -> i32 {
     let args = args.collect_str();
+    let usage = get_usage();
 
-    let mut opts = getopts::Options::new();
+    let matches = App::new(executable!())
+        .version(VERSION)
+        .about(ABOUT)
+        .usage(&usage[..])
+        .arg(
+            Arg::with_name(options::DIR)
+                .short("d")
+                .takes_value(true)
+                .help("If any of FROM and TO is not subpath of DIR, output absolute path instead of relative"),
+        )
+        .arg(
+            Arg::with_name(options::TO)
+                .required(true)
+                .takes_value(true),
+        )
+        .arg(
+            Arg::with_name(options::FROM)
+                .takes_value(true),
+        )
+        .get_matches_from(args);
 
-    opts.optflag("h", "help", "Show help and exit");
+    let to = Path::new(matches.value_of(options::TO).unwrap()).to_path_buf(); // required
-    opts.optflag("V", "version", "Show version and exit");
+    let from = match matches.value_of(options::FROM) {
-    opts.optopt(
+        Some(p) => Path::new(p).to_path_buf(),
-        "d",
+        None => env::current_dir().unwrap(),
-        "",
-        "If any of FROM and TO is not subpath of DIR, output absolute path instead of relative",
-        "DIR",
-    );
-
-    let matches = match opts.parse(&args[1..]) {
-        Ok(m) => m,
-        Err(f) => {
-            show_error!("{}", f);
-            show_usage(&opts);
-            return 1;
-        }
-    };
-
-    if matches.opt_present("V") {
-        version();
-        return 0;
-    }
-    if matches.opt_present("h") {
-        show_usage(&opts);
-        return 0;
-    }
-
-    if matches.free.is_empty() {
-        show_error!("Missing operand: TO");
-        println!("Try `{} --help` for more information.", NAME);
-        return 1;
-    }
-
-    let to = Path::new(&matches.free[0]);
-    let from = if matches.free.len() > 1 {
-        Path::new(&matches.free[1]).to_path_buf()
-    } else {
-        env::current_dir().unwrap()
     };
     let absto = canonicalize(to, CanonicalizeMode::Normal).unwrap();
     let absfrom = canonicalize(from, CanonicalizeMode::Normal).unwrap();
 
-    if matches.opt_present("d") {
+    if matches.is_present(options::DIR) {
-        let base = Path::new(&matches.opt_str("d").unwrap()).to_path_buf();
+        let base = Path::new(&matches.value_of(options::DIR).unwrap()).to_path_buf();
         let absbase = canonicalize(base, CanonicalizeMode::Normal).unwrap();
         if !absto.as_path().starts_with(absbase.as_path())
             || !absfrom.as_path().starts_with(absbase.as_path())
@@ -99,24 +97,3 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     println!("{}", result.display());
     0
 }
-
-fn version() {
-    println!("{} {}", NAME, VERSION)
-}
-
-fn show_usage(opts: &getopts::Options) {
-    version();
-    println!();
-    println!("Usage:");
-    println!("  {} [-d DIR] TO [FROM]", NAME);
-    println!("  {} -V|--version", NAME);
-    println!("  {} -h|--help", NAME);
-    println!();
-    print!(
-        "{}",
-        opts.usage(
-            "Convert TO destination to the relative path from the FROM dir.\n\
-             If FROM path is omitted, current working dir will be used."
-        )
-    );
-}

tests/by-util/test_relpath.rs

@@ -1 +1,165 @@
-// ToDO: add tests
+use crate::common::util::*;
+use std::borrow::Cow;
+use std::path::Path;
+
+struct TestCase<'a> {
+    from: &'a str,
+    to: &'a str,
+    expected: &'a str,
+}
+
+const TESTS: [TestCase; 10] = [
+    TestCase {
+        from: "A/B/C",
+        to: "A",
+        expected: "../..",
+    },
+    TestCase {
+        from: "A/B/C",
+        to: "A/B",
+        expected: "..",
+    },
+    TestCase {
+        from: "A/B/C",
+        to: "A/B/C",
+        expected: "",
+    },
+    TestCase {
+        from: "A/B/C",
+        to: "A/B/C/D",
+        expected: "D",
+    },
+    TestCase {
+        from: "A/B/C",
+        to: "A/B/C/D/E",
+        expected: "D/E",
+    },
+    TestCase {
+        from: "A/B/C",
+        to: "A/B/D",
+        expected: "../D",
+    },
+    TestCase {
+        from: "A/B/C",
+        to: "A/B/D/E",
+        expected: "../D/E",
+    },
+    TestCase {
+        from: "A/B/C",
+        to: "A/D",
+        expected: "../../D",
+    },
+    TestCase {
+        from: "A/B/C",
+        to: "D/E/F",
+        expected: "../../../D/E/F",
+    },
+    TestCase {
+        from: "A/B/C",
+        to: "A/D/E",
+        expected: "../../D/E",
+    },
+];
+
+fn convert_path<'a>(path: &'a str) -> Cow<'a, str> {
+    #[cfg(windows)]
+    return path.replace("/", "\\").into();
+    #[cfg(not(windows))]
+    return path.into();
+}
+
+#[test]
+fn test_relpath_with_from_no_d() {
+    for test in TESTS.iter() {
+        let scene = TestScenario::new(util_name!());
+        let at = &scene.fixtures;
+
+        let from: &str = &convert_path(test.from);
+        let to: &str = &convert_path(test.to);
+        let expected: &str = &convert_path(test.expected);
+
+        at.mkdir_all(to);
+        at.mkdir_all(from);
+
+        scene
+            .ucmd()
+            .arg(to)
+            .arg(from)
+            .succeeds()
+            .stdout_only(&format!("{}\n", expected));
+    }
+}
+
+#[test]
+fn test_relpath_with_from_with_d() {
+    for test in TESTS.iter() {
+        let scene = TestScenario::new(util_name!());
+        let at = &scene.fixtures;
+
+        let from: &str = &convert_path(test.from);
+        let to: &str = &convert_path(test.to);
+        let pwd = at.as_string();
+        at.mkdir_all(to);
+        at.mkdir_all(from);
+
+        // d is part of subpath -> expect relative path
+        let mut result = scene
+            .ucmd()
+            .arg(to)
+            .arg(from)
+            .arg(&format!("-d{}", pwd))
+            .run();
+        assert!(result.success);
+        // relax rules for windows test environment
+        #[cfg(not(windows))]
+        assert!(Path::new(&result.stdout).is_relative());
+
+        // d is not part of subpath -> expect absolut path
+        result = scene.ucmd().arg(to).arg(from).arg("-dnon_existing").run();
+        assert!(result.success);
+        assert!(Path::new(&result.stdout).is_absolute());
+    }
+}
+
+#[test]
+fn test_relpath_no_from_no_d() {
+    for test in TESTS.iter() {
+        let scene = TestScenario::new(util_name!());
+        let at = &scene.fixtures;
+
+        let to: &str = &convert_path(test.to);
+        at.mkdir_all(to);
+
+        let result = scene.ucmd().arg(to).run();
+        assert!(result.success);
+        #[cfg(not(windows))]
+        assert_eq!(result.stdout, format!("{}\n", to));
+        // relax rules for windows test environment
+        #[cfg(windows)]
+        assert!(result.stdout.ends_with(&format!("{}\n", to)));
+    }
+}
+
+#[test]
+fn test_relpath_no_from_with_d() {
+    for test in TESTS.iter() {
+        let scene = TestScenario::new(util_name!());
+        let at = &scene.fixtures;
+
+        let to: &str = &convert_path(test.to);
+        let pwd = at.as_string();
+        at.mkdir_all(to);
+
+        // d is part of subpath -> expect relative path
+        let mut result = scene.ucmd().arg(to).arg(&format!("-d{}", pwd)).run();
+        assert!(result.success);
+        // relax rules for windows test environment
+        #[cfg(not(windows))]
+        assert!(Path::new(&result.stdout).is_relative());
+
+        // d is not part of subpath -> expect absolut path
+        result = scene.ucmd().arg(to).arg("-dnon_existing").run();
+        assert!(result.success);
+        assert!(Path::new(&result.stdout).is_absolute());
+    }
+}