diff --git a/cli/installer.rs b/cli/installer.rs index b1a795f99c62c3..76a0243c0a7b8c 100644 --- a/cli/installer.rs +++ b/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,15 +326,10 @@ 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![], @@ -338,7 +337,7 @@ mod tests { ) .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); - } } } diff --git a/cli/test_util.rs b/cli/test_util.rs index 1b1a51e9231e29..9c0307096028a1 100644 --- a/cli/test_util.rs +++ b/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> = 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 {} }