Skip to content

Commit

Permalink
fix: fix tests failed on windows (#3155)
Browse files Browse the repository at this point in the history
* fix: fix tests failed on windows

* feat: add comments

* Update src/object-store/src/util.rs

Co-authored-by: Yingwen <[email protected]>

---------

Co-authored-by: Yingwen <[email protected]>
  • Loading branch information
WenyXu and evenyag authored Jan 12, 2024
1 parent 527e523 commit 75975ad
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/metric-engine/src/engine/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ mod test {
],
primary_key: vec![0],
options: HashMap::new(),
region_dir: "test_dir".to_string(),
region_dir: "/test_dir".to_string(),
};

let env = TestEnv::new().await;
Expand Down
50 changes: 37 additions & 13 deletions src/object-store/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,6 @@ pub async fn collect(stream: Lister) -> Result<Vec<Entry>, opendal::Error> {
stream.try_collect::<Vec<_>>().await
}

/// Normalize a directory path, ensure it is ends with '/'
pub fn normalize_dir(dir: &str) -> String {
let mut dir = dir.to_string();
if !dir.ends_with('/') {
dir.push('/')
}

dir
}

/// Join two paths and normalize the output dir.
///
/// The output dir is always ends with `/`. e.g.
Expand All @@ -44,8 +34,42 @@ pub fn normalize_dir(dir: &str) -> String {
pub fn join_dir(parent: &str, child: &str) -> String {
// Always adds a `/` to the output path.
let output = format!("{parent}/{child}/");
// We call opendal's normalize_root which keep the last `/`.
opendal::raw::normalize_root(&output)
normalize_dir(&output)
}

/// Modified from the `opendal::raw::normalize_root`
///
/// # The different
///
/// It doesn't always append `/` ahead of the path,
/// It only keeps `/` ahead if the original path starts with `/`.
///
/// Make sure the directory is normalized to style like `abc/def/`.
///
/// # Normalize Rules
///
/// - All whitespace will be trimmed: ` abc/def ` => `abc/def`
/// - All leading / will be trimmed: `///abc` => `abc`
/// - Internal // will be replaced by /: `abc///def` => `abc/def`
/// - Empty path will be `/`: `` => `/`
/// - **(Removed❗️)** ~~Add leading `/` if not starts with: `abc/` => `/abc/`~~
/// - Add trailing `/` if not ends with: `/abc` => `/abc/`
///
/// Finally, we will got path like `/path/to/root/`.
pub fn normalize_dir(v: &str) -> String {
let has_root = v.starts_with('/');
let mut v = v
.split('/')
.filter(|v| !v.is_empty())
.collect::<Vec<&str>>()
.join("/");
if has_root {
v.insert(0, '/');
}
if !v.ends_with('/') {
v.push('/')
}
v
}

/// Push `child` to `parent` dir and normalize the output path.
Expand Down Expand Up @@ -89,7 +113,7 @@ mod tests {
assert_eq!("/", join_dir("", "/"));
assert_eq!("/", join_dir("/", "/"));
assert_eq!("/a/", join_dir("/a", ""));
assert_eq!("/a/b/c/", join_dir("a/b", "c"));
assert_eq!("a/b/c/", join_dir("a/b", "c"));
assert_eq!("/a/b/c/", join_dir("/a/b", "c"));
assert_eq!("/a/b/c/", join_dir("/a/b", "c/"));
assert_eq!("/a/b/c/", join_dir("/a/b", "/c/"));
Expand Down

0 comments on commit 75975ad

Please sign in to comment.