Merge pull request #947 from irobert91/imag-link/rewrite-tests

imag-link/rewrite-tests WIP

Makefile

@@ -76,6 +76,8 @@ $(TARGETS): %: .FORCE
 	@$(CARGO) build --manifest-path ./$@/Cargo.toml
 
 $(BIN_TARGET_TESTS): %-test: % .FORCE
+	@$(ECHO) "\t[CARGO  ][TEST]: \t$@"
+	@$(CARGO) test --manifest-path ./$(subst -test,,$@)/Cargo.toml
 	@$(ECHO) "\t[BINTEST]:\t$@"
 	if [ -f $(subst -test,,$@)/tests/Makefile ]; then \
 		$(MAKE) -C $(subst -test,,$@)/tests || exit 1;\

imag-link/Cargo.toml

@@ -19,6 +19,7 @@ clap = ">=2.17"
 log = "0.3"
 version = "2.0.1"
 toml = "^0.4"
+toml-query = "0.1"
 url = "1.2"
 
 [dependencies.libimagstore]

imag-link/src/main.rs

@@ -36,6 +36,7 @@
 extern crate clap;
 extern crate semver;
 extern crate toml;
+extern crate toml_query;
 extern crate url;
 #[macro_use] extern crate version;
 
@@ -334,3 +335,165 @@ fn list_links_for_entry(store: &Store, entry: &mut FileLockEntry) {
         .ok();
 }
 
+#[cfg(test)]
+mod tests {
+    use handle_internal_linking;
+
+    use std::path::PathBuf;
+    use std::ffi::OsStr;
+
+    use clap::{App, ArgMatches};
+    use toml::value::Value;
+    use toml_query::read::TomlValueReadExt;
+    use toml_query::error::Result as TomlQueryResult;
+
+    use libimagrt::spec::CliSpec;
+    use libimagrt::runtime::Runtime;
+    use libimagrt::error::RuntimeError;
+    use libimagrt::configuration::{Configuration, InternalConfiguration};
+
+    use libimagstore::storeid::StoreId;
+    use libimagstore::store::{Result as StoreResult, FileLockEntry};
+
+    static DEFAULT_ENTRY: &'static str = "\
+---\
+[imag]\
+links = []\
+version = \"0.3.0\"\
+---";
+
+    #[derive(Clone)]
+    struct MockLinkApp<'a> {
+        args: Vec<&'static str>,
+        inner: App<'a, 'a>,
+    }
+
+    impl<'a> MockLinkApp<'a> {
+        fn new(args: Vec<&'static str>) -> Self {
+            MockLinkApp {
+                args: args,
+                inner: ::build_ui(Runtime::get_default_cli_builder("imag-link",
+                                                                   "0.3.0",
+                                                                   "Link entries test")),
+            }
+        }
+    }
+
+    impl<'a> CliSpec<'a> for MockLinkApp<'a> {
+        fn name(&self) -> &str {
+            self.inner.get_name()
+        }
+
+        fn matches(self) -> ArgMatches<'a> {
+            self.inner.get_matches_from(self.args)
+        }
+    }
+
+    impl<'a> InternalConfiguration for MockLinkApp<'a> {
+        fn enable_logging(&self) -> bool {
+            false
+        }
+
+        fn use_inmemory_fs(&self) -> bool {
+            true
+        }
+    }
+
+    fn generate_test_config() -> Option<Configuration> {
+        ::toml::de::from_str("[store]\nimplicit-create=true")
+                    .map(Configuration::with_value)
+                    .ok()
+    }
+
+    fn generate_test_runtime<'a>(mut args: Vec<&'static str>) -> Result<Runtime<'a>, RuntimeError> {
+        let mut cli_args = vec!["imag-link", "--rtp", "/tmp"];
+
+        cli_args.append(&mut args);
+
+        let cli_app = MockLinkApp::new(cli_args);
+        Runtime::with_configuration(cli_app, generate_test_config())
+    }
+
+    fn create_test_entry<'a, S: AsRef<OsStr>>(rt: &'a Runtime, name: S) -> StoreResult<StoreId> {
+        let mut path = PathBuf::new();
+        path.set_file_name(name);
+
+        let id = StoreId::new_baseless(path)?;
+        let mut entry = rt.store().create(id.clone())?;
+        entry.get_content_mut().push_str(DEFAULT_ENTRY);
+
+        Ok(id)
+    }
+
+    fn get_entry_links<'a>(entry: &'a FileLockEntry<'a>) -> TomlQueryResult<&'a Value> {
+        entry.get_header().read(&"imag.links".to_owned())
+    }
+
+    fn links_toml_value<'a, I: IntoIterator<Item = &'static str>>(links: I) -> Value {
+        Value::Array(links
+                         .into_iter()
+                         .map(|s| Value::String(s.to_owned()))
+                         .collect())
+    }
+
+    #[test]
+    fn test_link_modificates() {
+        let rt = generate_test_runtime(vec!["internal", "add", "--from", "test1", "--to", "test2"])
+            .unwrap();
+
+        let test_id1 = create_test_entry(&rt, "test1").unwrap();
+        let test_id2 = create_test_entry(&rt, "test2").unwrap();
+
+        handle_internal_linking(&rt);
+
+        let test_entry1 = rt.store().get(test_id1).unwrap().unwrap();
+        let test_links1 = get_entry_links(&test_entry1).unwrap();
+
+        let test_entry2 = rt.store().get(test_id2).unwrap().unwrap();
+        let test_links2 = get_entry_links(&test_entry2).unwrap();
+
+        assert_ne!(*test_links1, links_toml_value(vec![]));
+        assert_ne!(*test_links2, links_toml_value(vec![]));
+    }
+
+    #[test]
+    fn test_linking_links() {
+        let rt = generate_test_runtime(vec!["internal", "add", "--from", "test1", "--to", "test2"])
+            .unwrap();
+
+        let test_id1 = create_test_entry(&rt, "test1").unwrap();
+        let test_id2 = create_test_entry(&rt, "test2").unwrap();
+
+        handle_internal_linking(&rt);
+
+        let test_entry1 = rt.store().get(test_id1).unwrap().unwrap();
+        let test_links1 = get_entry_links(&test_entry1).unwrap();
+
+        let test_entry2 = rt.store().get(test_id2).unwrap().unwrap();
+        let test_links2 = get_entry_links(&test_entry2).unwrap();
+
+        assert_eq!(*test_links1, links_toml_value(vec!["test2"]));
+        assert_eq!(*test_links2, links_toml_value(vec!["test1"]));
+    }
+
+    #[test]
+    fn test_multilinking() {
+        let rt = generate_test_runtime(vec!["internal", "add", "--from", "test1", "--to", "test2"])
+            .unwrap();
+
+        let test_id1 = create_test_entry(&rt, "test1").unwrap();
+        let test_id2 = create_test_entry(&rt, "test2").unwrap();
+
+        handle_internal_linking(&rt);
+        handle_internal_linking(&rt);
+
+        let test_entry1 = rt.store().get(test_id1).unwrap().unwrap();
+        let test_links1 = get_entry_links(&test_entry1).unwrap();
+
+        let test_entry2 = rt.store().get(test_id2).unwrap().unwrap();
+        let test_links2 = get_entry_links(&test_entry2).unwrap();
+
+        assert_eq!(*test_links1, links_toml_value(vec!["test2"]));
+        assert_eq!(*test_links2, links_toml_value(vec!["test1"]));
+    }
+}

imag-link/tests/Makefile

@@ -1,13 +0,0 @@
-ECHO=$(shell which echo) -e
-TARGETS=$(shell find -name "*test.sh" -type f)
-BASH=$(shell which bash)
-
-all: $(TARGETS)
-	@$(ECHO) $(TARGETS)
-
-$(TARGETS): %: .FORCE
-	@$(ECHO) "\t[BASH   ]:\t$@"
-	@$(BASH) $@
-
-.FORCE:
-

imag-link/tests/link-test.sh

@@ -1,100 +0,0 @@
-#!/usr/bin/env bash
-
-source $(dirname ${BASH_SOURCE[0]})/../../tests/utils.sh
-source $(dirname ${BASH_SOURCE[0]})/utils.sh
-
-default_entry() {
-    cat <<EOS
----
-[imag]
-links = []
-version = "0.3.0"
----
-
-EOS
-}
-
-entry_linked_to() {
-    cat <<EOS
----
-[imag]
-links = [$1]
-version = "0.3.0"
----
-
-EOS
-}
-
-mktestentry() {
-    mkdir -p ${STORE}
-    default_entry > ${STORE}/$1
-}
-
-test_link_modificates() {
-    mktestentry "test"
-    mktestentry "test2"
-
-    imag-link internal add --from "test" --to "test2"
-
-    if [ "$(default_entry)" == "$(cat_entry 'test')" ] ||
-       [ "$(default_entry)" == "$(cat_entry 'test2')" ]
-    then
-        err "Entry was unmodified after linking"
-        return 1;
-    fi
-}
-
-test_linking_links() {
-    mktestentry "test"
-    mktestentry "test2"
-
-    imag-link internal add --from "test" --to "test2"
-
-    if [[ "$(entry_linked_to '"test"')" != "$(cat_entry 'test2')" ]];
-    then
-        err "Linking to 'test' didn't succeed for 'test2'"
-        err "\n$(cat_entry 'test2')\n"
-    fi
-
-    if [[ "$(entry_linked_to '"test2"')" != "$(cat_entry 'test')" ]];
-    then
-        err "Linking to 'test2' didn't succeed for 'test'"
-        err "\n$(cat_entry 'test')\n"
-    fi
-}
-
-test_multilinking() {
-    mktestentry "test"
-    mktestentry "test2"
-
-    imag-link internal add --from "test" --to "test2" || {
-        err "Linking failed"; return 1
-    }
-
-    imag-link internal add --from "test" --to "test2" || {
-        err "Linking again failed"; return 1
-    }
-
-    local linked_to_test="$(entry_linked_to '"test"' | sha1sum)"
-    local linked_to_test2="$(entry_linked_to '"test2"' | sha1sum)"
-    local t2="$(cat_entry 'test2' | sha1sum)"
-    local t1="$(cat_entry 'test' | sha1sum)"
-
-    if [ "${linked_to_test}" != "${t2}" ];
-    then
-        err "Linking twice to 'test' didn't result in the expected output for 'test2'"
-        err "\n$(cat_entry 'test2')\n"
-    fi
-
-    if [ "${linked_to_test2}" != "${t1}" ];
-    then
-        err "Linking twice to 'test2' didn't result in the expected output for 'test'"
-        err "\n$(cat_entry 'test')\n"
-    fi
-}
-
-invoke_tests            \
-    test_link_modificates \
-    test_linking_links \
-    test_multilinking
-

imag-link/tests/utils.sh

@@ -1,6 +0,0 @@
-source $(dirname ${BASH_SOURCE[0]})/../../tests/utils.sh
-
-imag-link() {
-    imag-call-binary "$(dirname ${BASH_SOURCE[0]})/../../target/debug/" imag-link $*
-}
-

libimagrt/src/configuration.rs

@@ -22,9 +22,7 @@ use std::result::Result as RResult;
 use std::ops::Deref;
 
 use toml::Value;
-
+use clap::App;
-use error::RuntimeErrorKind as REK;
-use libimagerror::into::IntoError;
 
 generate_error_module!(
     generate_error_types!(ConfigError, ConfigErrorKind,
@@ -92,6 +90,16 @@ impl Configuration {
         })
     }
 
+    /// Get a new configuration object built from the given toml value.
+    pub fn with_value(value: Value) -> Configuration {
+        Configuration{
+            verbosity: get_verbosity(&value),
+            editor: get_editor(&value),
+            editor_opts: get_editor_opts(&value),
+            config: value,
+        }
+    }
+
     /// Get the Editor setting from the configuration
     pub fn editor(&self) -> Option<&String> {
         self.editor.as_ref()
@@ -219,12 +227,14 @@ fn fetch_config(rtp: &PathBuf) -> Result<Value> {
     use std::env;
     use std::fs::File;
     use std::io::Read;
+    use std::io::Write;
+    use std::io::stderr;
 
     use xdg_basedir;
     use itertools::Itertools;
 
     use libimagutil::variants::generate_variants as gen_vars;
-    use self::error::MapErrInto;
+    use libimagerror::trace::trace_error;
 
     let variants = vec!["config", "config.toml", "imagrc", "imagrc.toml"];
     let modifier = |base: &PathBuf, v: &'static str| {
@@ -245,8 +255,7 @@ fn fetch_config(rtp: &PathBuf) -> Result<Value> {
     ].iter()
         .flatten()
         .filter(|path| path.exists() && path.is_file())
-        .filter_map(|path| if path.exists() && path.is_file() {
+        .map(|path| {
-            debug!("Reading {:?}", path);
             let content = {
                 let mut s = String::new();
                 let f = File::open(path);
@@ -258,19 +267,29 @@ fn fetch_config(rtp: &PathBuf) -> Result<Value> {
                 s
             };
 
-            trace!("Contents of config file: \n---\n{}\n---", content);
+            match ::toml::de::from_str(&content[..]) {
-
+                Ok(res) => res,
-            let toml = ::toml::de::from_str(&content[..])
+                Err(e) => {
-                .map_err_into(ConfigErrorKind::TOMLParserError)
+                    write!(stderr(), "Config file parser error:").ok();
-                .map_err(Box::new)
+                    trace_error(&e);
-                .map_err(|e| REK::Instantiate.into_error_with_cause(e));
-
-            Some(toml)
-        } else {
                     None
+                }
+            }
         })
-        .filter(|loaded| loaded.is_ok())
+        .filter(|loaded| loaded.is_some())
-        .map(|inner| Value::Table(inner.unwrap()))
         .nth(0)
+        .map(|inner| Value::Table(inner.unwrap()))
         .ok_or(ConfigErrorKind::NoConfigFileFound.into())
 }
+
+pub trait InternalConfiguration {
+    fn enable_logging(&self) -> bool {
+        true
+    }
+
+    fn use_inmemory_fs(&self) -> bool {
+        false
+    }
+}
+
+impl<'a> InternalConfiguration for App<'a, 'a> {}

libimagrt/src/lib.rs

@@ -53,4 +53,5 @@ pub mod configuration;
 pub mod logger;
 pub mod runtime;
 pub mod setup;
+pub mod spec;
 

libimagrt/src/runtime.rs

@@ -29,13 +29,15 @@ use clap::{Arg, ArgMatches};
 use log;
 use log::LogLevelFilter;
 
-use configuration::Configuration;
+use configuration::{Configuration, InternalConfiguration};
 use error::RuntimeError;
 use error::RuntimeErrorKind;
 use error::MapErrInto;
 use logger::ImagLogger;
 
 use libimagstore::store::Store;
+use libimagstore::file_abstraction::InMemoryFileAbstraction;
+use spec::CliSpec;
 
 /// The Runtime object
 ///
@@ -54,60 +56,25 @@ impl<'a> Runtime<'a> {
     /// in $HOME/.imag/config, $XDG_CONFIG_DIR/imag/config or from env("$IMAG_CONFIG")
     /// and builds the Runtime object with it.
     ///
-    /// The cli_spec object should be initially build with the ::get_default_cli_builder() function.
+    /// The cli_app object should be initially build with the ::get_default_cli_builder() function.
-    pub fn new(mut cli_spec: App<'a, 'a>) -> Result<Runtime<'a>, RuntimeError> {
+    pub fn new<C>(cli_app: C) -> Result<Runtime<'a>, RuntimeError>
-        use std::env;
+        where C: Clone + CliSpec<'a> + InternalConfiguration
-        use std::io::stdout;
+    {
-
-        use clap::Shell;
-
         use libimagerror::trace::trace_error;
         use libimagerror::into::IntoError;
 
         use configuration::error::ConfigErrorKind;
 
-        let matches = cli_spec.clone().get_matches();
+        let matches = cli_app.clone().matches();
 
-        let is_debugging = matches.is_present("debugging");
+        let rtp = get_rtp_match(&matches);
-        let is_verbose   = matches.is_present("verbosity");
-        let colored      = !matches.is_present("no-color-output");
 
-        Runtime::init_logger(is_debugging, is_verbose, colored);
+        let configpath = matches.value_of(Runtime::arg_config_name())
-
-        match matches.value_of(Runtime::arg_generate_compl()) {
-            Some(shell) => {
-                debug!("Generating shell completion script, writing to stdout");
-                let shell   = shell.parse::<Shell>().unwrap(); // clap has our back here.
-                let appname = String::from(cli_spec.get_name());
-                cli_spec.gen_completions_to(appname, shell, &mut stdout());
-            },
-            _ => debug!("Not generating shell completion script"),
-        }
-
-        let rtp : PathBuf = matches.value_of("runtimepath")
-            .map_or_else(|| {
-                env::var("HOME")
-                    .map(PathBuf::from)
-                    .map(|mut p| { p.push(".imag"); p})
-                    .unwrap_or_else(|_| {
-                        panic!("You seem to be $HOME-less. Please get a $HOME before using this software. We are sorry for you and hope you have some accommodation anyways.");
-                    })
-            }, PathBuf::from);
-        let storepath = matches.value_of("storepath")
-                                .map_or_else(|| {
-                                    let mut spath = rtp.clone();
-                                    spath.push("store");
-                                    spath
-                                }, PathBuf::from);
-
-        let configpath = matches.value_of("config")
                                 .map_or_else(|| rtp.clone(), PathBuf::from);
 
-        debug!("RTP path    = {:?}", rtp);
-        debug!("Store path  = {:?}", storepath);
         debug!("Config path = {:?}", configpath);
 
-        let cfg = match Configuration::new(&configpath) {
+        let config = match Configuration::new(&configpath) {
             Err(e) => if e.err_type() != ConfigErrorKind::NoConfigFileFound {
                 return Err(RuntimeErrorKind::Instantiate.into_error_with_cause(Box::new(e)));
             } else {
@@ -116,32 +83,91 @@ impl<'a> Runtime<'a> {
                 None
             },
 
-            Ok(mut cfg) => {
+            Ok(mut config) => {
-                if let Err(e) = cfg.override_config(get_override_specs(&matches)) {
+                if let Err(e) = config.override_config(get_override_specs(&matches)) {
                     error!("Could not apply config overrides");
                     trace_error(&e);
 
                     // TODO: continue question (interactive)
                 }
 
-                Some(cfg)
+                Some(config)
             }
         };
 
-        let store_config = match cfg {
+        Runtime::_new(cli_app, matches, config)
+    }
+
+    /// Builds the Runtime object using the given `config`.
+    pub fn with_configuration<C>(cli_app: C, config: Option<Configuration>)
+                                 -> Result<Runtime<'a>, RuntimeError>
+        where C: Clone + CliSpec<'a> + InternalConfiguration
+    {
+        let matches = cli_app.clone().matches();
+        Runtime::_new(cli_app, matches, config)
+    }
+
+    fn _new<C>(mut cli_app: C, matches: ArgMatches<'a>, config: Option<Configuration>)
+               -> Result<Runtime<'a>, RuntimeError>
+    where C: Clone + CliSpec<'a> + InternalConfiguration
+    {
+        use std::io::stdout;
+
+        use clap::Shell;
+
+        let is_debugging = matches.is_present(Runtime::arg_debugging_name());
+
+        if cli_app.enable_logging() {
+            let is_verbose = matches.is_present(Runtime::arg_verbosity_name());
+            let colored    = !matches.is_present(Runtime::arg_no_color_output_name());
+
+            Runtime::init_logger(is_debugging, is_verbose, colored);
+        }
+
+        match matches.value_of(Runtime::arg_generate_compl()) {
+            Some(shell) => {
+                debug!("Generating shell completion script, writing to stdout");
+                let shell   = shell.parse::<Shell>().unwrap(); // clap has our back here.
+                let appname = String::from(cli_app.name());
+                cli_app.completions(appname, shell, &mut stdout());
+            },
+            _ => debug!("Not generating shell completion script"),
+        }
+
+        let rtp = get_rtp_match(&matches);
+
+        let storepath = matches.value_of(Runtime::arg_storepath_name())
+                                .map_or_else(|| {
+                                    let mut spath = rtp.clone();
+                                    spath.push("store");
+                                    spath
+                                }, PathBuf::from);
+
+        debug!("RTP path    = {:?}", rtp);
+        debug!("Store path  = {:?}", storepath);
+
+        let store_config = match config {
             Some(ref c) => c.store_config().cloned(),
             None        => None,
         };
 
         if is_debugging {
-            write!(stderr(), "Config: {:?}\n", cfg).ok();
+            write!(stderr(), "Config: {:?}\n", config).ok();
             write!(stderr(), "Store-config: {:?}\n", store_config).ok();
         }
 
-        Store::new(storepath.clone(), store_config).map(|store| {
+        let store_result = if cli_app.use_inmemory_fs() {
+            Store::new_with_backend(storepath,
+                                    store_config,
+                                    Box::new(InMemoryFileAbstraction::new()))
+        } else {
+            Store::new(storepath, store_config)
+        };
+
+        store_result.map(|store| {
             Runtime {
                 cli_matches: matches,
-                configuration: cfg,
+                configuration: config,
                 rtp: rtp,
                 store: store,
             }
@@ -403,6 +429,22 @@ impl<'a> Runtime<'a> {
     }
 }
 
+fn get_rtp_match<'a>(matches: &ArgMatches<'a>) -> PathBuf {
+    use std::env;
+
+    matches.value_of(Runtime::arg_runtimepath_name())
+        .map_or_else(|| {
+            env::var("HOME")
+                .map(PathBuf::from)
+                .map(|mut p| { p.push(".imag"); p })
+                .unwrap_or_else(|_| {
+                    panic!("You seem to be $HOME-less. Please get a $HOME before using this \
+                            software. We are sorry for you and hope you have some \
+                            accommodation anyways.");
+                })
+        }, PathBuf::from)
+}
+
 fn get_override_specs(matches: &ArgMatches) -> Vec<String> {
     matches
         .values_of("config-override")

libimagrt/src/spec.rs

@@ -0,0 +1,30 @@
+use std::io::Write;
+
+use clap::{App, ArgMatches, Shell};
+
+/// An abstraction over `clap::App` functionality needed for initializing `Runtime`. Different
+/// implementations can be used for testing `imag` binaries without running them as separate
+/// processes.
+pub trait CliSpec<'a> {
+    fn name(&self) -> &str;
+    fn matches(self) -> ArgMatches<'a>;
+    fn completions<W: Write, S: Into<String>>(&mut self, _: S, _: Shell, _: &mut W) {}
+}
+
+impl<'a> CliSpec<'a> for App<'a, 'a> {
+    fn name(&self) -> &str {
+        self.get_name()
+    }
+
+    fn matches(self) -> ArgMatches<'a> {
+        self.get_matches()
+    }
+
+    fn completions<W: Write, S: Into<String>>(&mut self,
+                                              bin_name: S,
+                                              for_shell: Shell,
+                                              buf: &mut W) {
+
+        self.gen_completions_to(bin_name, for_shell, buf);
+    }
+}