From 7b9e43ab0602e222b9eea2758c9e6e84a6be67f8 Mon Sep 17 00:00:00 2001
From: Federico Terzi <federicoterzi96@gmail.com>
Date: Sat, 6 Mar 2021 15:26:34 +0100
Subject: [PATCH] Improve testing coverage of config module

---
 Cargo.lock                                    |   7 +
 espanso-config/Cargo.toml                     |   2 +-
 espanso-config/src/config/path.rs             | 122 ++--
 .../src/matches/group/loader/yaml/mod.rs      |  90 ++-
 espanso-config/src/matches/group/path.rs      |  95 ++-
 espanso-config/src/matches/store/default.rs   | 569 +++++++++++++++++-
 espanso-config/src/matches/store/mod.rs       |   4 +-
 7 files changed, 811 insertions(+), 78 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index d8b9a72..5dbbc8b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -196,6 +196,12 @@ version = "0.4.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "88d7ed2934d741c6b37e33e3832298e8850b53fd2d2bea03873375596c7cea4e"
 
+[[package]]
+name = "dunce"
+version = "1.0.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b2641c4a7c0c4101df53ea572bffdc561c146f6c2eb09e4df02bc4811e3feeb4"
+
 [[package]]
 name = "either"
 version = "1.6.1"
@@ -231,6 +237,7 @@ name = "espanso-config"
 version = "0.1.0"
 dependencies = [
  "anyhow",
+ "dunce",
  "glob",
  "lazy_static",
  "log",
diff --git a/espanso-config/Cargo.toml b/espanso-config/Cargo.toml
index 87bc91c..c086296 100644
--- a/espanso-config/Cargo.toml
+++ b/espanso-config/Cargo.toml
@@ -13,7 +13,7 @@ serde_yaml = "0.8.17"
 glob = "0.3.0"
 regex = "1.4.3"
 lazy_static = "1.4.0"
-
+dunce = "1.0.1"
 
 [dev-dependencies]
 tempdir = "0.3.7"
\ No newline at end of file
diff --git a/espanso-config/src/config/path.rs b/espanso-config/src/config/path.rs
index 387a787..c3f0a28 100644
--- a/espanso-config/src/config/path.rs
+++ b/espanso-config/src/config/path.rs
@@ -1,4 +1,7 @@
-use std::{collections::HashSet, path::{Path, PathBuf}};
+use std::{
+  collections::HashSet,
+  path::{Path, PathBuf},
+};
 
 use glob::glob;
 use log::error;
@@ -8,7 +11,10 @@ lazy_static! {
   static ref ABSOLUTE_PATH: Regex = Regex::new(r"(?m)^([a-zA-Z]:/|/).*$").unwrap();
 }
 
-pub fn calculate_paths<'a>(base_dir: &Path, glob_patterns: impl Iterator<Item = &'a String>) -> HashSet<String> {
+pub fn calculate_paths<'a>(
+  base_dir: &Path,
+  glob_patterns: impl Iterator<Item = &'a String>,
+) -> HashSet<String> {
   let mut path_set = HashSet::new();
   for glob_pattern in glob_patterns {
     // Handle relative and absolute paths appropriately
@@ -24,7 +30,15 @@ pub fn calculate_paths<'a>(base_dir: &Path, glob_patterns: impl Iterator<Item =
         for path in paths {
           match path {
             Ok(path) => {
-              path_set.insert(path.to_string_lossy().to_string());
+              // Canonicalize the path
+              match dunce::canonicalize(&path) {
+                Ok(canonical_path) => {
+                  path_set.insert(canonical_path.to_string_lossy().to_string());
+                }
+                Err(err) => {
+                  error!("unable to canonicalize path from glob: {:?}, with error: {}", path, err);
+                }
+              }
             }
             Err(err) => error!(
               "glob error when processing pattern: {}, with error: {}",
@@ -49,31 +63,63 @@ pub fn calculate_paths<'a>(base_dir: &Path, glob_patterns: impl Iterator<Item =
 pub mod tests {
   use super::*;
   use crate::util::tests::use_test_directory;
-  use std::{fs::create_dir_all};
+  use std::fs::create_dir_all;
 
   #[test]
   fn calculate_paths_relative_paths() {
-    use_test_directory(|base, match_dir, config_dir| {
+    use_test_directory(|base, match_dir, _| {
       let sub_dir = match_dir.join("sub");
       create_dir_all(&sub_dir).unwrap();
 
-      std::fs::write(match_dir.join("base.yml"), "test").unwrap();
-      std::fs::write(match_dir.join("another.yml"), "test").unwrap();
-      std::fs::write(match_dir.join("_sub.yml"), "test").unwrap();
-      std::fs::write(sub_dir.join("sub.yml"), "test").unwrap();
+      let base_file = match_dir.join("base.yml");
+      std::fs::write(&base_file, "test").unwrap();
+      let another_file = match_dir.join("another.yml");
+      std::fs::write(&another_file, "test").unwrap();
+      let under_file = match_dir.join("_sub.yml");
+      std::fs::write(&under_file, "test").unwrap();
+      let sub_file = sub_dir.join("sub.yml");
+      std::fs::write(&sub_file, "test").unwrap();
 
-      let result = calculate_paths(base, vec![
-        "**/*.yml".to_string(),
-        "match/sub/*.yml".to_string(),
-        // Invalid path
-        "invalid".to_string(),
-      ].iter());
+      let result = calculate_paths(
+        base,
+        vec![
+          "**/*.yml".to_string(),
+          "match/sub/*.yml".to_string(),
+          // Invalid path
+          "invalid".to_string(),
+        ]
+        .iter(),
+      );
 
       let mut expected = HashSet::new();
-      expected.insert(format!("{}/match/base.yml", base.to_string_lossy()));
-      expected.insert(format!("{}/match/another.yml", base.to_string_lossy()));
-      expected.insert(format!("{}/match/_sub.yml", base.to_string_lossy()));
-      expected.insert(format!("{}/match/sub/sub.yml", base.to_string_lossy()));
+      expected.insert(base_file.to_string_lossy().to_string());
+      expected.insert(another_file.to_string_lossy().to_string());
+      expected.insert(under_file.to_string_lossy().to_string());
+      expected.insert(sub_file.to_string_lossy().to_string());
+
+      assert_eq!(result, expected);
+    });
+  }
+
+  #[test]
+  fn calculate_paths_relative_with_parent_modifier() {
+    use_test_directory(|base, match_dir, _| {
+      let sub_dir = match_dir.join("sub");
+      create_dir_all(&sub_dir).unwrap();
+
+      let base_file = match_dir.join("base.yml");
+      std::fs::write(&base_file, "test").unwrap();
+      let another_file = match_dir.join("another.yml");
+      std::fs::write(&another_file, "test").unwrap();
+      let under_file = match_dir.join("_sub.yml");
+      std::fs::write(&under_file, "test").unwrap();
+      let sub_file = sub_dir.join("sub.yml");
+      std::fs::write(&sub_file, "test").unwrap();
+
+      let result = calculate_paths(base, vec!["match/sub/../sub/*.yml".to_string()].iter());
+
+      let mut expected = HashSet::new();
+      expected.insert(sub_file.to_string_lossy().to_string());
 
       assert_eq!(result, expected);
     });
@@ -81,27 +127,35 @@ pub mod tests {
 
   #[test]
   fn calculate_paths_absolute_paths() {
-    use_test_directory(|base, match_dir, config_dir| {
+    use_test_directory(|base, match_dir, _| {
       let sub_dir = match_dir.join("sub");
       create_dir_all(&sub_dir).unwrap();
 
-      std::fs::write(match_dir.join("base.yml"), "test").unwrap();
-      std::fs::write(match_dir.join("another.yml"), "test").unwrap();
-      std::fs::write(match_dir.join("_sub.yml"), "test").unwrap();
-      std::fs::write(sub_dir.join("sub.yml"), "test").unwrap();
+      let base_file = match_dir.join("base.yml");
+      std::fs::write(&base_file, "test").unwrap();
+      let another_file = match_dir.join("another.yml");
+      std::fs::write(&another_file, "test").unwrap();
+      let under_file = match_dir.join("_sub.yml");
+      std::fs::write(&under_file, "test").unwrap();
+      let sub_file = sub_dir.join("sub.yml");
+      std::fs::write(&sub_file, "test").unwrap();
 
-      let result = calculate_paths(base, vec![
-        format!("{}/**/*.yml", base.to_string_lossy()),
-        format!("{}/match/sub/*.yml", base.to_string_lossy()),
-        // Invalid path
-        "invalid".to_string(),
-      ].iter());
+      let result = calculate_paths(
+        base,
+        vec![
+          format!("{}/**/*.yml", base.to_string_lossy()),
+          format!("{}/match/sub/*.yml", base.to_string_lossy()),
+          // Invalid path
+          "invalid".to_string(),
+        ]
+        .iter(),
+      );
 
       let mut expected = HashSet::new();
-      expected.insert(format!("{}/match/base.yml", base.to_string_lossy()));
-      expected.insert(format!("{}/match/another.yml", base.to_string_lossy()));
-      expected.insert(format!("{}/match/_sub.yml", base.to_string_lossy()));
-      expected.insert(format!("{}/match/sub/sub.yml", base.to_string_lossy()));
+      expected.insert(base_file.to_string_lossy().to_string());
+      expected.insert(another_file.to_string_lossy().to_string());
+      expected.insert(under_file.to_string_lossy().to_string());
+      expected.insert(sub_file.to_string_lossy().to_string());
 
       assert_eq!(result, expected);
     });
diff --git a/espanso-config/src/matches/group/loader/yaml/mod.rs b/espanso-config/src/matches/group/loader/yaml/mod.rs
index 400376f..cc3edd9 100644
--- a/espanso-config/src/matches/group/loader/yaml/mod.rs
+++ b/espanso-config/src/matches/group/loader/yaml/mod.rs
@@ -1,7 +1,10 @@
-use crate::matches::{Match, Variable, group::{MatchGroup, path::resolve_imports}};
+use crate::matches::{
+  group::{path::resolve_imports, MatchGroup},
+  Match, Variable,
+};
+use anyhow::Result;
 use log::warn;
 use parse::YAMLMatchGroup;
-use anyhow::Result;
 use std::convert::{TryFrom, TryInto};
 
 use self::parse::{YAMLMatch, YAMLVariable};
@@ -24,9 +27,6 @@ impl Importer for YAMLImporter {
     extension == "yaml" || extension == "yml"
   }
 
-  // TODO: test
-  // TODO: test resolve imports
-  // TODO: test cyclical dependency
   fn load_group(
     &self,
     path: &std::path::Path,
@@ -65,7 +65,6 @@ impl Importer for YAMLImporter {
 impl TryFrom<YAMLMatch> for Match {
   type Error = anyhow::Error;
 
-  // TODO: test
   fn try_from(yaml_match: YAMLMatch) -> Result<Self, Self::Error> {
     let triggers = if let Some(trigger) = yaml_match.trigger {
       Some(vec![trigger])
@@ -110,7 +109,10 @@ impl TryFrom<YAMLMatch> for Match {
     };
 
     if let MatchEffect::None = effect {
-      warn!("match caused by {:?} does not produce any effect. Did you forget the 'replace' field?", cause);
+      warn!(
+        "match caused by {:?} does not produce any effect. Did you forget the 'replace' field?",
+        cause
+      );
     }
 
     Ok(Self {
@@ -125,7 +127,6 @@ impl TryFrom<YAMLMatch> for Match {
 impl TryFrom<YAMLVariable> for Variable {
   type Error = anyhow::Error;
 
-  // TODO: test
   fn try_from(yaml_var: YAMLVariable) -> Result<Self, Self::Error> {
     Ok(Self {
       name: yaml_var.name,
@@ -138,10 +139,10 @@ impl TryFrom<YAMLVariable> for Variable {
 
 #[cfg(test)]
 mod tests {
+  use super::*;
+  use std::fs::create_dir_all;
   use serde_yaml::{Mapping, Value};
-
-    use super::*;
-  use crate::matches::Match;
+  use crate::{matches::Match, util::tests::use_test_directory};
 
   fn create_match(yaml: &str) -> Result<Match> {
     let yaml_match: YAMLMatch = serde_yaml::from_str(yaml)?;
@@ -371,4 +372,71 @@ mod tests {
       }
     )
   }
+
+  #[test]
+  fn importer_is_supported() {
+    let importer = YAMLImporter::new();
+    assert!(importer.is_supported("yaml"));
+    assert!(importer.is_supported("yml"));
+    assert!(!importer.is_supported("invalid"));
+  }
+
+  #[test]
+  fn importer_works_correctly() {
+    use_test_directory(|_, match_dir, _| {
+      let sub_dir = match_dir.join("sub");
+      create_dir_all(&sub_dir).unwrap();
+
+      let base_file = match_dir.join("base.yml");
+      std::fs::write(&base_file, r#"
+      imports:
+        - "sub/sub.yml"
+        - "invalid/import.yml" # This should be discarded
+      
+      global_vars:
+        - name: "var1"
+          type: "test"
+      
+      matches:
+        - trigger: "hello"
+          replace: "world"
+      "#).unwrap();
+
+      let sub_file = sub_dir.join("sub.yml");
+      std::fs::write(&sub_file, "").unwrap(); 
+      
+      let importer = YAMLImporter::new();
+      let group = importer.load_group(&base_file).unwrap();
+      
+      let vars = vec![Variable {
+        name: "var1".to_string(),
+        var_type: "test".to_string(),
+        params: Mapping::new(),
+        ..Default::default()
+      }];
+
+      assert_eq!(
+        group,
+        MatchGroup {
+          imports: vec![
+            sub_file.to_string_lossy().to_string(),
+          ],
+          global_vars: vars,
+          matches: vec![
+            Match {
+              cause: MatchCause::Trigger(TriggerCause {
+                triggers: vec!["hello".to_string()],
+                ..Default::default()
+              }),
+              effect: MatchEffect::Text(TextEffect {
+                replace: "world".to_string(),
+                ..Default::default()
+              }),
+              ..Default::default()
+            }
+          ],
+        }
+      )
+    });
+  }
 }
diff --git a/espanso-config/src/matches/group/path.rs b/espanso-config/src/matches/group/path.rs
index ba1773c..8143ec9 100644
--- a/espanso-config/src/matches/group/path.rs
+++ b/espanso-config/src/matches/group/path.rs
@@ -1,9 +1,8 @@
-use std::path::{Path, PathBuf};
 use anyhow::Result;
 use log::error;
+use std::path::{Path, PathBuf};
 use thiserror::Error;
 
-// TODO: test
 pub fn resolve_imports(group_path: &Path, imports: &[String]) -> Result<Vec<String>> {
   let mut paths = Vec::new();
 
@@ -22,8 +21,8 @@ pub fn resolve_imports(group_path: &Path, imports: &[String]) -> Result<Vec<Stri
     }
   } else {
     group_path
-  }; 
-  
+  };
+
   for import in imports.iter() {
     let import_path = PathBuf::from(import);
 
@@ -34,11 +33,21 @@ pub fn resolve_imports(group_path: &Path, imports: &[String]) -> Result<Vec<Stri
       import_path
     };
 
-    if full_path.exists() && full_path.is_file() {
-      paths.push(full_path)
-    } else {
-      // Best effort imports
-      error!("unable to resolve import at path: {:?}", full_path);
+    match dunce::canonicalize(&full_path) {
+      Ok(canonical_path) => {
+        if canonical_path.exists() && canonical_path.is_file() {
+          paths.push(canonical_path)
+        } else {
+          // Best effort imports
+          error!("unable to resolve import at path: {:?}", canonical_path);
+        }
+      }
+      Err(error) => {
+        error!(
+          "unable to canonicalize import path: {:?}, with error: {}",
+          full_path, error
+        );
+      }
     }
   }
 
@@ -46,7 +55,7 @@ pub fn resolve_imports(group_path: &Path, imports: &[String]) -> Result<Vec<Stri
     .into_iter()
     .map(|path| path.to_string_lossy().to_string())
     .collect();
-  
+
   Ok(string_paths)
 }
 
@@ -54,4 +63,68 @@ pub fn resolve_imports(group_path: &Path, imports: &[String]) -> Result<Vec<Stri
 pub enum ResolveImportError {
   #[error("resolve import failed: `{0}`")]
   Failed(String),
-}
\ No newline at end of file
+}
+
+#[cfg(test)]
+pub mod tests {
+  use super::*;
+  use crate::util::tests::use_test_directory;
+  use std::fs::create_dir_all;
+
+  #[test]
+  fn resolve_imports_works_correctly() {
+    use_test_directory(|_, match_dir, _| {
+      let sub_dir = match_dir.join("sub");
+      create_dir_all(&sub_dir).unwrap();
+
+      let base_file = match_dir.join("base.yml");
+      std::fs::write(&base_file, "test").unwrap();
+
+      let another_file = match_dir.join("another.yml");
+      std::fs::write(&another_file, "test").unwrap();
+
+      let sub_file = sub_dir.join("sub.yml");
+      std::fs::write(&sub_file, "test").unwrap();
+
+      let absolute_file = sub_dir.join("absolute.yml");
+      std::fs::write(&absolute_file, "test").unwrap();
+
+      let imports = vec![
+        "another.yml".to_string(),
+        "sub/sub.yml".to_string(),
+        absolute_file.to_string_lossy().to_string(),
+        "sub/invalid.yml".to_string(), // Should be skipped
+      ];
+
+      assert_eq!(
+        resolve_imports(&base_file, &imports).unwrap(),
+        vec![
+          another_file.to_string_lossy().to_string(),
+          sub_file.to_string_lossy().to_string(),
+          absolute_file.to_string_lossy().to_string(),
+        ]
+      );
+    });
+  }
+
+  #[test]
+  fn resolve_imports_parent_relative_path() {
+    use_test_directory(|_, match_dir, _| {
+      let sub_dir = match_dir.join("sub");
+      create_dir_all(&sub_dir).unwrap();
+
+      let base_file = match_dir.join("base.yml");
+      std::fs::write(&base_file, "test").unwrap();
+
+      let sub_file = sub_dir.join("sub.yml");
+      std::fs::write(&sub_file, "test").unwrap();
+
+      let imports = vec!["../base.yml".to_string()];
+
+      assert_eq!(
+        resolve_imports(&sub_file, &imports).unwrap(),
+        vec![base_file.to_string_lossy().to_string(),]
+      );
+    });
+  }
+}
diff --git a/espanso-config/src/matches/store/default.rs b/espanso-config/src/matches/store/default.rs
index c3e9140..305d9ff 100644
--- a/espanso-config/src/matches/store/default.rs
+++ b/espanso-config/src/matches/store/default.rs
@@ -10,7 +10,6 @@ use crate::{
   matches::{group::MatchGroup, Match, Variable},
 };
 
-// TODO: implement store according to notes
 pub(crate) struct DefaultMatchStore {
   pub groups: HashMap<String, MatchGroup>,
 }
@@ -24,8 +23,6 @@ impl DefaultMatchStore {
 }
 
 impl MatchStore for DefaultMatchStore {
-  // TODO: test
-  // TODO: test cyclical imports
   fn load(&mut self, paths: &[String]) {
     // Because match groups can imports other match groups,
     // we have to load them recursively starting from the
@@ -33,9 +30,7 @@ impl MatchStore for DefaultMatchStore {
     load_match_groups_recursively(&mut self.groups, paths);
   }
 
-  // TODO: test
-  // TODO: test for cyclical imports
-  fn query_set(&self, paths: &[String]) -> MatchSet {
+  fn query(&self, paths: &[String]) -> MatchSet {
     let mut matches: Vec<&Match> = Vec::new();
     let mut global_vars: Vec<&Variable> = Vec::new();
     let mut visited_paths = HashSet::new();
@@ -65,8 +60,10 @@ fn load_match_groups_recursively(groups: &mut HashMap<String, MatchGroup>, paths
       let group_path = PathBuf::from(path);
       match MatchGroup::load(&group_path) {
         Ok(group) => {
-          load_match_groups_recursively(groups, &group.imports);
+          let imports = group.imports.clone();
           groups.insert(path.clone(), group);
+
+          load_match_groups_recursively(groups, &imports);
         }
         Err(error) => {
           error!("unable to load match group: {:?}", error);
@@ -76,7 +73,6 @@ fn load_match_groups_recursively(groups: &mut HashMap<String, MatchGroup>, paths
   }
 }
 
-// TODO: test
 fn query_matches_for_paths<'a>(
   groups: &'a HashMap<String, MatchGroup>,
   visited_paths: &mut HashSet<String>,
@@ -88,7 +84,19 @@ fn query_matches_for_paths<'a>(
 ) {
   for path in paths.iter() {
     if !visited_paths.contains(path) {
+      visited_paths.insert(path.clone());
+
       if let Some(group) = groups.get(path) {
+        query_matches_for_paths(
+          groups,
+          visited_paths,
+          visited_matches,
+          visited_global_vars,
+          matches,
+          global_vars,
+          &group.imports,
+        );
+
         for m in group.matches.iter() {
           if !visited_matches.contains(&m._id) {
             matches.push(m);
@@ -102,19 +110,540 @@ fn query_matches_for_paths<'a>(
             visited_global_vars.insert(var._id);
           }
         }
-
-        query_matches_for_paths(
-          groups,
-          visited_paths,
-          visited_matches,
-          visited_global_vars,
-          matches,
-          global_vars,
-          &group.imports,
-        )
       }
-
-      visited_paths.insert(path.clone());
     }
   }
 }
+
+#[cfg(test)]
+mod tests {
+  use super::*;
+  use crate::{
+    matches::{MatchCause, MatchEffect, TextEffect, TriggerCause},
+    util::tests::use_test_directory,
+  };
+  use std::fs::create_dir_all;
+
+  fn create_match(trigger: &str, replace: &str) -> Match {
+    Match {
+      cause: MatchCause::Trigger(TriggerCause {
+        triggers: vec![trigger.to_string()],
+        ..Default::default()
+      }),
+      effect: MatchEffect::Text(TextEffect {
+        replace: replace.to_string(),
+        ..Default::default()
+      }),
+      ..Default::default()
+    }
+  }
+
+  fn create_matches(matches: &[(&str, &str)]) -> Vec<Match> {
+    matches
+      .iter()
+      .map(|(trigger, replace)| create_match(trigger, replace))
+      .collect()
+  }
+
+  fn create_test_var(name: &str) -> Variable {
+    Variable {
+      name: name.to_string(),
+      var_type: "test".to_string(),
+      ..Default::default()
+    }
+  }
+
+  fn create_vars(vars: &[&str]) -> Vec<Variable> {
+    vars.iter().map(|var| create_test_var(var)).collect()
+  }
+
+  #[test]
+  fn match_store_loads_correctly() {
+    use_test_directory(|_, match_dir, _| {
+      let sub_dir = match_dir.join("sub");
+      create_dir_all(&sub_dir).unwrap();
+
+      let base_file = match_dir.join("base.yml");
+      std::fs::write(
+        &base_file,
+        r#"
+      imports:
+        - "_another.yml"
+      
+      matches:
+        - trigger: "hello"
+          replace: "world"
+      "#,
+      )
+      .unwrap();
+
+      let another_file = match_dir.join("_another.yml");
+      std::fs::write(
+        &another_file,
+        r#"
+      imports:
+        - "sub/sub.yml"
+      
+      matches:
+        - trigger: "hello"
+          replace: "world2" 
+        - trigger: "foo"
+          replace: "bar"
+      "#,
+      )
+      .unwrap();
+
+      let sub_file = sub_dir.join("sub.yml");
+      std::fs::write(
+        &sub_file,
+        r#"
+      matches:
+        - trigger: "hello"
+          replace: "world3" 
+      "#,
+      )
+      .unwrap();
+
+      let mut match_store = DefaultMatchStore::new();
+
+      match_store.load(&[base_file.to_string_lossy().to_string()]);
+
+      assert_eq!(match_store.groups.len(), 3);
+
+      let base_group = &match_store
+        .groups
+        .get(&base_file.to_string_lossy().to_string())
+        .unwrap()
+        .matches;
+      assert_eq!(base_group, &create_matches(&[("hello", "world")]));
+
+      let another_group = &match_store
+        .groups
+        .get(&another_file.to_string_lossy().to_string())
+        .unwrap()
+        .matches;
+      assert_eq!(
+        another_group,
+        &create_matches(&[("hello", "world2"), ("foo", "bar")])
+      );
+
+      let sub_group = &match_store
+        .groups
+        .get(&sub_file.to_string_lossy().to_string())
+        .unwrap()
+        .matches;
+      assert_eq!(sub_group, &create_matches(&[("hello", "world3")]));
+    });
+  }
+
+  #[test]
+  fn match_store_handles_circular_dependency() {
+    use_test_directory(|_, match_dir, _| {
+      let sub_dir = match_dir.join("sub");
+      create_dir_all(&sub_dir).unwrap();
+
+      let base_file = match_dir.join("base.yml");
+      std::fs::write(
+        &base_file,
+        r#"
+      imports:
+        - "_another.yml"
+      
+      matches:
+        - trigger: "hello"
+          replace: "world"
+      "#,
+      )
+      .unwrap();
+
+      let another_file = match_dir.join("_another.yml");
+      std::fs::write(
+        &another_file,
+        r#"
+      imports:
+        - "sub/sub.yml"
+      
+      matches:
+        - trigger: "hello"
+          replace: "world2" 
+        - trigger: "foo"
+          replace: "bar"
+      "#,
+      )
+      .unwrap();
+
+      let sub_file = sub_dir.join("sub.yml");
+      std::fs::write(
+        &sub_file,
+        r#"
+      imports:
+        - "../_another.yml"
+
+      matches:
+        - trigger: "hello"
+          replace: "world3" 
+      "#,
+      )
+      .unwrap();
+
+      let mut match_store = DefaultMatchStore::new();
+
+      match_store.load(&[base_file.to_string_lossy().to_string()]);
+
+      assert_eq!(match_store.groups.len(), 3);
+    });
+  }
+
+  #[test]
+  fn match_store_query_single_path_with_imports() {
+    use_test_directory(|_, match_dir, _| {
+      let sub_dir = match_dir.join("sub");
+      create_dir_all(&sub_dir).unwrap();
+
+      let base_file = match_dir.join("base.yml");
+      std::fs::write(
+        &base_file,
+        r#"
+      imports:
+        - "_another.yml"
+      
+      global_vars:
+        - name: var1
+          type: test
+
+      matches:
+        - trigger: "hello"
+          replace: "world"
+      "#,
+      )
+      .unwrap();
+
+      let another_file = match_dir.join("_another.yml");
+      std::fs::write(
+        &another_file,
+        r#"
+      imports:
+        - "sub/sub.yml"
+      
+      matches:
+        - trigger: "hello"
+          replace: "world2" 
+        - trigger: "foo"
+          replace: "bar"
+      "#,
+      )
+      .unwrap();
+
+      let sub_file = sub_dir.join("sub.yml");
+      std::fs::write(
+        &sub_file,
+        r#"
+      global_vars:
+        - name: var2
+          type: test
+
+      matches:
+        - trigger: "hello"
+          replace: "world3" 
+      "#,
+      )
+      .unwrap();
+
+      let mut match_store = DefaultMatchStore::new();
+
+      match_store.load(&[base_file.to_string_lossy().to_string()]);
+
+      let match_set = match_store.query(&[base_file.to_string_lossy().to_string()]);
+
+      assert_eq!(
+        match_set
+          .matches
+          .into_iter()
+          .cloned()
+          .collect::<Vec<Match>>(),
+        create_matches(&[
+          ("hello", "world3"),
+          ("hello", "world2"),
+          ("foo", "bar"),
+          ("hello", "world"),
+        ])
+      );
+
+      assert_eq!(
+        match_set
+          .global_vars
+          .into_iter()
+          .cloned()
+          .collect::<Vec<Variable>>(),
+        create_vars(&["var2", "var1"])
+      );
+    });
+  }
+
+  #[test]
+  fn match_store_query_handles_circular_depencencies() {
+    use_test_directory(|_, match_dir, _| {
+      let sub_dir = match_dir.join("sub");
+      create_dir_all(&sub_dir).unwrap();
+
+      let base_file = match_dir.join("base.yml");
+      std::fs::write(
+        &base_file,
+        r#"
+      imports:
+        - "_another.yml"
+      
+      global_vars:
+        - name: var1
+          type: test
+
+      matches:
+        - trigger: "hello"
+          replace: "world"
+      "#,
+      )
+      .unwrap();
+
+      let another_file = match_dir.join("_another.yml");
+      std::fs::write(
+        &another_file,
+        r#"
+      imports:
+        - "sub/sub.yml"
+      
+      matches:
+        - trigger: "hello"
+          replace: "world2" 
+        - trigger: "foo"
+          replace: "bar"
+      "#,
+      )
+      .unwrap();
+
+      let sub_file = sub_dir.join("sub.yml");
+      std::fs::write(
+        &sub_file,
+        r#"
+      imports:
+        - "../_another.yml"  # Circular import
+
+      global_vars:
+        - name: var2
+          type: test
+
+      matches:
+        - trigger: "hello"
+          replace: "world3" 
+      "#,
+      )
+      .unwrap();
+
+      let mut match_store = DefaultMatchStore::new();
+
+      match_store.load(&[base_file.to_string_lossy().to_string()]);
+
+      let match_set = match_store.query(&[base_file.to_string_lossy().to_string()]);
+
+      assert_eq!(
+        match_set
+          .matches
+          .into_iter()
+          .cloned()
+          .collect::<Vec<Match>>(),
+        create_matches(&[
+          ("hello", "world3"),
+          ("hello", "world2"),
+          ("foo", "bar"),
+          ("hello", "world"),
+        ])
+      );
+
+      assert_eq!(
+        match_set
+          .global_vars
+          .into_iter()
+          .cloned()
+          .collect::<Vec<Variable>>(),
+        create_vars(&["var2", "var1"])
+      );
+    });
+  }
+
+  #[test]
+  fn match_store_query_multiple_paths() {
+    use_test_directory(|_, match_dir, _| {
+      let sub_dir = match_dir.join("sub");
+      create_dir_all(&sub_dir).unwrap();
+
+      let base_file = match_dir.join("base.yml");
+      std::fs::write(
+        &base_file,
+        r#"
+      imports:
+        - "_another.yml"
+      
+      global_vars:
+        - name: var1
+          type: test
+
+      matches:
+        - trigger: "hello"
+          replace: "world"
+      "#,
+      )
+      .unwrap();
+
+      let another_file = match_dir.join("_another.yml");
+      std::fs::write(
+        &another_file,
+        r#"
+      matches:
+        - trigger: "hello"
+          replace: "world2" 
+        - trigger: "foo"
+          replace: "bar"
+      "#,
+      )
+      .unwrap();
+
+      let sub_file = sub_dir.join("sub.yml");
+      std::fs::write(
+        &sub_file,
+        r#"
+      global_vars:
+        - name: var2
+          type: test
+
+      matches:
+        - trigger: "hello"
+          replace: "world3" 
+      "#,
+      )
+      .unwrap();
+
+      let mut match_store = DefaultMatchStore::new();
+
+      match_store.load(&[
+        base_file.to_string_lossy().to_string(),
+        sub_file.to_string_lossy().to_string(),
+      ]);
+
+      let match_set = match_store.query(&[
+        base_file.to_string_lossy().to_string(),
+        sub_file.to_string_lossy().to_string(),
+      ]);
+
+      assert_eq!(
+        match_set
+          .matches
+          .into_iter()
+          .cloned()
+          .collect::<Vec<Match>>(),
+        create_matches(&[
+          ("hello", "world2"),
+          ("foo", "bar"),
+          ("hello", "world"),
+          ("hello", "world3"),
+        ])
+      );
+
+      assert_eq!(
+        match_set
+          .global_vars
+          .into_iter()
+          .cloned()
+          .collect::<Vec<Variable>>(),
+        create_vars(&["var1", "var2"])
+      );
+    });
+  }
+
+  #[test]
+  fn match_store_query_handle_duplicates_when_imports_and_paths_overlap() {
+    use_test_directory(|_, match_dir, _| {
+      let sub_dir = match_dir.join("sub");
+      create_dir_all(&sub_dir).unwrap();
+
+      let base_file = match_dir.join("base.yml");
+      std::fs::write(
+        &base_file,
+        r#"
+      imports:
+        - "_another.yml"
+      
+      global_vars:
+        - name: var1
+          type: test
+
+      matches:
+        - trigger: "hello"
+          replace: "world"
+      "#,
+      )
+      .unwrap();
+
+      let another_file = match_dir.join("_another.yml");
+      std::fs::write(
+        &another_file,
+        r#"
+      imports:
+        - "sub/sub.yml"
+      
+      matches:
+        - trigger: "hello"
+          replace: "world2" 
+        - trigger: "foo"
+          replace: "bar"
+      "#,
+      )
+      .unwrap();
+
+      let sub_file = sub_dir.join("sub.yml");
+      std::fs::write(
+        &sub_file,
+        r#"
+      global_vars:
+        - name: var2
+          type: test
+
+      matches:
+        - trigger: "hello"
+          replace: "world3" 
+      "#,
+      )
+      .unwrap();
+
+      let mut match_store = DefaultMatchStore::new();
+
+      match_store.load(&[base_file.to_string_lossy().to_string()]);
+
+      let match_set = match_store.query(&[
+        base_file.to_string_lossy().to_string(),
+        sub_file.to_string_lossy().to_string(),
+      ]);
+
+      assert_eq!(
+        match_set
+          .matches
+          .into_iter()
+          .cloned()
+          .collect::<Vec<Match>>(),
+        create_matches(&[
+          ("hello", "world3"),  // This appears only once, though it appears 2 times
+          ("hello", "world2"),
+          ("foo", "bar"),
+          ("hello", "world"),
+        ])
+      );
+
+      assert_eq!(
+        match_set
+          .global_vars
+          .into_iter()
+          .cloned()
+          .collect::<Vec<Variable>>(),
+        create_vars(&["var2", "var1"])
+      );
+    });
+  }
+}
diff --git a/espanso-config/src/matches/store/mod.rs b/espanso-config/src/matches/store/mod.rs
index 1ebcb6a..2feb44c 100644
--- a/espanso-config/src/matches/store/mod.rs
+++ b/espanso-config/src/matches/store/mod.rs
@@ -4,7 +4,7 @@ mod default;
 
 pub trait MatchStore {
   fn load(&mut self, paths: &[String]);
-  fn query_set(&self, paths: &[String]) -> MatchSet;
+  fn query(&self, paths: &[String]) -> MatchSet;
 }
 
 #[derive(Debug, Clone, PartialEq)]
@@ -14,5 +14,7 @@ pub struct MatchSet<'a> {
 }
 
 pub fn new() -> impl MatchStore {
+  // TODO: here we can replace the DefaultMatchStore with a caching wrapper
+  // that returns the same response for the given "paths" query
   default::DefaultMatchStore::new()
 }
\ No newline at end of file