From bffcc19d276380f9f0decbc4091c91cec11027b1 Mon Sep 17 00:00:00 2001 From: MK Date: Tue, 23 Jun 2026 20:32:51 +0800 Subject: [PATCH] fix(test): stabilize flaky network-dependent tests --- .../vite_global_cli/src/commands/env/exec.rs | 74 ++++++++-------- crates/vite_install/src/package_manager.rs | 19 +++-- crates/vite_install/src/request.rs | 40 ++++++--- crates/vite_js_runtime/src/download.rs | 84 ++++++++++--------- crates/vite_js_runtime/src/providers/node.rs | 14 ++-- .../__tests__/register-template.spec.ts | 3 +- 6 files changed, 133 insertions(+), 101 deletions(-) diff --git a/crates/vite_global_cli/src/commands/env/exec.rs b/crates/vite_global_cli/src/commands/env/exec.rs index 2ca43bfc98..abe75fb2e3 100644 --- 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 { - 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 index 5d8aa1c1ff..5a9d1eecb0 100644 --- 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 index 79e1cda18e..900f9f4133 100644 --- 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(&self, url: &str) -> Result { - tracing::debug!("Fetching JSON from: {}", url); - - let response = self.get(url).await?; - let data = response.json::().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 { - 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( + &self, + url: &str, + accept: Option<&str>, + ) -> Result { + tracing::debug!("Fetching JSON from: {} (accept: {:?})", url, accept); - let response = self.get_with_accept(url, Some(accept)).await?; - let data = response.json::().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::(response.json::().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 = 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 index 22eaafc84d..5f28bf6f45 100644 --- 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, +pub struct CachedFetchResponse { + /// Deserialized response body (None if 304 Not Modified) + pub body: Option, /// `ETag` header value pub etag: Option, /// Cache max-age in seconds (from Cache-Control header) @@ -164,26 +164,60 @@ pub async fn download_text(url: &str) -> Result { 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( url: &str, if_none_match: Option<&str>, -) -> Result { +) -> Result, 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::, 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 index ad84867fa8..88f9650d6c 100644 --- 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::>(&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 = 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::>(&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 = 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 index 37ad22f627..b281781087 100644 --- 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