From c47974677ef76122a59d8bce7bc93aaf51c1dc32 Mon Sep 17 00:00:00 2001
From: David Sherret <dsherret@users.noreply.github.com>
Date: Tue, 22 Mar 2022 13:37:15 -0400
Subject: [PATCH] fix(tests): do not use global env vars in install tests
 (#14078)

---
 cli/tests/integration/compile_tests.rs |  40 ++++---
 cli/tests/integration/install_tests.rs |  81 ++++++++++++++
 cli/tools/installer.rs                 | 141 +++++--------------------
 3 files changed, 124 insertions(+), 138 deletions(-)

diff --git a/cli/tests/integration/compile_tests.rs b/cli/tests/integration/compile_tests.rs
index a6cceb8a7b..40bb250a06 100644
--- a/cli/tests/integration/compile_tests.rs
+++ b/cli/tests/integration/compile_tests.rs
@@ -7,7 +7,7 @@ use test_util as util;
 
 #[test]
 fn compile() {
-  let dir = TempDir::new().expect("tempdir fail");
+  let dir = TempDir::new().unwrap();
   let exe = if cfg!(windows) {
     dir.path().join("welcome.exe")
   } else {
@@ -36,12 +36,11 @@ fn compile() {
   assert_eq!(output.stdout, "Welcome to Deno!\n".as_bytes());
 }
 
-#[ignore]
 #[test]
 #[cfg(windows)]
 // https://github.com/denoland/deno/issues/9667
 fn compile_windows_ext() {
-  let dir = TempDir::new().expect("tempdir fail");
+  let dir = TempDir::new().unwrap();
   let exe = dir.path().join("welcome_9667");
   let output = util::deno_cmd()
     .current_dir(util::root_path())
@@ -65,7 +64,7 @@ fn compile_windows_ext() {
 
 #[test]
 fn standalone_args() {
-  let dir = TempDir::new().expect("tempdir fail");
+  let dir = TempDir::new().unwrap();
   let exe = if cfg!(windows) {
     dir.path().join("args.exe")
   } else {
@@ -101,7 +100,7 @@ fn standalone_args() {
 
 #[test]
 fn standalone_error() {
-  let dir = TempDir::new().expect("tempdir fail");
+  let dir = TempDir::new().unwrap();
   let exe = if cfg!(windows) {
     dir.path().join("error.exe")
   } else {
@@ -143,7 +142,7 @@ fn standalone_error() {
 
 #[test]
 fn standalone_error_module_with_imports() {
-  let dir = TempDir::new().expect("tempdir fail");
+  let dir = TempDir::new().unwrap();
   let exe = if cfg!(windows) {
     dir.path().join("error.exe")
   } else {
@@ -183,7 +182,7 @@ fn standalone_error_module_with_imports() {
 
 #[test]
 fn standalone_load_datauri() {
-  let dir = TempDir::new().expect("tempdir fail");
+  let dir = TempDir::new().unwrap();
   let exe = if cfg!(windows) {
     dir.path().join("load_datauri.exe")
   } else {
@@ -215,7 +214,7 @@ fn standalone_load_datauri() {
 
 #[test]
 fn standalone_compiler_ops() {
-  let dir = TempDir::new().expect("tempdir fail");
+  let dir = TempDir::new().unwrap();
   let exe = if cfg!(windows) {
     dir.path().join("standalone_compiler_ops.exe")
   } else {
@@ -247,7 +246,7 @@ fn standalone_compiler_ops() {
 
 #[test]
 fn compile_with_directory_output_flag() {
-  let dir = TempDir::new().expect("tempdir fail");
+  let dir = TempDir::new().unwrap();
   let output_path = if cfg!(windows) {
     dir.path().join(r"args\random\")
   } else {
@@ -285,14 +284,14 @@ fn compile_with_directory_output_flag() {
 
 #[test]
 fn compile_with_file_exists_error() {
-  let dir = TempDir::new().expect("tempdir fail");
+  let dir = TempDir::new().unwrap();
   let output_path = if cfg!(windows) {
     dir.path().join(r"args\")
   } else {
     dir.path().join("args/")
   };
   let file_path = dir.path().join("args");
-  File::create(&file_path).expect("cannot create file");
+  File::create(&file_path).unwrap();
   let output = util::deno_cmd()
     .current_dir(util::testdata_path())
     .arg("compile")
@@ -320,13 +319,13 @@ fn compile_with_file_exists_error() {
 
 #[test]
 fn compile_with_directory_exists_error() {
-  let dir = TempDir::new().expect("tempdir fail");
+  let dir = TempDir::new().unwrap();
   let exe = if cfg!(windows) {
     dir.path().join("args.exe")
   } else {
     dir.path().join("args")
   };
-  std::fs::create_dir(&exe).expect("cannot create directory");
+  std::fs::create_dir(&exe).unwrap();
   let output = util::deno_cmd()
     .current_dir(util::testdata_path())
     .arg("compile")
@@ -354,14 +353,13 @@ fn compile_with_directory_exists_error() {
 
 #[test]
 fn compile_with_conflict_file_exists_error() {
-  let dir = TempDir::new().expect("tempdir fail");
+  let dir = TempDir::new().unwrap();
   let exe = if cfg!(windows) {
     dir.path().join("args.exe")
   } else {
     dir.path().join("args")
   };
-  std::fs::write(&exe, b"SHOULD NOT BE OVERWRITTEN")
-    .expect("cannot create file");
+  std::fs::write(&exe, b"SHOULD NOT BE OVERWRITTEN").unwrap();
   let output = util::deno_cmd()
     .current_dir(util::testdata_path())
     .arg("compile")
@@ -387,13 +385,13 @@ fn compile_with_conflict_file_exists_error() {
   dbg!(&stderr);
   assert!(stderr.contains(&expected_stderr));
   assert!(std::fs::read(&exe)
-    .expect("cannot read file")
+    .unwrap()
     .eq(b"SHOULD NOT BE OVERWRITTEN"));
 }
 
 #[test]
 fn compile_and_overwrite_file() {
-  let dir = TempDir::new().expect("tempdir fail");
+  let dir = TempDir::new().unwrap();
   let exe = if cfg!(windows) {
     dir.path().join("args.exe")
   } else {
@@ -431,7 +429,7 @@ fn compile_and_overwrite_file() {
 
 #[test]
 fn standalone_runtime_flags() {
-  let dir = TempDir::new().expect("tempdir fail");
+  let dir = TempDir::new().unwrap();
   let exe = if cfg!(windows) {
     dir.path().join("flags.exe")
   } else {
@@ -470,7 +468,7 @@ fn standalone_runtime_flags() {
 
 #[test]
 fn standalone_import_map() {
-  let dir = TempDir::new().expect("tempdir fail");
+  let dir = TempDir::new().unwrap();
   let exe = if cfg!(windows) {
     dir.path().join("import_map.exe")
   } else {
@@ -505,7 +503,7 @@ fn standalone_import_map() {
 #[test]
 // https://github.com/denoland/deno/issues/12670
 fn skip_rebundle() {
-  let dir = TempDir::new().expect("tempdir fail");
+  let dir = TempDir::new().unwrap();
   let exe = if cfg!(windows) {
     dir.path().join("hello_world.exe")
   } else {
diff --git a/cli/tests/integration/install_tests.rs b/cli/tests/integration/install_tests.rs
index 68f519686d..18f0cf9c04 100644
--- a/cli/tests/integration/install_tests.rs
+++ b/cli/tests/integration/install_tests.rs
@@ -1,9 +1,90 @@
 // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
 
+use std::fs;
 use std::process::Command;
 use tempfile::TempDir;
 use test_util as util;
 
+#[test]
+fn install_basic() {
+  let _guard = util::http_server();
+  let temp_dir = TempDir::new().unwrap();
+  let temp_dir_str = temp_dir.path().to_string_lossy().to_string();
+
+  let status = util::deno_cmd()
+    .current_dir(temp_dir.path())
+    .arg("install")
+    .arg("--name")
+    .arg("echo_test")
+    .arg("http://localhost:4545/echo.ts")
+    .envs([
+      ("HOME", temp_dir_str.as_str()),
+      ("USERPROFILE", temp_dir_str.as_str()),
+      ("DENO_INSTALL_ROOT", ""),
+    ])
+    .spawn()
+    .unwrap()
+    .wait()
+    .unwrap();
+  assert!(status.success());
+
+  let mut file_path = temp_dir.path().join(".deno/bin/echo_test");
+  assert!(file_path.exists());
+
+  if cfg!(windows) {
+    file_path = file_path.with_extension("cmd");
+  }
+
+  let content = fs::read_to_string(file_path).unwrap();
+  // ensure there's a trailing newline so the shell script can be
+  // more versatile.
+  assert_eq!(content.chars().last().unwrap(), '\n');
+
+  if cfg!(windows) {
+    assert!(content.contains(r#""run" "http://localhost:4545/echo.ts""#));
+  } else {
+    assert!(content.contains(r#"run 'http://localhost:4545/echo.ts'"#));
+  }
+}
+
+#[test]
+fn install_custom_dir_env_var() {
+  let _guard = util::http_server();
+  let temp_dir = TempDir::new().unwrap();
+  let temp_dir_str = temp_dir.path().to_string_lossy().to_string();
+
+  let status = util::deno_cmd()
+    .current_dir(util::root_path()) // different cwd
+    .arg("install")
+    .arg("--name")
+    .arg("echo_test")
+    .arg("http://localhost:4545/echo.ts")
+    .envs([
+      ("HOME", temp_dir_str.as_str()),
+      ("USERPROFILE", temp_dir_str.as_str()),
+      ("DENO_INSTALL_ROOT", temp_dir_str.as_str()),
+    ])
+    .spawn()
+    .unwrap()
+    .wait()
+    .unwrap();
+  assert!(status.success());
+
+  let mut file_path = temp_dir.path().join("bin/echo_test");
+  assert!(file_path.exists());
+
+  if cfg!(windows) {
+    file_path = file_path.with_extension("cmd");
+  }
+
+  let content = fs::read_to_string(file_path).unwrap();
+  if cfg!(windows) {
+    assert!(content.contains(r#""run" "http://localhost:4545/echo.ts""#));
+  } else {
+    assert!(content.contains(r#"run 'http://localhost:4545/echo.ts'"#));
+  }
+}
+
 #[test]
 fn installer_test_local_module_run() {
   let temp_dir = TempDir::new().expect("tempdir fail");
diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs
index 8318c72bf7..22119f7312 100644
--- a/cli/tools/installer.rs
+++ b/cli/tools/installer.rs
@@ -397,14 +397,11 @@ fn is_in_path(dir: &Path) -> bool {
 #[cfg(test)]
 mod tests {
   use super::*;
-  use deno_core::parking_lot::Mutex;
-  use once_cell::sync::Lazy;
+
   use std::process::Command;
   use tempfile::TempDir;
   use test_util::testdata_path;
 
-  pub static ENV_LOCK: Lazy<Mutex<()>> = Lazy::new(|| Mutex::new(()));
-
   #[test]
   fn install_infer_name_from_url() {
     assert_eq!(
@@ -478,67 +475,9 @@ mod tests {
     );
   }
 
-  #[test]
-  fn install_basic() {
-    let _guard = ENV_LOCK.lock();
-    let temp_dir = TempDir::new().expect("tempdir fail");
-    let temp_dir_str = temp_dir.path().to_string_lossy().to_string();
-    // NOTE: this test overrides environmental variables
-    // don't add other tests in this file that mess with "HOME" and "USEPROFILE"
-    // otherwise transient failures are possible because tests are run in parallel.
-    // It means that other test can override env vars when this test is running.
-    let original_home = env::var_os("HOME");
-    let original_user_profile = env::var_os("HOME");
-    let original_install_root = env::var_os("DENO_INSTALL_ROOT");
-    env::set_var("HOME", &temp_dir_str);
-    env::set_var("USERPROFILE", &temp_dir_str);
-    env::set_var("DENO_INSTALL_ROOT", "");
-
-    install(
-      Flags::default(),
-      InstallFlags {
-        module_url: "http://localhost:4545/echo_server.ts".to_string(),
-        args: vec![],
-        name: Some("echo_test".to_string()),
-        root: None,
-        force: false,
-      },
-    )
-    .expect("Install failed");
-
-    if let Some(home) = original_home {
-      env::set_var("HOME", home);
-    }
-    if let Some(user_profile) = original_user_profile {
-      env::set_var("USERPROFILE", user_profile);
-    }
-    if let Some(install_root) = original_install_root {
-      env::set_var("DENO_INSTALL_ROOT", install_root);
-    }
-
-    let mut file_path = temp_dir.path().join(".deno/bin/echo_test");
-    assert!(file_path.exists());
-
-    if cfg!(windows) {
-      file_path = file_path.with_extension("cmd");
-    }
-
-    let content = fs::read_to_string(file_path).unwrap();
-    // It's annoying when shell scripts don't have NL at the end.
-    assert_eq!(content.chars().last().unwrap(), '\n');
-
-    if cfg!(windows) {
-      assert!(
-        content.contains(r#""run" "http://localhost:4545/echo_server.ts""#)
-      );
-    } else {
-      assert!(content.contains(r#"run 'http://localhost:4545/echo_server.ts'"#));
-    }
-  }
-
   #[test]
   fn install_unstable() {
-    let temp_dir = TempDir::new().expect("tempdir fail");
+    let temp_dir = TempDir::new().unwrap();
     let bin_dir = temp_dir.path().join("bin");
     std::fs::create_dir(&bin_dir).unwrap();
 
@@ -555,7 +494,7 @@ mod tests {
         force: false,
       },
     )
-    .expect("Install failed");
+    .unwrap();
 
     let mut file_path = bin_dir.join("echo_test");
     if cfg!(windows) {
@@ -639,42 +578,6 @@ mod tests {
     );
   }
 
-  #[test]
-  fn install_custom_dir_env_var() {
-    let _guard = ENV_LOCK.lock();
-    let temp_dir = TempDir::new().expect("tempdir fail");
-    let bin_dir = temp_dir.path().join("bin");
-    std::fs::create_dir(&bin_dir).unwrap();
-    let original_install_root = env::var_os("DENO_INSTALL_ROOT");
-    env::set_var("DENO_INSTALL_ROOT", temp_dir.path());
-
-    let shim_data = resolve_shim_data(
-      &Flags::default(),
-      &InstallFlags {
-        module_url: "http://localhost:4545/echo_server.ts".to_string(),
-        args: vec![],
-        name: Some("echo_test".to_string()),
-        root: None,
-        force: false,
-      },
-    )
-    .unwrap();
-
-    if let Some(install_root) = original_install_root {
-      env::set_var("DENO_INSTALL_ROOT", install_root);
-    }
-
-    assert_eq!(
-      fs::canonicalize(shim_data.installation_dir).unwrap(),
-      fs::canonicalize(bin_dir).unwrap()
-    );
-    assert_eq!(shim_data.name, "echo_test");
-    assert_eq!(
-      shim_data.args,
-      vec!["run", "http://localhost:4545/echo_server.ts",]
-    );
-  }
-
   #[test]
   fn install_with_flags() {
     let shim_data = resolve_shim_data(
@@ -758,7 +661,7 @@ mod tests {
 
   #[test]
   fn install_local_module() {
-    let temp_dir = TempDir::new().expect("tempdir fail");
+    let temp_dir = TempDir::new().unwrap();
     let bin_dir = temp_dir.path().join("bin");
     std::fs::create_dir(&bin_dir).unwrap();
     let local_module = env::current_dir().unwrap().join("echo_server.ts");
@@ -775,7 +678,7 @@ mod tests {
         force: false,
       },
     )
-    .expect("Install failed");
+    .unwrap();
 
     let mut file_path = bin_dir.join("echo_test");
     if cfg!(windows) {
@@ -789,7 +692,7 @@ mod tests {
 
   #[test]
   fn install_force() {
-    let temp_dir = TempDir::new().expect("tempdir fail");
+    let temp_dir = TempDir::new().unwrap();
     let bin_dir = temp_dir.path().join("bin");
     std::fs::create_dir(&bin_dir).unwrap();
 
@@ -803,7 +706,7 @@ mod tests {
         force: false,
       },
     )
-    .expect("Install failed");
+    .unwrap();
 
     let mut file_path = bin_dir.join("echo_test");
     if cfg!(windows) {
@@ -850,7 +753,7 @@ mod tests {
 
   #[test]
   fn install_with_config() {
-    let temp_dir = TempDir::new().expect("tempdir fail");
+    let temp_dir = TempDir::new().unwrap();
     let bin_dir = temp_dir.path().join("bin");
     let config_file_path = temp_dir.path().join("test_tsconfig.json");
     let config = "{}";
@@ -886,7 +789,7 @@ mod tests {
   #[cfg(not(windows))]
   #[test]
   fn install_shell_escaping() {
-    let temp_dir = TempDir::new().expect("tempdir fail");
+    let temp_dir = TempDir::new().unwrap();
     let bin_dir = temp_dir.path().join("bin");
     std::fs::create_dir(&bin_dir).unwrap();
 
@@ -900,7 +803,7 @@ mod tests {
         force: false,
       },
     )
-    .expect("Install failed");
+    .unwrap();
 
     let mut file_path = bin_dir.join("echo_test");
     if cfg!(windows) {
@@ -919,12 +822,9 @@ mod tests {
     }
   }
 
-  // This test is disabled because it uses the `deno` binary found in `$PATH`.
-  // It should use the one located in `./target/{debug|release}/`.
   #[test]
-  #[ignore]
   fn install_unicode() {
-    let temp_dir = TempDir::new().expect("tempdir fail");
+    let temp_dir = TempDir::new().unwrap();
     let bin_dir = temp_dir.path().join("bin");
     std::fs::create_dir(&bin_dir).unwrap();
     let unicode_dir = temp_dir.path().join("Magnús");
@@ -943,7 +843,7 @@ mod tests {
         force: false,
       },
     )
-    .expect("Install failed");
+    .unwrap();
 
     let mut file_path = bin_dir.join("echo_test");
     if cfg!(windows) {
@@ -951,13 +851,20 @@ mod tests {
     }
 
     // We need to actually run it to make sure the URL is interpreted correctly
-    let status = Command::new(file_path).spawn().unwrap().wait().unwrap();
+    let status = Command::new(file_path)
+      .env_clear()
+      // use the deno binary in the target directory
+      .env("PATH", test_util::target_dir())
+      .spawn()
+      .unwrap()
+      .wait()
+      .unwrap();
     assert!(status.success());
   }
 
   #[test]
   fn install_with_import_map() {
-    let temp_dir = TempDir::new().expect("tempdir fail");
+    let temp_dir = TempDir::new().unwrap();
     let bin_dir = temp_dir.path().join("bin");
     let import_map_path = temp_dir.path().join("import_map.json");
     let import_map_url = Url::from_file_path(&import_map_path).unwrap();
@@ -1005,7 +912,7 @@ mod tests {
   // Regression test for https://github.com/denoland/deno/issues/10556.
   #[test]
   fn install_file_url() {
-    let temp_dir = TempDir::new().expect("tempdir fail");
+    let temp_dir = TempDir::new().unwrap();
     let bin_dir = temp_dir.path().join("bin");
     let module_path = fs::canonicalize(testdata_path().join("cat.ts")).unwrap();
     let file_module_string =
@@ -1041,7 +948,7 @@ mod tests {
 
   #[test]
   fn uninstall_basic() {
-    let temp_dir = TempDir::new().expect("tempdir fail");
+    let temp_dir = TempDir::new().unwrap();
     let bin_dir = temp_dir.path().join("bin");
     std::fs::create_dir(&bin_dir).unwrap();
 
@@ -1059,7 +966,7 @@ mod tests {
     File::create(&file_path).unwrap();
 
     uninstall("echo_test".to_string(), Some(temp_dir.path().to_path_buf()))
-      .expect("Uninstall failed");
+      .unwrap();
 
     assert!(!file_path.exists());
     assert!(!file_path.with_extension("tsconfig.json").exists());