chore: share HTTP server between tests

cli/installer.rs

@@ -204,6 +204,10 @@ mod tests {
   fn install_basic() {
     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");
     env::set_var("HOME", &temp_dir_str);
@@ -322,23 +326,18 @@ mod tests {
   #[test]
   fn install_force() {
     let temp_dir = TempDir::new().expect("tempdir fail");
-    let temp_dir_str = temp_dir.path().to_string_lossy().to_string();
-    let original_home = env::var_os("HOME");
-    let original_user_profile = env::var_os("HOME");
-    env::set_var("HOME", &temp_dir_str);
-    env::set_var("USERPROFILE", &temp_dir_str);
 
     install(
       DenoFlags::default(),
-      None,
+      Some(temp_dir.path().to_path_buf()),
       "echo_test",
       "http://localhost:4545/cli/tests/echo_server.ts",
       vec![],
       false,
     )
     .expect("Install failed");
 
-    let mut file_path = temp_dir.path().join(".deno/bin/echo_test");
+    let mut file_path = temp_dir.path().join("echo_test");
     if cfg!(windows) {
       file_path = file_path.with_extension(".cmd");
     }
@@ -347,7 +346,7 @@ mod tests {
     // No force. Install failed.
     let no_force_result = install(
       DenoFlags::default(),
-      None,
+      Some(temp_dir.path().to_path_buf()),
       "echo_test",
       "http://localhost:4545/cli/tests/cat.ts", // using a different URL
       vec![],
@@ -365,7 +364,7 @@ mod tests {
     // Force. Install success.
     let force_result = install(
       DenoFlags::default(),
-      None,
+      Some(temp_dir.path().to_path_buf()),
       "echo_test",
       "http://localhost:4545/cli/tests/cat.ts", // using a different URL
       vec![],
@@ -375,12 +374,5 @@ mod tests {
     // Assert modified
     let file_content_2 = fs::read_to_string(&file_path).unwrap();
     assert!(file_content_2.contains("cat.ts"));
-
-    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);
-    }
   }
 }

cli/test_util.rs

@@ -8,11 +8,13 @@ use std::path::PathBuf;
 use std::process::Child;
 use std::process::Command;
 use std::process::Stdio;
+use std::sync::atomic::AtomicUsize;
+use std::sync::atomic::Ordering;
 use std::sync::Mutex;
-use std::sync::MutexGuard;
 
 lazy_static! {
-  static ref GUARD: Mutex<()> = Mutex::new(());
+  static ref SERVER: Mutex<Option<Child>> = Mutex::new(None);
+  static ref SERVER_COUNT: AtomicUsize = AtomicUsize::new(0);
 }
 
 pub fn root_path() -> PathBuf {
@@ -35,45 +37,56 @@ pub fn deno_exe_path() -> PathBuf {
   p
 }
 
-pub struct HttpServerGuard<'a> {
-  #[allow(dead_code)]
-  g: MutexGuard<'a, ()>,
-  child: Child,
-}
+pub struct HttpServerGuard {}
 
-impl<'a> Drop for HttpServerGuard<'a> {
+impl Drop for HttpServerGuard {
   fn drop(&mut self) {
-    match self.child.try_wait() {
-      Ok(None) => {
-        self.child.kill().expect("failed to kill http_server.py");
-      }
-      Ok(Some(status)) => {
-        panic!("http_server.py exited unexpectedly {}", status)
-      }
-      Err(e) => panic!("http_server.py err {}", e),
+    let count = SERVER_COUNT.fetch_sub(1, Ordering::SeqCst);
+    // If no more tests hold guard we can kill the server
+
+    if count == 1 {
+      kill_http_server();
+    }
+  }
+}
+
+fn kill_http_server() {
+  let mut server_guard = SERVER.lock().unwrap();
+  let mut child = server_guard
+    .take()
+    .expect("Trying to kill server but already killed");
+  match child.try_wait() {
+    Ok(None) => {
+      child.kill().expect("failed to kill http_server.py");
     }
+    Ok(Some(status)) => panic!("http_server.py exited unexpectedly {}", status),
+    Err(e) => panic!("http_server.py error: {}", e),
   }
+  drop(server_guard);
 }
 
-/// Starts tools/http_server.py when the returned guard is dropped, the server
-/// will be killed.
-pub fn http_server<'a>() -> HttpServerGuard<'a> {
-  // TODO(ry) Allow tests to use the http server in parallel.
-  let g = GUARD.lock().unwrap();
+pub fn http_server() -> HttpServerGuard {
+  SERVER_COUNT.fetch_add(1, Ordering::SeqCst);
 
-  println!("tools/http_server.py starting...");
-  let mut child = Command::new("python")
-    .current_dir(root_path())
-    .args(&["-u", "tools/http_server.py"])
-    .stdout(Stdio::piped())
-    .spawn()
-    .expect("failed to execute child");
+  {
+    let mut server_guard = SERVER.lock().unwrap();
+    if server_guard.is_none() {
+      println!("tools/http_server.py starting...");
+      let mut child = Command::new("python")
+        .current_dir(root_path())
+        .args(&["-u", "tools/http_server.py"])
+        .stdout(Stdio::piped())
+        .spawn()
+        .expect("failed to execute child");
 
-  let stdout = child.stdout.as_mut().unwrap();
-  use std::io::{BufRead, BufReader};
-  let mut lines = BufReader::new(stdout).lines();
-  let line = lines.next().unwrap().unwrap();
-  assert!(line.starts_with("ready"));
+      let stdout = child.stdout.as_mut().unwrap();
+      use std::io::{BufRead, BufReader};
+      let mut lines = BufReader::new(stdout).lines();
+      let line = lines.next().unwrap().unwrap();
+      assert!(line.starts_with("ready"));
+      server_guard.replace(child);
+    }
+  }
 
-  HttpServerGuard { child, g }
+  HttpServerGuard {}
 }