Merge pull request #2416 from rbtcollins/unit-tests

More unit testifications

Author: rbtcollins

Cargo.lock

@@ -1,69 +1,70 @@
 [package]
-name = "rustup"
-version = "1.22.1"
-authors = [ "Daniel Silverstone <[email protected]>", "Diggory Blake <[email protected]>" ]
+authors = ["Daniel Silverstone <[email protected]>", "Diggory Blake <[email protected]>"]
 build = "build.rs"
+description = "Manage multiple rust installations with ease"
 edition = "2018"
 homepage = "https://github.com/rust-lang/rustup"
 keywords = ["rustup", "multirust", "install", "proxy"]
 license = "MIT OR Apache-2.0"
+name = "rustup"
 readme = "README.md"
 repository = "https://github.com/rust-lang/rustup"
-description = "Manage multiple rust installations with ease"
+version = "1.22.1"
 
 [features]
-default = ["curl-backend", "reqwest-backend"]
 curl-backend = ["download/curl-backend"]
+default = ["curl-backend", "reqwest-backend"]
 reqwest-backend = ["download/reqwest-backend"]
 vendored-openssl = ['openssl/vendored']
 # Include in the default set to disable self-update and uninstall.
 no-self-update = []
 
 # Sorted by alphabetic order
 [dependencies]
+anyhow = "1.0.31"
 cfg-if = "0.1"
 chrono = "0.4"
 clap = "2"
-download = { path = "download" }
+download = {path = "download"}
 error-chain = "0.12"
 flate2 = "1"
 git-testament = "0.1.4"
-home = { git = "https://github.com/rbtcollins/home", rev="a243ee2fbee6022c57d56f5aa79aefe194eabe53"  }
+home = {git = "https://github.com/rbtcollins/home", rev = "a243ee2fbee6022c57d56f5aa79aefe194eabe53"}
 lazy_static = "1"
 libc = "0.2"
 num_cpus = "1.13"
 opener = "0.4.0"
 # Used by `curl` or `reqwest` backend although it isn't imported
 # by our rustup.
-openssl = { version = "0.10", optional = true }
-pulldown-cmark = { version = "0.7", default-features = false }
+openssl = {version = "0.10", optional = true}
+pulldown-cmark = {version = "0.7", default-features = false}
 rand = "0.7"
 regex = "1"
 remove_dir_all = "0.5.1"
 same-file = "1"
 scopeguard = "1"
 semver = "0.9"
-serde = { version = "1.0", features = ["derive"] }
+serde = {version = "1.0", features = ["derive"]}
 sha2 = "0.8"
 strsim = "0.10"
 tar = "0.4.26"
 tempfile = "3.1"
 # FIXME(issue #1788)
+pgp = {version = "0.5", default-features = false}
 term = "=0.5.1"
 threadpool = "1"
 toml = "0.5"
 url = "2.1"
 wait-timeout = "0.2"
 xz2 = "0.1.3"
-pgp = { version = "0.5", default-features = false}
 
 [dependencies.retry]
-version = "1"
 default-features = false
+version = "1"
 
 [dependencies.rs_tracing]
-version = "1.0.1"
 features = ["rs_tracing"]
+version = "1.0.1"
 
 [target.'cfg(not(any(target_os="freebsd", target_os="netbsd")))'.dependencies]
 effective-limits = "0.5.1"
@@ -73,30 +74,30 @@ cc = "1"
 winreg = "0.6"
 
 [target."cfg(windows)".dependencies.winapi]
-version = "0.3"
 features = [
-    "combaseapi",
-    "errhandlingapi",
-    "fileapi",
-    "handleapi",
-    "ioapiset",
-    "jobapi",
-    "jobapi2",
-    "minwindef",
-    "processthreadsapi",
-    "psapi",
-    "shlobj",
-    "shtypes",
-    "synchapi",
-    "sysinfoapi",
-    "tlhelp32",
-    "userenv",
-    "winbase",
-    "winerror",
-    "winioctl",
-    "winnt",
-    "winuser",
+  "combaseapi",
+  "errhandlingapi",
+  "fileapi",
+  "handleapi",
+  "ioapiset",
+  "jobapi",
+  "jobapi2",
+  "minwindef",
+  "processthreadsapi",
+  "psapi",
+  "shlobj",
+  "shtypes",
+  "synchapi",
+  "sysinfoapi",
+  "tlhelp32",
+  "userenv",
+  "winbase",
+  "winerror",
+  "winioctl",
+  "winnt",
+  "winuser",
 ]
+version = "0.3"
 
 [dev-dependencies]
 walkdir = "2"
@@ -109,8 +110,8 @@ name = "rustup"
 path = "src/lib.rs"
 
 [profile.release]
-lto = true
 codegen-units = 1
+lto = true
 
 # Reduce build time by setting proc-macro crates non optimized.
 [profile.release.build-override]

Cargo.toml

@@ -45,7 +45,7 @@ use super::markdown::md;
 use super::term2;
 use crate::dist::dist::{self, Profile, TargetTriple};
 use crate::process;
-use crate::toolchain::DistributableToolchain;
+use crate::toolchain::{DistributableToolchain, Toolchain};
 use crate::utils::utils;
 use crate::utils::Notification;
 use crate::{Cfg, UpdateStatus};
@@ -721,6 +721,39 @@ fn maybe_install_rust(
     quiet: bool,
 ) -> Result<()> {
     let mut cfg = common::set_globals(verbose, quiet)?;
+
+    let toolchain = _install_selection(
+        &mut cfg,
+        toolchain,
+        profile_str,
+        default_host_triple,
+        update_existing_toolchain,
+        components,
+        targets,
+    )?;
+    if let Some(toolchain) = toolchain {
+        if toolchain.exists() {
+            warn!("Updating existing toolchain, profile choice will be ignored");
+        }
+        let distributable = DistributableToolchain::new(&toolchain)?;
+        let status = distributable.install_from_dist(true, false, components, targets)?;
+        let toolchain_str = toolchain.name().to_owned();
+        toolchain.cfg().set_default(&toolchain_str)?;
+        writeln!(process().stdout())?;
+        common::show_channel_update(&toolchain.cfg(), &toolchain_str, Ok(status))?;
+    }
+    Ok(())
+}
+
+fn _install_selection<'a>(
+    cfg: &'a mut Cfg,
+    toolchain_opt: Option<&str>,
+    profile_str: &str,
+    default_host_triple: Option<&str>,
+    update_existing_toolchain: bool,
+    components: &[&str],
+    targets: &[&str],
+) -> Result<Option<Toolchain<'a>>> {
     cfg.set_profile(profile_str)?;
 
     if let Some(default_host_triple) = default_host_triple {
@@ -731,7 +764,7 @@ fn maybe_install_rust(
         info!("default host triple is {}", cfg.get_default_host_triple()?);
     }
 
-    let user_specified_something = toolchain.is_some()
+    let user_specified_something = toolchain_opt.is_some()
         || !targets.is_empty()
         || !components.is_empty()
         || update_existing_toolchain;
@@ -741,7 +774,7 @@ fn maybe_install_rust(
     // a toolchain (updating if it's already present) and then if neither of
     // those are true, we have a user who doesn't mind, and already has an
     // install, so we leave their setup alone.
-    if toolchain == Some("none") {
+    Ok(if toolchain_opt == Some("none") {
         info!("skipping toolchain installation");
         if !components.is_empty() {
             warn!(
@@ -758,28 +791,20 @@ fn maybe_install_rust(
             );
         }
         writeln!(process().stdout())?;
+        None
     } else if user_specified_something || cfg.find_default()?.is_none() {
-        let (toolchain_str, toolchain) = match toolchain {
-            Some(s) => (s.to_owned(), cfg.get_toolchain(s, false)?),
+        Some(match toolchain_opt {
+            Some(s) => cfg.get_toolchain(s, false)?,
             None => match cfg.find_default()? {
-                Some(t) => (t.name().to_owned(), t),
-                None => ("stable".to_owned(), cfg.get_toolchain("stable", false)?),
+                Some(t) => t,
+                None => cfg.get_toolchain("stable", false)?,
             },
-        };
-        if toolchain.exists() {
-            warn!("Updating existing toolchain, profile choice will be ignored");
-        }
-        let distributable = DistributableToolchain::new(&toolchain)?;
-        let status = distributable.install_from_dist(true, false, components, targets)?;
-        cfg.set_default(&toolchain_str)?;
-        writeln!(process().stdout())?;
-        common::show_channel_update(&cfg, &toolchain_str, Ok(status))?;
+        })
     } else {
         info!("updating existing rustup installation - leaving toolchains alone");
         writeln!(process().stdout())?;
-    }
-
-    Ok(())
+        None
+    })
 }
 
 pub fn uninstall(no_prompt: bool) -> Result<utils::ExitCode> {
@@ -1060,3 +1085,59 @@ pub fn cleanup_self_updater() -> Result<()> {
 
     Ok(())
 }
+
+#[cfg(test)]
+mod test {
+    use std::collections::HashMap;
+
+    use crate::cli::common;
+    use crate::dist::dist::ToolchainDesc;
+    use crate::test::with_rustup_home;
+    use crate::{currentprocess, for_host};
+
+    #[test]
+    fn default_toolchain_is_stable() {
+        with_rustup_home(|home| {
+            let mut vars = HashMap::new();
+            home.apply(&mut vars);
+            let tp = Box::new(currentprocess::TestProcess {
+                vars,
+                ..Default::default()
+            });
+            currentprocess::with(tp.clone(), || -> anyhow::Result<()> {
+                // TODO: we could pass in a custom cfg to get notification
+                // callbacks rather than output to the tp sink.
+                let mut cfg = common::set_globals(false, false).unwrap();
+                assert_eq!(
+                    "stable",
+                    super::_install_selection(
+                        &mut cfg,
+                        None,      // No toolchain specified
+                        "default", // default profile
+                        None,
+                        false,
+                        &[],
+                        &[],
+                    )
+                    .unwrap() // result
+                    .unwrap() // option
+                    .name()
+                    .parse::<ToolchainDesc>()
+                    .unwrap()
+                    .channel
+                );
+                Ok(())
+            })?;
+            assert_eq!(
+                for_host!(
+                    r"info: profile set to 'default'
+info: default host triple is {0}
+"
+                ),
+                &String::from_utf8(tp.get_stderr()).unwrap()
+            );
+            Ok(())
+        })
+        .unwrap();
+    }
+}

src/cli/self_update.rs

@@ -537,6 +537,57 @@ mod tests {
         });
     }
 
+    #[test]
+    fn windows_path_delete_key_when_empty() {
+        use std::io;
+        // during uninstall the PATH key may end up empty; if so we should
+        // delete it.
+        let tp = Box::new(currentprocess::TestProcess::default());
+        with_registry_edits(&|| {
+            currentprocess::with(tp.clone(), || {
+                let root = RegKey::predef(HKEY_CURRENT_USER);
+                let environment = root
+                    .open_subkey_with_flags("Environment", KEY_READ | KEY_WRITE)
+                    .unwrap();
+                environment
+                    .set_raw_value(
+                        "PATH",
+                        &RegValue {
+                            bytes: utils::string_to_winreg_bytes("foo"),
+                            vtype: RegType::REG_EXPAND_SZ,
+                        },
+                    )
+                    .unwrap();
+
+                assert_eq!((), super::_apply_new_path(Some("".into())).unwrap());
+
+                let reg_value = environment.get_raw_value("PATH");
+                match reg_value {
+                    Ok(_) => panic!("key not deleted"),
+                    Err(ref e) if e.kind() == io::ErrorKind::NotFound => {}
+                    Err(ref e) => panic!("error {}", e),
+                }
+            })
+        });
+    }
+
+    #[test]
+    fn windows_treat_missing_path_as_empty() {
+        // during install the PATH key may be missing; treat it as empty
+        let tp = Box::new(currentprocess::TestProcess::default());
+        with_registry_edits(&|| {
+            currentprocess::with(tp.clone(), || {
+                let root = RegKey::predef(HKEY_CURRENT_USER);
+                let environment = root
+                    .open_subkey_with_flags("Environment", KEY_READ | KEY_WRITE)
+                    .unwrap();
+                environment.delete_value("PATH").unwrap();
+
+                assert_eq!(Some("".into()), super::get_windows_path_var().unwrap());
+            })
+        });
+    }
+
     #[test]
     fn windows_uninstall_removes_semicolon_from_path_prefix() {
         assert_eq!(

src/cli/self_update/windows.rs

@@ -66,5 +66,6 @@ pub mod fallback_settings;
 mod install;
 mod notifications;
 pub mod settings;
+pub mod test;
 pub mod toolchain;
 pub mod utils;