Better tests for non unicode strings

Author: kellda

src/cli/self_update/test.rs

@@ -19,7 +19,7 @@ pub fn get_path() -> Option<RegValue> {
         Ok(val) => Some(val),
         Err(ref e) if e.kind() == std::io::ErrorKind::NotFound => None,
         Err(e) => panic!(
-            "Error getting PATH: {}\nBetter abort to avoid trahsing it.",
+            "Error getting PATH: {}\nBetter abort to avoid trashing it.",
             e
         ),
     }

src/cli/self_update/windows.rs

@@ -437,37 +437,22 @@ mod tests {
     }
 
     #[test]
-    fn windows_doesnt_mess_with_a_non_string_path() {
-        // This writes an error, so we want a sink for it.
-        let tp = Box::new(currentprocess::TestProcess {
-            vars: [("HOME".to_string(), "/unused".to_string())]
-                .iter()
-                .cloned()
-                .collect(),
-            ..Default::default()
-        });
-        with_saved_path(&|| {
-            currentprocess::with(tp.clone(), || {
-                let root = RegKey::predef(HKEY_CURRENT_USER);
-                let environment = root
-                    .open_subkey_with_flags("Environment", KEY_READ | KEY_WRITE)
-                    .unwrap();
-                let reg_value = RegValue {
-                    bytes: vec![0x00, 0xD8, 0x01, 0x01, 0x00, 0x00],
-                    vtype: RegType::REG_BINARY,
-                };
-                environment.set_raw_value("PATH", &reg_value).unwrap();
-                // Ok(None) signals no change to the PATH setting layer
-                assert_eq!(
-                    None,
-                    super::_with_path_cargo_home_bin(|_, _| panic!("called")).unwrap()
-                );
-            })
-        });
+    fn windows_handle_non_unicode_path() {
+        let initial_path = vec![
+            0xD800, // leading surrogate
+            0x0101, // bogus trailing surrogate
+            0x0000, // null
+        ];
+        let cargo_home = wide(r"c:\users\example\.cargo\bin");
+        let final_path = [&cargo_home, &[b';' as u16][..], &initial_path].join(&[][..]);
+
         assert_eq!(
-            r"warning: the registry key HKEY_CURRENT_USER\Environment\PATH is not a string. Not modifying the PATH variable
-",
-            String::from_utf8(tp.get_stderr()).unwrap()
+            &final_path,
+            &super::_add_to_path(initial_path.clone(), cargo_home.clone(),).unwrap()
+        );
+        assert_eq!(
+            &initial_path,
+            &super::_remove_from_path(final_path, cargo_home,).unwrap()
         );
     }
 
@@ -536,6 +521,41 @@ mod tests {
         });
     }
 
+    #[test]
+    fn windows_doesnt_mess_with_a_non_string_path() {
+        // This writes an error, so we want a sink for it.
+        let tp = Box::new(currentprocess::TestProcess {
+            vars: [("HOME".to_string(), "/unused".to_string())]
+                .iter()
+                .cloned()
+                .collect(),
+            ..Default::default()
+        });
+        with_saved_path(&|| {
+            currentprocess::with(tp.clone(), || {
+                let root = RegKey::predef(HKEY_CURRENT_USER);
+                let environment = root
+                    .open_subkey_with_flags("Environment", KEY_READ | KEY_WRITE)
+                    .unwrap();
+                let reg_value = RegValue {
+                    bytes: vec![0x12, 0x34],
+                    vtype: RegType::REG_BINARY,
+                };
+                environment.set_raw_value("PATH", &reg_value).unwrap();
+                // Ok(None) signals no change to the PATH setting layer
+                assert_eq!(
+                    None,
+                    super::_with_path_cargo_home_bin(|_, _| panic!("called")).unwrap()
+                );
+            })
+        });
+        assert_eq!(
+            r"warning: the registry key HKEY_CURRENT_USER\Environment\PATH is not a string. Not modifying the PATH variable
+",
+            String::from_utf8(tp.get_stderr()).unwrap()
+        );
+    }
+
     #[test]
     fn windows_treat_missing_path_as_empty() {
         // during install the PATH key may be missing; treat it as empty

tests/cli-paths.rs

@@ -309,7 +309,6 @@ mod windows {
     use crate::mock::clitools::{self, expect_ok, Scenario};
 
     #[test]
-    #[cfg(windows)]
     /// Smoke test for end-to-end code connectivity of the installer path mgmt on windows.
     fn install_uninstall_affect_path() {
         clitools::setup(Scenario::Empty, &|config| {
@@ -330,4 +329,49 @@ mod windows {
             })
         });
     }
+
+    #[test]
+    /// Smoke test for end-to-end code connectivity of the installer path mgmt on windows.
+    fn install_uninstall_affect_path_with_non_unicode() {
+        use std::ffi::OsString;
+        use std::os::windows::ffi::OsStrExt;
+        use winreg::enums::{RegType, HKEY_CURRENT_USER, KEY_READ, KEY_WRITE};
+        use winreg::{RegKey, RegValue};
+
+        clitools::setup(Scenario::Empty, &|config| {
+            with_saved_path(&|| {
+                // Set up a non unicode PATH
+                let reg_value = RegValue {
+                    bytes: vec![
+                        0x00, 0xD8, // leading surrogate
+                        0x01, 0x01, // bogus trailing surrogate
+                        0x00, 0x00, // null
+                    ],
+                    vtype: RegType::REG_EXPAND_SZ,
+                };
+                RegKey::predef(HKEY_CURRENT_USER)
+                    .open_subkey_with_flags("Environment", KEY_READ | KEY_WRITE)
+                    .unwrap()
+                    .set_raw_value("PATH", &reg_value)
+                    .unwrap();
+
+                // compute expected path after installation
+                let expected = RegValue {
+                    bytes: OsString::from(config.cargodir.join("bin"))
+                        .encode_wide()
+                        .flat_map(|v| vec![v as u8, (v >> 8) as u8])
+                        .chain(vec![b';', 0])
+                        .chain(reg_value.bytes.iter().copied())
+                        .collect(),
+                    vtype: RegType::REG_EXPAND_SZ,
+                };
+
+                expect_ok(config, &INIT_NONE);
+                assert_eq!(get_path().unwrap(), expected);
+
+                expect_ok(config, &["rustup", "self", "uninstall", "-y"]);
+                assert_eq!(get_path().unwrap(), reg_value);
+            })
+        });
+    }
 }