Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(blocking): tests for blocking append #3023

Merged
merged 5 commits into from Sep 18, 2023
Merged

test(blocking): tests for blocking append #3023

merged 5 commits into from Sep 18, 2023

Conversation

suyanhanx
Copy link
Member

@suyanhanx suyanhanx commented Sep 8, 2023

Although there are not many services having native implementations of blocking_write now, we still need to cover them.
(because append has been merged into write).

core/tests/behavior/write.rs Outdated Show resolved Hide resolved
@suyanhanx
Copy link
Member Author

suyanhanx commented Sep 8, 2023

https://github.com/apache/incubator-opendal/blob/1bb63ab676293cb20cb90fbb177d58bd25340c8c/core/src/services/fs/writer.rs#L57-L68

For the service fs, doing append will go wrong definitely here if we enable atomic_write.

It's better to set a new test job for atomic_write.

@Xuanwo
Copy link
Member

Xuanwo commented Sep 8, 2023

It's better to set a new test job for atomic_write.

Negative, this is a violation of our VISION. We can't add different test for services with different configure values.

For the service fs, doing append will go wrong definitely here if we enable atomic_write.

It's a bug. We should fix it from fs internal.

core/tests/behavior/main.rs Outdated Show resolved Hide resolved
core/src/services/fs/backend.rs Outdated Show resolved Hide resolved
Signed-off-by: suyanhanx <suyanhanx@gmail.com>
Signed-off-by: suyanhanx <suyanhanx@gmail.com>
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Xuanwo Xuanwo marked this pull request as ready for review September 18, 2023 04:55
@Xuanwo Xuanwo merged commit 4b02228 into main Sep 18, 2023
156 of 182 checks passed
@Xuanwo Xuanwo deleted the empty-file-test branch September 18, 2023 05:16
Young-Flash pushed a commit to Young-Flash/opendal that referenced this pull request Sep 19, 2023
* test for blocking append & fix fs

Signed-off-by: suyanhanx <suyanhanx@gmail.com>

* move path check to path build block

Signed-off-by: suyanhanx <suyanhanx@gmail.com>

* path check when append enabled only

Signed-off-by: suyanhanx <suyanhanx@gmail.com>

---------

Signed-off-by: suyanhanx <suyanhanx@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants