You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "youngsofun (via GitHub)" <gi...@apache.org> on 2024/01/25 07:08:02 UTC
[PR] Fix: list path recursive should not return path itself [opendal]
youngsofun opened a new pull request, #4067:
URL: https://github.com/apache/opendal/pull/4067
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: list path recursive should not return path itself [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #4067:
URL: https://github.com/apache/opendal/pull/4067
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: list path recursive should not return path itself [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4067:
URL: https://github.com/apache/opendal/pull/4067#discussion_r1465978569
##########
core/tests/behavior/async_list.rs:
##########
@@ -476,17 +476,37 @@ pub async fn test_list_root_with_recursive(op: Operator) -> Result<()> {
.map(|v| v.path().to_string())
.collect::<HashSet<_>>();
- assert!(!actual.contains("/"), "empty root should return itself");
- assert!(!actual.contains(""), "empty root should return empty");
+ assert!(!actual.contains("/"), "empty root should not return itself");
+ assert!(!actual.contains(""), "empty root should not return empty");
Ok(())
}
+async fn list_recursive_helper(op: &Operator, parent: &str, path: &str) -> Result<Vec<String>> {
Review Comment:
Hello, our behavior tests are structured for easy replication to ensure readers understand the test's purpose without referencing helper functions.
Would you like to revert this change?
##########
core/tests/behavior/blocking_list.rs:
##########
@@ -187,11 +187,27 @@ pub fn test_blocking_scan(op: BlockingOperator) -> Result<()> {
}
}
+ op.remove_all(&format!("{parent}/x/"))?;
+
+ for path in expected.iter() {
+ if path.ends_with('/') {
+ continue;
+ }
+ assert!(
+ !op.is_exist(&format!("{parent}/{path}"))?,
+ "{parent}/{path} should be removed"
+ )
+ }
+
+ Ok(())
+}
+
+fn list_recursive_helper(op: &BlockingOperator, parent: &str, path: &str) -> Result<Vec<String>> {
Review Comment:
The same.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: list path recursive should not return path itself [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #4067:
URL: https://github.com/apache/opendal/pull/4067#issuecomment-1909754188
https://github.com/apache/opendal/pull/4070 has been merged, please try again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: list path recursive should not return path itself [opendal]
Posted by "youngsofun (via GitHub)" <gi...@apache.org>.
youngsofun commented on code in PR #4067:
URL: https://github.com/apache/opendal/pull/4067#discussion_r1466048498
##########
core/tests/behavior/blocking_list.rs:
##########
@@ -187,11 +187,27 @@ pub fn test_blocking_scan(op: BlockingOperator) -> Result<()> {
}
}
+ op.remove_all(&format!("{parent}/x/"))?;
+
+ for path in expected.iter() {
+ if path.ends_with('/') {
+ continue;
+ }
+ assert!(
+ !op.is_exist(&format!("{parent}/{path}"))?,
+ "{parent}/{path} should be removed"
+ )
+ }
+
+ Ok(())
+}
+
+fn list_recursive_helper(op: &BlockingOperator, parent: &str, path: &str) -> Result<Vec<String>> {
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: list path recursive should not return path itself [opendal]
Posted by "youngsofun (via GitHub)" <gi...@apache.org>.
youngsofun commented on PR #4067:
URL: https://github.com/apache/opendal/pull/4067#issuecomment-1909699816
waiting for https://github.com/apache/opendal/pull/4070
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org