chore: maybe make watcher tests less flaky (#23683)

Closes #23637

2 files changed, 76 insertions, 61 deletions

diff --git a/tests/util/server/src/fs.rs b/tests/util/server/src/fs.rs
index b9ae81b49..558d25ffb 100644
--- a/tests/util/server/src/fs.rs
+++ b/tests/util/server/src/fs.rs
@@ -150,24 +150,29 @@ impl PathRef {
     .unwrap_or_else(|| panic!("JSON file was empty for {}", self))
   }
 
+  #[track_caller]
   pub fn rename(&self, to: impl AsRef<Path>) {
     fs::rename(self, self.join(to)).unwrap();
   }
 
+  #[track_caller]
   pub fn append(&self, text: impl AsRef<str>) {
     let mut file = OpenOptions::new().append(true).open(self).unwrap();
     file.write_all(text.as_ref().as_bytes()).unwrap();
   }
 
+  #[track_caller]
   pub fn write(&self, text: impl AsRef<[u8]>) {
     fs::write(self, text).unwrap();
   }
 
+  #[track_caller]
   pub fn write_json<TValue: Serialize>(&self, value: &TValue) {
     let text = serde_json::to_string_pretty(value).unwrap();
     self.write(text);
   }
 
+  #[track_caller]
   pub fn symlink_dir(
     &self,
     oldpath: impl AsRef<Path>,
@@ -187,6 +192,7 @@ impl PathRef {
     }
   }
 
+  #[track_caller]
   pub fn symlink_file(
     &self,
     oldpath: impl AsRef<Path>,
@@ -206,12 +212,14 @@ impl PathRef {
     }
   }
 
+  #[track_caller]
   pub fn read_dir(&self) -> fs::ReadDir {
     fs::read_dir(self.as_path())
       .with_context(|| format!("Reading {}", self.as_path().display()))
       .unwrap()
   }
 
+  #[track_caller]
   pub fn copy(&self, to: &impl AsRef<Path>) {
     std::fs::copy(self.as_path(), to)
       .with_context(|| format!("Copying {} to {}", self, to.as_ref().display()))
@@ -247,6 +255,7 @@ impl PathRef {
     }
   }
 
+  #[track_caller]
   pub fn make_dir_readonly(&self) {
     self.create_dir_all();
     if cfg!(windows) {
diff --git a/tests/util/server/src/lib.rs b/tests/util/server/src/lib.rs
index febbb2066..c1046c528 100644
--- a/tests/util/server/src/lib.rs
+++ b/tests/util/server/src/lib.rs
@@ -1,14 +1,5 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
-// Usage: provide a port as argument to run hyper_hello benchmark server
-// otherwise this starts multiple servers on many ports for test endpoints.
-use futures::FutureExt;
-use futures::Stream;
-use futures::StreamExt;
-use once_cell::sync::Lazy;
-use pretty_assertions::assert_eq;
-use pty::Pty;
-use regex::Regex;
-use serde::Serialize;
+
 use std::collections::HashMap;
 use std::env;
 use std::io::Write;
@@ -18,8 +9,17 @@ use std::process::Command;
 use std::process::Output;
 use std::process::Stdio;
 use std::result::Result;
-use std::sync::Mutex;
-use std::sync::MutexGuard;
+
+use futures::FutureExt;
+use futures::Stream;
+use futures::StreamExt;
+use once_cell::sync::Lazy;
+use parking_lot::Mutex;
+use parking_lot::MutexGuard;
+use pretty_assertions::assert_eq;
+use pty::Pty;
+use regex::Regex;
+use serde::Serialize;
 use tokio::net::TcpStream;
 use url::Url;
 
@@ -47,8 +47,7 @@ pub const PERMISSION_VARIANTS: [&str; 5] =
   ["read", "write", "env", "net", "run"];
 pub const PERMISSION_DENIED_PATTERN: &str = "PermissionDenied";
 
-static GUARD: Lazy<Mutex<HttpServerCount>> =
-  Lazy::new(|| Mutex::new(HttpServerCount::default()));
+static GUARD: Lazy<Mutex<HttpServerCount>> = Lazy::new(Default::default);
 
 pub fn env_vars_for_npm_tests() -> Vec<(String, String)> {
   vec![
@@ -308,41 +307,14 @@ async fn get_tcp_listener_stream(
 #[derive(Default)]
 struct HttpServerCount {
   count: usize,
-  test_server: Option<Child>,
+  test_server: Option<HttpServerStarter>,
 }
 
 impl HttpServerCount {
   fn inc(&mut self) {
     self.count += 1;
     if self.test_server.is_none() {
-      assert_eq!(self.count, 1);
-
-      println!("test_server starting...");
-      let mut test_server = Command::new(test_server_path())
-        .current_dir(testdata_path())
-        .stdout(Stdio::piped())
-        .spawn()
-        .expect("failed to execute test_server");
-      let stdout = test_server.stdout.as_mut().unwrap();
-      use std::io::BufRead;
-      use std::io::BufReader;
-      let lines = BufReader::new(stdout).lines();
-
-      // Wait for all the servers to report being ready.
-      let mut ready_count = 0;
-      for maybe_line in lines {
-        if let Ok(line) = maybe_line {
-          if line.starts_with("ready:") {
-            ready_count += 1;
-          }
-          if ready_count == 12 {
-            break;
-          }
-        } else {
-          panic!("{}", maybe_line.unwrap_err());
-        }
-      }
-      self.test_server = Some(test_server);
+      self.test_server = Some(Default::default());
     }
   }
 
@@ -350,17 +322,7 @@ impl HttpServerCount {
     assert!(self.count > 0);
     self.count -= 1;
     if self.count == 0 {
-      let mut test_server = self.test_server.take().unwrap();
-      match test_server.try_wait() {
-        Ok(None) => {
-          test_server.kill().expect("failed to kill test_server");
-          let _ = test_server.wait();
-        }
-        Ok(Some(status)) => {
-          panic!("test_server exited unexpectedly {status}")
-        }
-        Err(e) => panic!("test_server error: {e}"),
-      }
+      self.test_server.take();
     }
   }
 }
@@ -372,16 +334,60 @@ impl Drop for HttpServerCount {
   }
 }
 
-fn lock_http_server<'a>() -> MutexGuard<'a, HttpServerCount> {
-  let r = GUARD.lock();
-  if let Err(poison_err) = r {
-    // If panics happened, ignore it. This is for tests.
-    poison_err.into_inner()
-  } else {
-    r.unwrap()
+struct HttpServerStarter {
+  test_server: Child,
+}
+
+impl Default for HttpServerStarter {
+  fn default() -> Self {
+    println!("test_server starting...");
+    let mut test_server = Command::new(test_server_path())
+      .current_dir(testdata_path())
+      .stdout(Stdio::piped())
+      .spawn()
+      .expect("failed to execute test_server");
+    let stdout = test_server.stdout.as_mut().unwrap();
+    use std::io::BufRead;
+    use std::io::BufReader;
+    let lines = BufReader::new(stdout).lines();
+
+    // Wait for all the servers to report being ready.
+    let mut ready_count = 0;
+    for maybe_line in lines {
+      if let Ok(line) = maybe_line {
+        if line.starts_with("ready:") {
+          ready_count += 1;
+        }
+        if ready_count == 12 {
+          break;
+        }
+      } else {
+        panic!("{}", maybe_line.unwrap_err());
+      }
+    }
+    Self { test_server }
   }
 }
 
+impl Drop for HttpServerStarter {
+  fn drop(&mut self) {
+    match self.test_server.try_wait() {
+      Ok(None) => {
+        self.test_server.kill().expect("failed to kill test_server");
+        let _ = self.test_server.wait();
+      }
+      Ok(Some(status)) => {
+        panic!("test_server exited unexpectedly {status}")
+      }
+      Err(e) => panic!("test_server error: {e}"),
+    }
+  }
+}
+
+fn lock_http_server<'a>() -> MutexGuard<'a, HttpServerCount> {
+  GUARD.lock()
+}
+
 pub struct HttpServerGuard {}
 
 impl Drop for HttpServerGuard {