test: improve locator coverage (Fixes #389)

Author: karthiknadig

crates/pet-core/src/python_environment.rs

@@ -415,13 +415,16 @@ pub fn get_environment_key(env: &PythonEnvironment) -> Option<PathBuf> {
     if let Some(exe) = &env.executable {
         Some(exe.clone())
     } else if let Some(prefix) = &env.prefix {
-        // If this is a conda env without Python, then the exe will be prefix/bin/python
+        // If this is a conda env without Python, use the platform's default interpreter path.
         if env.kind == Some(PythonEnvironmentKind::Conda) {
-            Some(prefix.join("bin").join(if cfg!(windows) {
-                "python.exe"
-            } else {
-                "python"
-            }))
+            #[cfg(windows)]
+            {
+                Some(prefix.join("python.exe"))
+            }
+            #[cfg(not(windows))]
+            {
+                Some(prefix.join("bin").join("python"))
+            }
         } else {
             Some(prefix.clone())
         }
@@ -436,11 +439,12 @@ pub fn get_environment_key(env: &PythonEnvironment) -> Option<PathBuf> {
 
 #[cfg(test)]
 mod tests {
-    #[cfg(windows)]
-    use super::{get_shortest_executable, PythonEnvironmentKind};
-    #[cfg(windows)]
+    use super::{get_environment_key, PythonEnvironment, PythonEnvironmentKind};
     use std::path::PathBuf;
 
+    #[cfg(windows)]
+    use super::get_shortest_executable;
+
     #[test]
     #[cfg(windows)]
     fn shorted_exe_path_windows_store() {
@@ -459,4 +463,64 @@ mod tests {
             ))
         );
     }
+
+    #[test]
+    fn environment_key_uses_executable_when_available() {
+        let executable = PathBuf::from(if cfg!(windows) {
+            r"C:\env\Scripts\python.exe"
+        } else {
+            "/env/bin/python"
+        });
+        let prefix = PathBuf::from(if cfg!(windows) { r"C:\env" } else { "/env" });
+        let environment = PythonEnvironment {
+            executable: Some(executable.clone()),
+            prefix: Some(prefix),
+            kind: Some(PythonEnvironmentKind::Venv),
+            ..Default::default()
+        };
+
+        assert_eq!(get_environment_key(&environment), Some(executable));
+    }
+
+    #[test]
+    fn environment_key_uses_conda_default_python_when_executable_is_missing() {
+        let prefix = PathBuf::from(if cfg!(windows) {
+            r"C:\conda-env"
+        } else {
+            "/conda-env"
+        });
+        let environment = PythonEnvironment {
+            executable: None,
+            prefix: Some(prefix.clone()),
+            kind: Some(PythonEnvironmentKind::Conda),
+            ..Default::default()
+        };
+
+        assert_eq!(
+            get_environment_key(&environment),
+            Some(if cfg!(windows) {
+                prefix.join("python.exe")
+            } else {
+                prefix.join("bin").join("python")
+            })
+        );
+    }
+
+    #[test]
+    fn environment_key_uses_non_conda_prefix_when_executable_is_missing() {
+        let prefix = PathBuf::from(if cfg!(windows) { r"C:\env" } else { "/env" });
+        let environment = PythonEnvironment {
+            executable: None,
+            prefix: Some(prefix.clone()),
+            kind: Some(PythonEnvironmentKind::Venv),
+            ..Default::default()
+        };
+
+        assert_eq!(get_environment_key(&environment), Some(prefix));
+    }
+
+    #[test]
+    fn environment_key_returns_none_without_executable_or_prefix() {
+        assert_eq!(get_environment_key(&PythonEnvironment::default()), None);
+    }
 }

crates/pet-env-var-path/src/lib.rs

@@ -3,29 +3,47 @@
 
 use pet_core::os_environment::Environment;
 use std::collections::HashSet;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 pub fn get_search_paths_from_env_variables(environment: &dyn Environment) -> Vec<PathBuf> {
-    // Exclude files from this folder, as they would have been discovered elsewhere (widows_store)
+    let search_paths = environment
+        .get_know_global_search_locations()
+        .into_iter()
+        .map(normalize_search_path)
+        .collect::<HashSet<PathBuf>>();
+
+    // Exclude files from this folder, as they would have been discovered elsewhere (windows_store)
     // Also the exe is merely a pointer to another file.
-    if let Some(home) = environment.get_user_home() {
+    let user_home = environment.get_user_home();
+    search_paths
+        .into_iter()
+        .filter(|search_path| !is_windows_apps_path(search_path, user_home.as_ref()))
+        .collect()
+}
+
+fn is_windows_apps_path(search_path: &Path, user_home: Option<&PathBuf>) -> bool {
+    if let Some(home) = user_home {
         let apps_path = home
             .join("AppData")
             .join("Local")
             .join("Microsoft")
             .join("WindowsApps");
-
-        environment
-            .get_know_global_search_locations()
-            .into_iter()
-            .map(normalize_search_path)
-            .collect::<HashSet<PathBuf>>()
-            .into_iter()
-            .filter(|p| !p.starts_with(apps_path.clone()))
-            .collect()
-    } else {
-        Vec::new()
+        if search_path.starts_with(apps_path) {
+            return true;
+        }
     }
+
+    let components = search_path
+        .components()
+        .map(|component| component.as_os_str().to_string_lossy())
+        .collect::<Vec<_>>();
+
+    components.windows(4).any(|components| {
+        components[0].eq_ignore_ascii_case("AppData")
+            && components[1].eq_ignore_ascii_case("Local")
+            && components[2].eq_ignore_ascii_case("Microsoft")
+            && components[3].eq_ignore_ascii_case("WindowsApps")
+    })
 }
 
 /// Normalizes a search path for deduplication purposes.
@@ -52,3 +70,97 @@ fn normalize_search_path(path: PathBuf) -> PathBuf {
         pet_fs::path::norm_case(&path)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::{
+        fs,
+        time::{SystemTime, UNIX_EPOCH},
+    };
+
+    struct TestEnvironment {
+        user_home: Option<PathBuf>,
+        global_search_locations: Vec<PathBuf>,
+    }
+
+    impl Environment for TestEnvironment {
+        fn get_user_home(&self) -> Option<PathBuf> {
+            self.user_home.clone()
+        }
+
+        fn get_root(&self) -> Option<PathBuf> {
+            None
+        }
+
+        fn get_env_var(&self, _key: String) -> Option<String> {
+            None
+        }
+
+        fn get_know_global_search_locations(&self) -> Vec<PathBuf> {
+            self.global_search_locations.clone()
+        }
+    }
+
+    fn create_test_dir(name: &str) -> PathBuf {
+        let unique = SystemTime::now()
+            .duration_since(UNIX_EPOCH)
+            .unwrap()
+            .as_nanos();
+        let directory = std::env::temp_dir().join(format!(
+            "pet-env-var-path-{name}-{}-{unique}",
+            std::process::id()
+        ));
+        fs::create_dir_all(&directory).unwrap();
+        directory
+    }
+
+    #[test]
+    fn search_paths_are_deduplicated_and_windows_apps_paths_are_filtered() {
+        let home = create_test_dir("home");
+        let regular_path = home.join("Python");
+        let windows_apps_path = home
+            .join("AppData")
+            .join("Local")
+            .join("Microsoft")
+            .join("WindowsApps");
+        fs::create_dir_all(&regular_path).unwrap();
+        fs::create_dir_all(&windows_apps_path).unwrap();
+
+        let environment = TestEnvironment {
+            user_home: Some(home.clone()),
+            global_search_locations: vec![
+                regular_path.clone(),
+                regular_path.clone(),
+                windows_apps_path,
+            ],
+        };
+
+        let mut search_paths = get_search_paths_from_env_variables(&environment);
+        search_paths.sort();
+
+        assert_eq!(search_paths, vec![normalize_search_path(regular_path)]);
+
+        fs::remove_dir_all(home).unwrap();
+    }
+
+    #[test]
+    fn search_paths_are_preserved_when_home_is_unknown() {
+        let environment = TestEnvironment {
+            user_home: None,
+            global_search_locations: vec![
+                PathBuf::from("/usr/bin"),
+                PathBuf::from(if cfg!(windows) {
+                    r"C:\Users\User\AppData\Local\Microsoft\WindowsApps"
+                } else {
+                    "/Users/user/AppData/Local/Microsoft/WindowsApps"
+                }),
+            ],
+        };
+
+        assert_eq!(
+            get_search_paths_from_env_variables(&environment),
+            vec![normalize_search_path(PathBuf::from("/usr/bin"))]
+        );
+    }
+}