mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-17 10:17:55 +00:00
fix: handle Windows paths in pre-commit tests (#2974)
Co-authored-by: houseme <housemecn@gmail.com>
This commit is contained in:
@@ -82,17 +82,22 @@ impl TryFrom<&str> for Endpoint {
|
||||
#[cfg(not(windows))]
|
||||
let path = Path::new(&path).absolutize()?;
|
||||
|
||||
// On windows having a preceding SlashSeparator will cause problems, if the
|
||||
// command line already has C:/<export-folder/ in it. Final resulting
|
||||
// path on windows might become C:/C:/ this will cause problems
|
||||
// of starting rustfs server properly in distributed mode on windows.
|
||||
// As a special case make sure to trim the separator.
|
||||
#[cfg(windows)]
|
||||
let path = Path::new(&path[1..]).absolutize()?;
|
||||
let path = if has_leading_slash_windows_drive(&path) {
|
||||
// Url::path() exposes file-like Windows paths as `/C:/...`.
|
||||
// Strip only that synthetic leading slash; plain URL paths
|
||||
// such as `/export1` must stay URL paths, not become
|
||||
// relative paths under the current drive.
|
||||
Path::new(&path[1..]).absolutize()?.to_string_lossy().into_owned()
|
||||
} else {
|
||||
path
|
||||
};
|
||||
#[cfg(windows)]
|
||||
let path = Path::new(&path);
|
||||
|
||||
debug!("endpoint try_from: path={}", path.display());
|
||||
|
||||
if path.parent().is_none() || Path::new("").eq(&path) {
|
||||
if path.parent().is_none() || path.as_os_str().is_empty() {
|
||||
return Err(Error::other("empty or root path is not supported in URL endpoint"));
|
||||
}
|
||||
|
||||
@@ -217,6 +222,12 @@ impl Endpoint {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn has_leading_slash_windows_drive(path: &str) -> bool {
|
||||
let bytes = path.as_bytes();
|
||||
bytes.len() >= 4 && bytes[0] == b'/' && bytes[1].is_ascii_alphabetic() && bytes[2] == b':' && bytes[3] == b'/'
|
||||
}
|
||||
|
||||
/// parse a file path into a URL.
|
||||
fn url_parse_from_file_path(value: &str) -> Result<Url> {
|
||||
// Only check if the arg is an ip address and ask for scheme since its absent.
|
||||
@@ -242,6 +253,14 @@ fn url_parse_from_file_path(value: &str) -> Result<Url> {
|
||||
mod test {
|
||||
use super::*;
|
||||
|
||||
fn expected_file_path(path: &str) -> String {
|
||||
Path::new(path).absolutize().unwrap().to_string_lossy().replace('\\', "/")
|
||||
}
|
||||
|
||||
fn expected_file_url(path: &str) -> Url {
|
||||
url_parse_from_file_path(path).unwrap()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_new_endpoint() {
|
||||
#[derive(Default)]
|
||||
@@ -255,7 +274,7 @@ mod test {
|
||||
let u2 = Url::parse("https://example.org/path").unwrap();
|
||||
let u4 = Url::parse("http://192.168.253.200/path").unwrap();
|
||||
let u6 = Url::parse("http://server:/path").unwrap();
|
||||
let root_slash_foo = Url::from_file_path("/foo").unwrap();
|
||||
let root_slash_foo = expected_file_url("/foo");
|
||||
|
||||
let test_cases = [
|
||||
TestCase {
|
||||
@@ -416,7 +435,7 @@ mod test {
|
||||
// Test file path display
|
||||
let file_endpoint = Endpoint::try_from("/tmp/data").unwrap();
|
||||
let display_str = format!("{file_endpoint}");
|
||||
assert_eq!(display_str, "/tmp/data");
|
||||
assert_eq!(display_str, expected_file_path("/tmp/data"));
|
||||
|
||||
// Test URL display
|
||||
let url_endpoint = Endpoint::try_from("http://example.com:9000/path").unwrap();
|
||||
@@ -479,12 +498,25 @@ mod test {
|
||||
#[test]
|
||||
fn test_endpoint_get_file_path() {
|
||||
let file_endpoint = Endpoint::try_from("/tmp/data").unwrap();
|
||||
assert_eq!(file_endpoint.get_file_path(), "/tmp/data");
|
||||
assert_eq!(file_endpoint.get_file_path(), expected_file_path("/tmp/data"));
|
||||
|
||||
let url_endpoint = Endpoint::try_from("http://example.com:9000/path/to/data").unwrap();
|
||||
assert_eq!(url_endpoint.get_file_path(), "/path/to/data");
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn test_windows_url_drive_path_requires_separator_after_colon() {
|
||||
let drive_path_endpoint = Endpoint::try_from("http://host/C:/data").unwrap();
|
||||
assert_eq!(drive_path_endpoint.get_type(), EndpointType::Url);
|
||||
assert!(has_leading_slash_windows_drive(Url::parse("http://host/C:/data").unwrap().path()));
|
||||
|
||||
let url_path_endpoint = Endpoint::try_from("http://host/C:foo").unwrap();
|
||||
assert_eq!(url_path_endpoint.get_type(), EndpointType::Url);
|
||||
assert!(!has_leading_slash_windows_drive(Url::parse("http://host/C:foo").unwrap().path()));
|
||||
assert_eq!(url_path_endpoint.get_file_path(), "/C:foo");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_endpoint_clone_and_equality() {
|
||||
let endpoint1 = Endpoint::try_from("/tmp/data").unwrap();
|
||||
@@ -503,7 +535,7 @@ mod test {
|
||||
// Test with complex paths
|
||||
let complex_path = "/var/lib/rustfs/data/bucket1";
|
||||
let endpoint = Endpoint::try_from(complex_path).unwrap();
|
||||
assert_eq!(endpoint.get_file_path(), complex_path);
|
||||
assert_eq!(endpoint.get_file_path(), expected_file_path(complex_path));
|
||||
assert!(endpoint.is_local);
|
||||
assert_eq!(endpoint.get_type(), EndpointType::Path);
|
||||
}
|
||||
@@ -512,7 +544,7 @@ mod test {
|
||||
fn test_endpoint_with_spaces_in_path() {
|
||||
let path_with_spaces = "/Users/test/Library/Application Support/rustfs/data";
|
||||
let endpoint = Endpoint::try_from(path_with_spaces).unwrap();
|
||||
assert_eq!(endpoint.get_file_path(), path_with_spaces);
|
||||
assert_eq!(endpoint.get_file_path(), expected_file_path(path_with_spaces));
|
||||
assert!(endpoint.is_local);
|
||||
assert_eq!(endpoint.get_type(), EndpointType::Path);
|
||||
}
|
||||
@@ -532,7 +564,7 @@ mod test {
|
||||
// Verify that get_file_path() decodes the percent-encoded path correctly
|
||||
assert_eq!(
|
||||
endpoint.get_file_path(),
|
||||
"/Users/test/Library/Application Support/rustfs/data",
|
||||
expected_file_path("/Users/test/Library/Application Support/rustfs/data"),
|
||||
"get_file_path() should decode percent-encoded spaces"
|
||||
);
|
||||
}
|
||||
@@ -544,7 +576,7 @@ mod test {
|
||||
let endpoint = Endpoint::try_from(path_with_special).unwrap();
|
||||
|
||||
// get_file_path() should return the original path with decoded characters
|
||||
assert_eq!(endpoint.get_file_path(), path_with_special);
|
||||
assert_eq!(endpoint.get_file_path(), expected_file_path(path_with_special));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -545,7 +545,7 @@ mod tests {
|
||||
|
||||
// Create two different files
|
||||
tokio::fs::write(&file1_path, b"content1").await.unwrap();
|
||||
tokio::fs::write(&file2_path, b"content2").await.unwrap();
|
||||
tokio::fs::write(&file2_path, b"different content").await.unwrap();
|
||||
|
||||
// Get metadata
|
||||
let metadata1 = tokio::fs::metadata(&file1_path).await.unwrap();
|
||||
|
||||
@@ -2153,9 +2153,6 @@ impl DiskAPI for LocalDisk {
|
||||
#[allow(unsafe_code)]
|
||||
#[tracing::instrument(level = "debug", skip(self))]
|
||||
async fn read_file_zero_copy(&self, volume: &str, path: &str, offset: usize, length: usize) -> Result<Bytes> {
|
||||
use std::time::Instant;
|
||||
|
||||
let start = Instant::now();
|
||||
let volume_dir = self.get_bucket_path(volume)?;
|
||||
if !skip_access_checks(volume) {
|
||||
access(&volume_dir)
|
||||
@@ -2188,6 +2185,9 @@ impl DiskAPI for LocalDisk {
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use memmap2::MmapOptions;
|
||||
use std::time::Instant;
|
||||
|
||||
let start = Instant::now();
|
||||
let file_path_clone = file_path.clone();
|
||||
|
||||
let should_reclaim_after_read = should_reclaim_file_cache_after_read(length);
|
||||
@@ -2272,8 +2272,7 @@ impl DiskAPI for LocalDisk {
|
||||
f.seek(SeekFrom::Start(offset as u64)).await?;
|
||||
}
|
||||
|
||||
let mut buffer = Vec::with_capacity(length);
|
||||
buffer.resize(length, 0);
|
||||
let mut buffer = vec![0; length];
|
||||
f.read_exact(&mut buffer).await?;
|
||||
|
||||
Ok(Bytes::from(buffer))
|
||||
@@ -3497,10 +3496,10 @@ mod test {
|
||||
let disk_info = disk.disk_info(&disk_info_opts).await.unwrap();
|
||||
|
||||
// Basic checks on disk info
|
||||
// Note: On macOS and some other Unix systems, fs_type may be empty
|
||||
// Note: On macOS, Windows, and some other systems, fs_type may be empty
|
||||
// because statvfs does not provide filesystem type information.
|
||||
// This is a platform limitation, not a bug.
|
||||
#[cfg(not(target_os = "macos"))]
|
||||
#[cfg(not(any(target_os = "macos", windows)))]
|
||||
assert!(!disk_info.fs_type.is_empty(), "fs_type should not be empty on this platform");
|
||||
assert!(disk_info.total > 0);
|
||||
assert!(disk_info.free <= disk_info.total);
|
||||
|
||||
@@ -789,7 +789,6 @@ mod tests {
|
||||
use super::*;
|
||||
use endpoint::Endpoint;
|
||||
use local::LocalDisk;
|
||||
use std::path::PathBuf;
|
||||
use tokio::fs;
|
||||
use uuid::Uuid;
|
||||
|
||||
@@ -1094,7 +1093,7 @@ mod tests {
|
||||
assert!(disk.is_ok());
|
||||
|
||||
let disk = disk.unwrap();
|
||||
assert_eq!(disk.path(), PathBuf::from(test_dir).canonicalize().unwrap());
|
||||
assert_eq!(disk.path(), rustfs_utils::canonicalize(test_dir).unwrap());
|
||||
assert!(disk.is_local());
|
||||
// Note: is_online() might return false for local disks without proper initialization
|
||||
// This is expected behavior for test environments
|
||||
|
||||
@@ -749,6 +749,7 @@ fn validate_local_cross_device_mounts(local_paths: &[String]) -> Result<()> {
|
||||
|
||||
#[cfg(test)]
|
||||
mod test {
|
||||
use path_absolutize::Absolutize;
|
||||
use rustfs_utils::must_get_local_ips;
|
||||
|
||||
use super::*;
|
||||
@@ -1452,9 +1453,10 @@ mod test {
|
||||
}
|
||||
|
||||
fn must_file_path(s: impl AsRef<Path>) -> url::Url {
|
||||
let url = url::Url::from_file_path(s.as_ref());
|
||||
let path = s.as_ref().absolutize().expect("absolute test path");
|
||||
let url = url::Url::from_file_path(&path);
|
||||
|
||||
assert!(url.is_ok(), "failed to convert path to URL: {}", s.as_ref().display());
|
||||
assert!(url.is_ok(), "failed to convert path to URL: {}", path.display());
|
||||
|
||||
url.unwrap()
|
||||
}
|
||||
|
||||
@@ -872,10 +872,7 @@ mod tests {
|
||||
let result = call_peer_with_timeout(
|
||||
Duration::from_millis(5),
|
||||
"peer-3",
|
||||
|| async {
|
||||
tokio::time::sleep(Duration::from_millis(25)).await;
|
||||
Ok::<_, Error>(build_props("slow"))
|
||||
},
|
||||
std::future::pending::<Result<ServerProperties>>,
|
||||
|| build_props("fallback"),
|
||||
)
|
||||
.await;
|
||||
|
||||
Reference in New Issue
Block a user