Fix unstable package_manager tests - mock network requests and resolve race conditions

crates/vite_install/src/package_manager.rs

@@ -381,27 +381,65 @@ async fn download_package_manager(
     }
 
     // rename $target_dir_tmp to $target_dir
+    // We don't remove target_dir first to avoid race conditions where another thread
+    // might be in the middle of creating it. Instead, we rely on the rename operation
+    // to fail if the directory already exists, which we handle gracefully.
     tracing::debug!("Rename {:?} to {:?}", target_dir_tmp, target_dir);
-    remove_dir_all_force(&target_dir).await?;
-    tokio::fs::rename(&target_dir_tmp, &target_dir).await?;
-
-    // create shim file
-    tracing::debug!("Create shim files for {}", bin_name);
-    create_shim_files(package_manager_type, &bin_prefix).await?;
-
-    Ok(install_dir)
+
+    match tokio::fs::rename(&target_dir_tmp, &target_dir).await {
+        Ok(()) => {
+            // create shim file
+            tracing::debug!("Create shim files for {}", bin_name);
+            create_shim_files(package_manager_type, &bin_prefix).await?;
+            Ok(install_dir)
+        }
+        Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {
+            // Another concurrent process created the directory - that's OK
+            tracing::debug!("Target directory already exists (concurrent creation), using it");
+            // Clean up the temporary directory we don't need anymore
+            // Use remove_dir_all_force to handle potential concurrent cleanup attempts
+            if let Err(cleanup_err) = remove_dir_all_force(&target_dir_tmp).await {
+                tracing::warn!(
+                    "Failed to clean up temporary directory {:?}: {}",
+                    target_dir_tmp,
+                    cleanup_err
+                );
+            }
+            // Ensure shim files exist - they should have been created by the other process,
+            // but create them if they're missing to ensure a complete installation
+            if !is_exists_file(&bin_file)?
+                || !is_exists_file(bin_file.with_extension("cmd"))?
+                || !is_exists_file(bin_file.with_extension("ps1"))?
+            {
+                tracing::debug!("Shim files missing, creating them");
+                create_shim_files(package_manager_type, &bin_prefix).await?;
+            }
+            Ok(install_dir)
+        }
+        Err(e) => Err(e.into()),
+    }
 }
 
 /// Remove the directory and all its contents.
-/// Ignore the error if the directory is not found.
+/// Ignore the error if the directory is not found or already being removed by another process.
 async fn remove_dir_all_force(path: impl AsRef<Path>) -> Result<(), std::io::Error> {
     match remove_dir_all(path).await {
         Ok(()) => Ok(()),
         Err(e) => {
-            if e.kind() == std::io::ErrorKind::NotFound {
-                Ok(())
-            } else {
-                Err(e)
+            // Ignore errors that can occur during concurrent test execution
+            // Using ErrorKind for cross-platform compatibility
+            match e.kind() {
+                // Directory was already removed by another process
+                std::io::ErrorKind::NotFound => {
+                    tracing::debug!("Ignoring NotFound error (concurrent removal): {}", e);
+                    Ok(())
+                }
+                // Another process is actively using/removing the directory
+                std::io::ErrorKind::DirectoryNotEmpty => {
+                    tracing::debug!("Ignoring DirectoryNotEmpty error (concurrent access): {}", e);
+                    Ok(())
+                }
+                _ => Err(e),
             }
         }
     }
@@ -571,8 +609,11 @@ pub(crate) async fn run_command(
 
 #[cfg(test)]
 mod tests {
+    //! Tests for package manager detection and installation.
+
     use std::fs;
 
+    use httpmock::prelude::*;
     use tempfile::{TempDir, tempdir};
 
     use super::*;
@@ -590,6 +631,100 @@ mod tests {
             .expect("Failed to write pnpm-workspace.yaml");
     }
 
+    /// Helper to create a mock package tar.gz for testing
+    fn create_mock_package_tgz(package_manager_type: PackageManagerType) -> Vec<u8> {
+        use flate2::write::GzEncoder;
+        use flate2::Compression;
+        use std::io::Write;
+        use tar::Builder;
+
+        let bin_name = package_manager_type.to_string();
+        let mut tar_builder = Builder::new(Vec::new());
+
+        // Add package.json
+        let package_json = format!(r#"{{"name":"{}","version":"1.0.0"}}"#, bin_name);
+        let mut header = tar::Header::new_gnu();
+        header.set_size(package_json.len() as u64);
+        header.set_mode(0o644);
+        tar_builder
+            .append_data(&mut header, "package/package.json", package_json.as_bytes())
+            .unwrap();
+
+        // Add bin file
+        let bin_content = format!("#!/usr/bin/env node\nconsole.log('mock {}');", bin_name);
+        let mut header = tar::Header::new_gnu();
+        header.set_size(bin_content.len() as u64);
+        header.set_mode(0o755);
+        tar_builder
+            .append_data(
+                &mut header,
+                &format!("package/bin/{}", bin_name),
+                bin_content.as_bytes(),
+            )
+            .unwrap();
+
+        let tar_data = tar_builder.into_inner().unwrap();
+
+        // Compress with gzip
+        let mut gz_encoder = GzEncoder::new(Vec::new(), Compression::default());
+        gz_encoder.write_all(&tar_data).unwrap();
+        gz_encoder.finish().unwrap()
+    }
+
+    /// Helper to setup a mock npm registry server for testing
+    /// Returns the MockServer instance
+    fn setup_mock_registry() -> MockServer {
+        MockServer::start()
+    }
+
+    /// Helper to mock a package download endpoint
+    fn mock_package_download(
+        server: &MockServer,
+        package_name: &str,
+        version: &str,
+        package_manager_type: PackageManagerType,
+    ) -> Mock {
+        let filename = package_name.split('/').next_back().unwrap_or(package_name);
+        let path = format!("/{}/-/{}-{}.tgz", package_name, filename, version);
+        let mock_tgz = create_mock_package_tgz(package_manager_type);
+
+        server.mock(|when, then| {
+            when.method(GET).path(&path);
+            then.status(200)
+                .header("content-type", "application/octet-stream")
+                .body(mock_tgz);
+        })
+    }
+
+    /// Helper to mock a 404 response for a non-existent package version
+    fn mock_package_not_found(server: &MockServer, package_name: &str, version: &str) -> Mock {
+        let filename = package_name.split('/').next_back().unwrap_or(package_name);
+        let path = format!("/{}/-/{}-{}.tgz", package_name, filename, version);
+
+        server.mock(|when, then| {
+            when.method(GET).path(&path);
+            then.status(404).body("Not Found");
+        })
+    }
+
+    /// Helper to mock the package version endpoint (e.g., /package_name/latest)
+    fn mock_package_version(
+        server: &MockServer,
+        package_name: &str,
+        version_or_tag: &str,
+        actual_version: &str,
+    ) -> Mock {
+        let path = format!("/{}/{}", package_name, version_or_tag);
+        let response_json = format!(r#"{{"version":"{}"}}"#, actual_version);
+
+        server.mock(|when, then| {
+            when.method(GET).path(&path);
+            then.status(200)
+                .header("content-type", "application/json")
+                .body(response_json);
+        })
+    }
+
     #[test]
     fn test_find_package_root() {
         let temp_dir = create_temp_dir();
@@ -867,18 +1002,27 @@ mod tests {
 
     #[tokio::test]
     async fn test_detect_package_manager_with_pnpm_workspace_yaml() {
+        let server = setup_mock_registry();
         let temp_dir = create_temp_dir();
         let temp_dir_path = AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap();
         let workspace_content = "packages:\n  - 'packages/*'";
         create_pnpm_workspace_yaml(&temp_dir_path, workspace_content);
 
+        // Mock pnpm package download (uses "latest" version by default)
+        mock_package_version(&server, "pnpm", "latest", "9.0.0");
+        mock_package_download(&server, "pnpm", "9.0.0", PackageManagerType::Pnpm);
+
+        std::env::set_var("NPM_CONFIG_REGISTRY", &server.base_url());
         let result =
             PackageManager::builder(temp_dir_path).build().await.expect("Should detect pnpm");
+        std::env::remove_var("NPM_CONFIG_REGISTRY");
+
         assert_eq!(result.bin_name, "pnpm");
     }
 
     #[tokio::test]
     async fn test_detect_package_manager_with_pnpm_lock_yaml() {
+        let server = setup_mock_registry();
         let temp_dir = create_temp_dir();
         let temp_dir_path = AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap();
         let package_content = r#"{"name": "test-package", "version": "1.0.0"}"#;
@@ -888,8 +1032,15 @@ mod tests {
         fs::write(temp_dir_path.join("pnpm-lock.yaml"), "lockfileVersion: '6.0'")
             .expect("Failed to write pnpm-lock.yaml");
 
+        // Mock pnpm package download
+        mock_package_version(&server, "pnpm", "latest", "9.0.0");
+        mock_package_download(&server, "pnpm", "9.0.0", PackageManagerType::Pnpm);
+
+        std::env::set_var("NPM_CONFIG_REGISTRY", &server.base_url());
         let result =
             PackageManager::builder(temp_dir_path).build().await.expect("Should detect pnpm");
+        std::env::remove_var("NPM_CONFIG_REGISTRY");
+
         assert_eq!(result.bin_name, "pnpm");
 
         // check if the package.json file has the `packageManager` field
@@ -940,6 +1091,7 @@ mod tests {
     async fn test_detect_package_manager_with_package_lock_json() {
         use std::process::Command;
 
+        let server = setup_mock_registry();
         let temp_dir = create_temp_dir();
         let temp_dir_path = AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap();
         let package_content = r#"{"name": "test-package"}"#;
@@ -949,8 +1101,15 @@ mod tests {
         fs::write(temp_dir_path.join("package-lock.json"), r#"{"lockfileVersion": 2}"#)
             .expect("Failed to write package-lock.json");
 
+        // Mock npm package download
+        mock_package_version(&server, "npm", "latest", "10.0.0");
+        mock_package_download(&server, "npm", "10.0.0", PackageManagerType::Npm);
+
+        std::env::set_var("NPM_CONFIG_REGISTRY", &server.base_url());
         let result =
             PackageManager::builder(temp_dir_path).build().await.expect("Should detect npm");
+        std::env::remove_var("NPM_CONFIG_REGISTRY");
+
         assert_eq!(result.bin_name, "npm");
 
         // check shim files
@@ -1170,25 +1329,31 @@ mod tests {
 
     #[tokio::test]
     async fn test_download_failed_package_manager_with_hash() {
+        let server = setup_mock_registry();
         let temp_dir = create_temp_dir();
         let temp_dir_path = AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap();
         let package_content = r#"{"name": "test-package", "packageManager": "yarn@1.22.21+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e"}"#;
         create_package_json(&temp_dir_path, package_content);
 
+        // Mock yarn package download (yarn 1.x uses "yarn" as package name)
+        // This will provide a package with a different hash than expected
+        mock_package_download(&server, "yarn", "1.22.21", PackageManagerType::Yarn);
+
+        std::env::set_var("NPM_CONFIG_REGISTRY", &server.base_url());
         let result = PackageManager::builder(temp_dir_path).build().await;
+        std::env::remove_var("NPM_CONFIG_REGISTRY");
+
         assert!(result.is_err());
         // Check if it's the expected error type
         if let Err(Error::HashMismatch { expected, actual }) = result {
             assert_eq!(
                 expected,
                 "sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e"
             );
-            assert_eq!(
-                actual,
-                "sha512.ca75da26c00327d26267ce33536e5790f18ebd53266796fbb664d2a4a5116308042dd8ee7003b276a20eace7d3c5561c3577bdd71bcb67071187af124779620a"
-            );
+            // The actual hash will be from our mock package, so we just verify it's different
+            assert_ne!(expected, actual);
         } else {
-            panic!("Expected HashMismatch error");
+            panic!("Expected HashMismatch error, got {result:?}");
         }
     }
 
@@ -1269,15 +1434,22 @@ mod tests {
 
     #[tokio::test]
     async fn test_detect_package_manager_with_npm_package_manager_field() {
+        let server = setup_mock_registry();
         let temp_dir = create_temp_dir();
         let temp_dir_path = AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap();
         let package_content = r#"{"name": "test-package", "packageManager": "npm@10.0.0"}"#;
         create_package_json(&temp_dir_path, package_content);
 
+        // Mock npm package download
+        mock_package_download(&server, "npm", "10.0.0", PackageManagerType::Npm);
+
+        std::env::set_var("NPM_CONFIG_REGISTRY", &server.base_url());
         let result = PackageManager::builder(temp_dir_path)
             .build()
             .await
             .expect("Should detect npm with version");
+        std::env::remove_var("NPM_CONFIG_REGISTRY");
+
         assert_eq!(result.bin_name, "npm");
     }
 
@@ -1300,13 +1472,25 @@ mod tests {
 
     #[tokio::test]
     async fn test_detect_package_manager_with_not_exists_version_in_package_manager_field() {
+        let server = setup_mock_registry();
         let temp_dir = create_temp_dir();
         let temp_dir_path = AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap();
         let package_content =
             r#"{"name": "test-package", "packageManager": "yarn@10000000000.0.0"}"#;
         create_package_json(&temp_dir_path, package_content);
 
+        // Mock 404 response for non-existent version
+        // Yarn >= 2.0.0 uses @yarnpkg/cli-dist as package name
+        mock_package_not_found(&server, "@yarnpkg/cli-dist", "10000000000.0.0");
+
+        // Set the npm registry to point to our mock server
+        std::env::set_var("NPM_CONFIG_REGISTRY", &server.base_url());
+
         let result = PackageManager::builder(temp_dir_path).build().await;
+
+        // Clean up env var
+        std::env::remove_var("NPM_CONFIG_REGISTRY");
+
         assert!(result.is_err());
         println!("result: {result:?}");
         // Check if it's the expected error type
@@ -1333,16 +1517,24 @@ mod tests {
 
     #[tokio::test]
     async fn test_detect_package_manager_with_default_fallback() {
+        let server = setup_mock_registry();
         let temp_dir = create_temp_dir();
         let temp_dir_path = AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap();
         let package_content = r#"{"name": "test-package"}"#;
         create_package_json(&temp_dir_path, package_content);
 
+        // Mock yarn package download (yarn >= 2.0.0 uses @yarnpkg/cli-dist)
+        mock_package_version(&server, "@yarnpkg/cli-dist", "latest", "4.0.0");
+        mock_package_download(&server, "@yarnpkg/cli-dist", "4.0.0", PackageManagerType::Yarn);
+
+        std::env::set_var("NPM_CONFIG_REGISTRY", &server.base_url());
         let result = PackageManager::builder(temp_dir_path.clone())
             .package_manager_type(PackageManagerType::Yarn)
             .build()
             .await
             .expect("Should use default");
+        std::env::remove_var("NPM_CONFIG_REGISTRY");
+
         assert_eq!(result.bin_name, "yarn");
         // package.json should have the `packageManager` field
         let package_json_path = temp_dir_path.join("package.json");
@@ -1602,6 +1794,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_detect_package_manager_pnpmfile_over_yarn_config() {
+        let server = setup_mock_registry();
         let temp_dir = create_temp_dir();
         let temp_dir_path = AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap();
         let package_content = r#"{"name": "test-package"}"#;
@@ -1617,11 +1810,18 @@ mod tests {
         )
         .expect("Failed to write yarn.config.cjs");
 
+        // Mock pnpm package download
+        mock_package_version(&server, "pnpm", "latest", "9.0.0");
+        mock_package_download(&server, "pnpm", "9.0.0", PackageManagerType::Pnpm);
+
+        std::env::set_var("NPM_CONFIG_REGISTRY", &server.base_url());
         // pnpmfile.cjs should be detected first (before yarn.config.cjs)
         let result = PackageManager::builder(temp_dir_path)
             .build()
             .await
             .expect("Should detect pnpm from pnpmfile.cjs");
+        std::env::remove_var("NPM_CONFIG_REGISTRY");
+
         assert_eq!(
             result.bin_name, "pnpm",
             "pnpmfile.cjs should be detected before yarn.config.cjs"