add new test and add fish script to add to path

Author: Artemis Livingstone

.gitignore

@@ -9,3 +9,4 @@
 /local-rustup
 /snapcraft
 flake.lock
+.vscode/

src/cli/self_update/env.fish

@@ -0,0 +1 @@
+contains {cargo_bin} $fish_user_paths; or set -Ua fish_user_paths {cargo_bin}

src/cli/self_update/shell.rs

@@ -1,3 +1,7 @@
+#![warn(clippy::all)]
+#![warn(clippy::pedantic)]
+#![allow(clippy::doc_markdown)]
+
 //! Paths and Unix shells
 //!
 //! MacOS, Linux, FreeBSD, and many other OS model their design on Unix,
@@ -95,7 +99,7 @@ pub(crate) trait UnixShell {
     // Gives rcs that should be written to.
     fn update_rcs(&self) -> Vec<PathBuf>;
 
-    // By defualt follows POSIX and sources <cargo home>/env
+    // By default follows POSIX and sources <cargo home>/env
     fn add_to_path(&self) -> Result<(), anyhow::Error> {
         let source_cmd = self.source_string()?;
         let source_cmd_with_newline = format!("\n{}", &source_cmd);
@@ -249,13 +253,13 @@ impl Fish {
     #![allow(non_snake_case)]
     /// Gets fish vendor config location from `XDG_DATA_DIRS`
     /// Returns None if `XDG_DATA_DIRS` is not set or if there is no fis`fish/vendor_conf.d`.d directory found
-    pub fn get_XDG_DATA_DIRS() -> Option<PathBuf> {
-        #[cfg(test)]
-        return None;
+    pub fn get_vendor_config_from_XDG_DATA_DIRS() -> Option<PathBuf> {
+        // Skip the directory during testing as we don't want to write into the XDG_DATA_DIRS by accident
+        // TODO: Change the definition of XDG_DATA_DIRS in test so that doesn't happen
 
+        // TODO: Set up XDG DATA DIRS in a test to test the location being correct
         return process()
             .var("XDG_DATA_DIRS")
-            // If var is found
             .map(|var| {
                 var.split(':')
                     .map(PathBuf::from)
@@ -274,14 +278,15 @@ impl UnixShell for Fish {
 
     fn rcfiles(&self) -> Vec<PathBuf> {
         // As per https://fishshell.com/docs/current/language.html#configuration
-        // Vendor config files should be written to `/fish/vendor_config.d/`
+        // Vendor config files should be written to `/usr/share/fish/vendor_config.d/`
         // if that does not exist then it should be written to `/usr/share/fish/vendor_conf.d/`
         // otherwise it should be written to `$HOME/.config/fish/conf.d/ as per discussions in github issue #478
         [
-            #[cfg(test)] // Write to test location so we don't pollute during testing.
-            utils::home_dir().map(|home| home.join(".config/fish/conf.d/")),
-            Self::get_XDG_DATA_DIRS(),
+            // #[cfg(test)] // Write to test location so we don't pollute during testing.
+            // utils::home_dir().map(|home| home.join(".config/fish/conf.d/")),
+            Self::get_vendor_config_from_XDG_DATA_DIRS(),
             Some(PathBuf::from("/usr/share/fish/vendor_conf.d/")),
+            Some(PathBuf::from("/usr/local/share/fish/vendor_conf.d/")),
             utils::home_dir().map(|home| home.join(".config/fish/conf.d/")),
         ]
         .iter_mut()
@@ -291,17 +296,16 @@ impl UnixShell for Fish {
     }
 
     fn update_rcs(&self) -> Vec<PathBuf> {
+        // TODO: Change rcfiles to just read parent dirs
         self.rcfiles()
             .into_iter()
             .filter(|rc| {
-                rc.metadata() // Returns error if path doesnt exist so separate check is not needed
-                    .map_or(false, |metadata| {
-                        dbg!(rc);
-                        dbg!(metadata.permissions());
-                        dbg!(metadata.permissions().readonly());
-                        dbg!();
-                        !metadata.permissions().readonly()
-                    })
+                // We only want to check if the directory exists as in fish we create a new file for every applications env
+                rc.parent().map_or(false, |parent| {
+                    parent
+                        .metadata() // Returns error if path doesn't exist so separate check is not needed
+                        .map_or(false, |metadata| !metadata.permissions().readonly())
+                })
             })
             .collect()
     }
@@ -331,8 +335,8 @@ impl UnixShell for Fish {
 
     fn env_script(&self) -> ShellScript {
         ShellScript {
-            name: "env",
-            content: include_str!("env.sh"),
+            name: "env.fish",
+            content: include_str!("env.fish"),
         }
     }
 
@@ -346,13 +350,14 @@ impl UnixShell for Fish {
 }
 
 pub(crate) fn legacy_paths() -> impl Iterator<Item = PathBuf> {
-    let zprofiles = Zsh::zdotdir()
+    let z_profiles = Zsh::zdotdir()
         .into_iter()
         .chain(utils::home_dir())
         .map(|d| d.join(".zprofile"));
-    let profiles = [".bash_profile", ".profile"]
+
+    let bash_profiles = [".bash_profile", ".profile"]
         .iter()
         .filter_map(|rc| utils::home_dir().map(|d| d.join(rc)));
 
-    profiles.chain(zprofiles)
+    bash_profiles.chain(z_profiles)
 }

tests/cli-paths.rs

@@ -71,6 +71,31 @@ export PATH="$HOME/apple/bin"
         });
     }
 
+    #[test]
+    fn install_creates_necessary_fish_scripts_new() {
+        clitools::setup(Scenario::Empty, &|config| {
+            let mut cmd = clitools::cmd(config, "rustup-init", &INIT_NONE[1..]);
+            let rcs: Vec<PathBuf> = [".cargo/env.fish", ".config/fish/config.fish"]
+                .iter()
+                .map(|file| config.homedir.join(file))
+                .collect();
+            for file in &rcs {
+                assert!(!file.exists());
+            }
+            cmd.env_remove("CARGO_HOME");
+            cmd.env("SHELL", "fish");
+            cmd.env("XDG_DATA_DIRS", ""); // Overwrite XDG as host shouldn't be affected by the test.
+            assert!(cmd.output().unwrap().status.success());
+            let expected = include_str!("../src/cli/self_update/env.fish")
+                .replace("{cargo_bin}", "$HOME/.cargo/bin");
+
+            assert!(rcs
+                .iter()
+                .filter_map(|rc| fs::read_to_string(rc).ok()) // Read contents of files that exist
+                .any(|rc_content| rc_content.contains(&expected)));
+        })
+    }
+
     #[test]
     fn install_updates_bash_rcs() {
         clitools::setup(Scenario::Empty, &|config| {

tests/mock/clitools.rs

@@ -99,7 +99,8 @@ pub fn setup(s: Scenario, f: &dyn Fn(&mut Config)) {
     env::remove_var("RUSTUP_TOOLCHAIN");
     env::remove_var("SHELL");
     env::remove_var("ZDOTDIR");
-    // clap does its own terminal colour probing, and that isn't
+    env::remove_var("XDG_DATA_DIRS");
+    // clap does it's own terminal colour probing, and that isn't
     // trait-controllable, but it does honour the terminal. To avoid testing
     // claps code, lie about whatever terminal this process was started under.
     env::set_var("TERM", "dumb");