fix(test): stabilize flaky network-dependent tests (#1923)

fengmk2 · web-flow · commit 8f58ac92b1cf · 2026-06-24T20:13:56.000+08:00
## Summary

- retry the complete HTTP body and JSON decode operation for npm
metadata and the Node.js version index
- keep version classification and package-manager priority unit tests
deterministic instead of reaching live registries
- allow the first Vite config evaluation more time on slow Windows CI
workers

## Validation

- `cargo fmt --all -- --check`
- `cargo clippy -p vite_install -p vite_js_runtime -p vite_global_cli
--all-targets -- -D warnings`
- `env -u NODE_OPTIONS RUST_MIN_STACK=8388608 cargo test -p vite_install
-p vite_js_runtime --locked`
- `cargo test -p vite_global_cli test_classify_version --locked`
- `git diff --check`

The focused TypeScript test requires generated Vite/Core/CLI build
artifacts that are not present in a fresh worktree; CI will run it after
the normal build setup.
diff --git a/crates/vite_global_cli/src/commands/env/exec.rs b/crates/vite_global_cli/src/commands/env/exec.rs
@@ -155,30 +155,43 @@ async fn execute_with_version(
 ///
 /// Handles aliases (lts, latest) and version ranges.
 async fn resolve_version(version: &str, provider: &NodeProvider) -> Result<String, Error> {
-    match version.to_lowercase().as_str() {
-        "lts" => {
+    match classify_version(version) {
+        VersionSelector::LatestLts => {
             let resolved = provider.resolve_latest_version().await?;
             Ok(resolved.to_string())
         }
-        "latest" => {
+        VersionSelector::AbsoluteLatest => {
             let resolved = provider.resolve_absolute_latest_version().await?;
             Ok(resolved.to_string())
         }
-        _ => {
-            // For exact versions, use directly
-            if NodeProvider::is_exact_version(version) {
-                // Strip v prefix if present
-                let normalized = version.strip_prefix('v').unwrap_or(version);
-                Ok(normalized.to_string())
-            } else {
-                // For ranges/partial versions, resolve to exact
-                let resolved = provider.resolve_version(version).await?;
-                Ok(resolved.to_string())
-            }
+        VersionSelector::Exact(version) => Ok(version.to_string()),
+        VersionSelector::Range(version) => {
+            let resolved = provider.resolve_version(version).await?;
+            Ok(resolved.to_string())
         }
     }
 }
 
+#[derive(Debug, PartialEq, Eq)]
+enum VersionSelector<'a> {
+    LatestLts,
+    AbsoluteLatest,
+    Exact(&'a str),
+    Range(&'a str),
+}
+
+fn classify_version(version: &str) -> VersionSelector<'_> {
+    if version.eq_ignore_ascii_case("lts") {
+        VersionSelector::LatestLts
+    } else if version.eq_ignore_ascii_case("latest") {
+        VersionSelector::AbsoluteLatest
+    } else if NodeProvider::is_exact_version(version) {
+        VersionSelector::Exact(version.strip_prefix('v').unwrap_or(version))
+    } else {
+        VersionSelector::Range(version)
+    }
+}
+
 /// Create an exit status with the given code.
 fn exit_status(code: i32) -> ExitStatus {
     #[cfg(unix)]
@@ -232,32 +245,21 @@ mod tests {
         assert_eq!(version, "20.18.0");
     }
 
-    #[tokio::test]
-    async fn test_resolve_version_partial() {
-        let provider = NodeProvider::new();
-        let version = resolve_version("20", &provider).await.unwrap();
-        // Should resolve to a 20.x.x version - check starts with "20."
-        assert!(version.starts_with("20."), "Expected version starting with '20.', got: {version}");
+    #[test]
+    fn test_classify_version_partial() {
+        assert_eq!(classify_version("20"), VersionSelector::Range("20"));
     }
 
-    #[tokio::test]
-    async fn test_resolve_version_range() {
-        let provider = NodeProvider::new();
-        let version = resolve_version("^20.0.0", &provider).await.unwrap();
-        // Should resolve to a 20.x.x version - check starts with "20."
-        assert!(version.starts_with("20."), "Expected version starting with '20.', got: {version}");
+    #[test]
+    fn test_classify_version_range() {
+        assert_eq!(classify_version("^20.0.0"), VersionSelector::Range("^20.0.0"));
     }
 
-    #[tokio::test]
-    async fn test_resolve_version_lts() {
-        let provider = NodeProvider::new();
-        let version = resolve_version("lts", &provider).await.unwrap();
-        // Should resolve to a valid version (format: x.y.z)
-        let parts: Vec<&str> = version.split('.').collect();
-        assert_eq!(parts.len(), 3, "Expected version format x.y.z, got: {version}");
-        // Major version should be >= 20 (current LTS line)
-        let major: u32 = parts[0].parse().expect("Major version should be a number");
-        assert!(major >= 20, "Expected major version >= 20, got: {major}");
+    #[test]
+    fn test_classify_version_aliases() {
+        assert_eq!(classify_version("lts"), VersionSelector::LatestLts);
+        assert_eq!(classify_version("LTS"), VersionSelector::LatestLts);
+        assert_eq!(classify_version("latest"), VersionSelector::AbsoluteLatest);
     }
 
     #[tokio::test]
diff --git a/crates/vite_install/src/package_manager.rs b/crates/vite_install/src/package_manager.rs
@@ -3247,8 +3247,8 @@ mod tests {
         assert_eq!(package_json["name"].as_str().unwrap(), "test-package");
     }
 
-    #[tokio::test]
-    async fn test_detect_package_manager_priority_order_lock_over_config() {
+    #[test]
+    fn test_detect_package_manager_priority_order_lock_over_config() {
         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"}"#;
@@ -3272,14 +3272,19 @@ mod tests {
         fs::write(temp_dir_path.join("package-lock.json"), r#"{"lockfileVersion": 3}"#)
             .expect("Failed to write package-lock.json");
 
-        let result = PackageManager::builder(temp_dir_path)
-            .build()
-            .await
-            .expect("Should detect npm from package-lock.json");
+        let (workspace_root, _) =
+            find_workspace_root(&temp_dir_path).expect("Should find workspace root");
+        let (package_manager_type, version, hash, source) =
+            get_package_manager_type_and_version(&workspace_root, None)
+                .expect("Should detect npm from package-lock.json");
         assert_eq!(
-            result.bin_name, "npm",
+            package_manager_type,
+            PackageManagerType::Npm,
             "package-lock.json should take precedence over pnpmfile.cjs and yarn.config.cjs"
         );
+        assert_eq!(version, "latest");
+        assert_eq!(hash, None);
+        assert_eq!(source, PackageManagerSource::LockfileOrConfig);
     }
 
     #[tokio::test]
diff --git a/crates/vite_install/src/request.rs b/crates/vite_install/src/request.rs
@@ -94,11 +94,7 @@ impl HttpClient {
     /// * `Ok(T)` - Deserialized JSON data
     /// * `Err(e)` - If the request fails or JSON deserialization fails
     pub async fn get_json<T: DeserializeOwned>(&self, url: &str) -> Result<T, Error> {
-        tracing::debug!("Fetching JSON from: {}", url);
-
-        let response = self.get(url).await?;
-        let data = response.json::<T>().await?;
-        Ok(data)
+        self.get_json_with_optional_accept(url, None).await
     }
 
     /// Get JSON data from a URL with a custom Accept header
@@ -119,11 +115,32 @@ impl HttpClient {
         url: &str,
         accept: &str,
     ) -> Result<T, Error> {
-        tracing::debug!("Fetching JSON from: {} (accept: {})", url, accept);
+        self.get_json_with_optional_accept(url, Some(accept)).await
+    }
+
+    async fn get_json_with_optional_accept<T: DeserializeOwned>(
+        &self,
+        url: &str,
+        accept: Option<&str>,
+    ) -> Result<T, Error> {
+        tracing::debug!("Fetching JSON from: {} (accept: {:?})", url, accept);
 
-        let response = self.get_with_accept(url, Some(accept)).await?;
-        let data = response.json::<T>().await?;
-        Ok(data)
+        let client = vite_shared::shared_http_client();
+        (|| async {
+            let mut request = client.get(url);
+            if let Some(accept) = accept {
+                request = request.header(reqwest::header::ACCEPT, accept);
+            }
+            let response = request.send().await?.error_for_status()?;
+            Ok::<T, Error>(response.json::<T>().await?)
+        })
+        .retry(
+            ExponentialBuilder::default()
+                .with_jitter()
+                .with_min_delay(Duration::from_millis(self.min_delay))
+                .with_max_times(self.max_times),
+        )
+        .await
     }
 
     /// Download a file to a specified path
@@ -814,15 +831,16 @@ mod tests {
         let server = MockServer::start();
 
         // Mock response with invalid JSON
-        server.mock(|when, then| {
+        let mock = server.mock(|when, then| {
             when.method(GET).path("/invalid.json");
             then.status(200).header("content-type", "application/json").body("not valid json");
         });
 
-        let client = HttpClient::new();
+        let client = HttpClient::with_config(2, 1);
         let url = format!("{}/invalid.json", server.base_url());
 
         let result: Result<TestData, _> = client.get_json(&url).await;
         assert!(result.is_err(), "Expected JSON parsing to fail");
+        mock.assert_hits(3);
     }
 }
diff --git a/crates/vite_js_runtime/src/download.rs b/crates/vite_js_runtime/src/download.rs
@@ -8,6 +8,7 @@ use std::{fs::File, io::IsTerminal, time::Duration};
 use backon::{ExponentialBuilder, Retryable};
 use futures_util::StreamExt;
 use indicatif::{ProgressBar, ProgressStyle};
+use serde::de::DeserializeOwned;
 use sha2::{Digest, Sha256};
 use tokio::{fs, io::AsyncWriteExt};
 use vite_path::{AbsolutePath, AbsolutePathBuf};
@@ -16,10 +17,9 @@ use vite_str::Str;
 use crate::{Error, provider::ArchiveFormat};
 
 /// Response from a cached fetch operation
-pub struct CachedFetchResponse {
-    /// Response body (None if 304 Not Modified)
-    #[expect(clippy::disallowed_types, reason = "HTTP response body is a String")]
-    pub body: Option<String>,
+pub struct CachedFetchResponse<T> {
+    /// Deserialized response body (None if 304 Not Modified)
+    pub body: Option<T>,
     /// `ETag` header value
     pub etag: Option<Str>,
     /// Cache max-age in seconds (from Cache-Control header)
@@ -164,26 +164,60 @@ pub async fn download_text(url: &str) -> Result<String, Error> {
     Ok(content)
 }
 
-/// Fetch text with conditional request support
+/// Fetch JSON with conditional request support.
 ///
 /// If `if_none_match` is provided, sends `If-None-Match` header for conditional request.
-/// Returns response with cache headers and `not_modified` flag.
-pub async fn fetch_with_cache_headers(
+/// The request, response body, and JSON decoding are retried as one operation so a
+/// truncated body cannot escape the retry boundary as a deserialization error.
+pub async fn fetch_json_with_cache_headers<T: DeserializeOwned>(
     url: &str,
     if_none_match: Option<&str>,
-) -> Result<CachedFetchResponse, Error> {
+) -> Result<CachedFetchResponse<T>, Error> {
     let client = vite_shared::shared_http_client();
 
     tracing::debug!("Fetching with cache headers from {url}");
 
-    let response = (|| async {
+    (|| async {
         let mut request = client.get(url);
 
         if let Some(etag) = if_none_match {
             request = request.header("If-None-Match", etag);
         }
 
-        request.send().await
+        let response = request.send().await?.error_for_status()?;
+
+        if response.status() == reqwest::StatusCode::NOT_MODIFIED {
+            tracing::debug!("Received 304 Not Modified for {url}");
+            return Ok(CachedFetchResponse {
+                body: None,
+                etag: None,
+                max_age: None,
+                not_modified: true,
+            });
+        }
+
+        // Extract headers before consuming the response.
+        let etag = response
+            .headers()
+            .get("etag")
+            .and_then(|v| v.to_str().ok())
+            .map(std::convert::Into::into);
+
+        let max_age = response
+            .headers()
+            .get("cache-control")
+            .and_then(|v| v.to_str().ok())
+            .and_then(parse_max_age);
+
+        let bytes = response.bytes().await?;
+        let body = serde_json::from_slice(&bytes)?;
+
+        Ok::<CachedFetchResponse<T>, Error>(CachedFetchResponse {
+            body: Some(body),
+            etag,
+            max_age,
+            not_modified: false,
+        })
     })
     .retry(
         ExponentialBuilder::default()
@@ -195,35 +229,7 @@ pub async fn fetch_with_cache_headers(
     .map_err(|e| Error::DownloadFailed {
         url: url.into(),
         reason: vite_shared::format_error_chain(&e).into(),
-    })?;
-
-    // Check for 304 Not Modified
-    if response.status() == reqwest::StatusCode::NOT_MODIFIED {
-        tracing::debug!("Received 304 Not Modified for {url}");
-        return Ok(CachedFetchResponse {
-            body: None,
-            etag: None,
-            max_age: None,
-            not_modified: true,
-        });
-    }
-
-    // Extract headers before consuming response
-    let etag =
-        response.headers().get("etag").and_then(|v| v.to_str().ok()).map(std::convert::Into::into);
-
-    let max_age = response
-        .headers()
-        .get("cache-control")
-        .and_then(|v| v.to_str().ok())
-        .and_then(parse_max_age);
-
-    let body = response.text().await.map_err(|e| Error::DownloadFailed {
-        url: url.into(),
-        reason: vite_shared::format_error_chain(&e).into(),
-    })?;
-
-    Ok(CachedFetchResponse { body: Some(body), etag, max_age, not_modified: false })
+    })
 }
 
 /// Parse max-age from Cache-Control header value
diff --git a/crates/vite_js_runtime/src/providers/node.rs b/crates/vite_js_runtime/src/providers/node.rs
@@ -13,7 +13,7 @@ use vite_str::Str;
 use crate::provider::ShasumsSignature;
 use crate::{
     Error, Platform,
-    download::fetch_with_cache_headers,
+    download::fetch_json_with_cache_headers,
     platform::Os,
     provider::{ArchiveFormat, DownloadInfo, HashVerification, JsRuntimeProvider},
 };
@@ -237,7 +237,8 @@ impl NodeProvider {
         let base_url = get_dist_url();
         let index_url = vite_str::format!("{base_url}/index.json");
 
-        let response = fetch_with_cache_headers(&index_url, Some(etag)).await?;
+        let response =
+            fetch_json_with_cache_headers::<Vec<NodeVersionEntry>>(&index_url, Some(etag)).await?;
 
         if response.not_modified {
             // Server confirmed data hasn't changed, refresh TTL
@@ -252,10 +253,9 @@ impl NodeProvider {
         }
 
         // Got new data
-        let body = response.body.ok_or_else(|| Error::VersionIndexParseFailed {
+        let versions = response.body.ok_or_else(|| Error::VersionIndexParseFailed {
             reason: "Empty response body".into(),
         })?;
-        let versions: Vec<NodeVersionEntry> = serde_json::from_str(&body)?;
 
         let new_cache = VersionIndexCache {
             expires_at: calculate_expires_at(response.max_age),
@@ -276,12 +276,12 @@ impl NodeProvider {
         let index_url = vite_str::format!("{base_url}/index.json");
 
         tracing::debug!("Fetching version index from {index_url}");
-        let response = fetch_with_cache_headers(&index_url, None).await?;
+        let response =
+            fetch_json_with_cache_headers::<Vec<NodeVersionEntry>>(&index_url, None).await?;
 
-        let body = response.body.ok_or_else(|| Error::VersionIndexParseFailed {
+        let versions = response.body.ok_or_else(|| Error::VersionIndexParseFailed {
             reason: "Empty response body".into(),
         })?;
-        let versions: Vec<NodeVersionEntry> = serde_json::from_str(&body)?;
 
         let cache = VersionIndexCache {
             expires_at: calculate_expires_at(response.max_age),
diff --git a/packages/cli/src/create/__tests__/register-template.spec.ts b/packages/cli/src/create/__tests__/register-template.spec.ts
@@ -60,6 +60,7 @@ describe('registerLocalTemplate', () => {
     return config.create ?? {};
   }
 
+  // The first Vite config evaluation can cold-start slowly on Windows CI.
   it('creates a vite.config.ts with create.templates when none exists', async () => {
     expect(fs.existsSync(path.join(workspaceRoot, 'vite.config.ts'))).toBe(false);
 
@@ -69,7 +70,7 @@ describe('registerLocalTemplate', () => {
     const create = await readCreate();
     expect(create.defaultTemplate).toBeUndefined();
     expect(create.templates).toEqual([ENTRY_A]);
-  });
+  }, 15_000);
 
   it('targets an existing vite.config.mts instead of creating a stray vite.config.ts', async () => {
     // A monorepo whose only config is a .mts (or .cts/.cjs) file must be the
