Skip to content

test(s3): cover batch delete partial response#131

Draft
overtrue wants to merge 1 commit intomainfrom
codex/s3-batch-delete-partial-response-gap
Draft

test(s3): cover batch delete partial response#131
overtrue wants to merge 1 commit intomainfrom
codex/s3-batch-delete-partial-response-gap

Conversation

@overtrue
Copy link
Copy Markdown
Contributor

Summary

This adds focused regression coverage for the batch delete path introduced with the recent RustFS force-delete work.

The new delete_objects_with_options helper now handles RustFS-specific request options and parses S3 batch delete responses. The existing tests cover the force-delete header behavior, but they do not exercise the branch where the backend returns a mixed response with both <Deleted> and <Error> entries. That left a gap in the changed code where a future refactor could incorrectly fail the whole request or drop successfully deleted keys.

Root cause

The follow-up test coverage around rm --purge concentrated on request construction and the full purge flow. It did not assert the non-fatal partial-success behavior already implemented in delete_objects_with_options.

Fix

This PR adds one narrow unit test in crates/s3/src/client.rs that feeds a mixed DeleteResult payload into the captured request client and verifies two things:

  1. the batch delete request is still issued to the expected delete endpoint;
  2. the client returns only the successfully deleted keys instead of treating the partial response as a hard failure.

The change is test-only and stays inside the recently modified S3 delete helper.

Validation

I ran the required checks successfully:

  • cargo test -p rc-s3 delete_objects_with_partial_errors_returns_deleted_keys --lib
  • cargo fmt --all --check
  • cargo clippy --workspace -- -D warnings
  • cargo test --workspace

I also ran make pre-commit per the automation instruction, but this checkout does not define that target and make returned No rule to make target 'pre-commit'.

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.

1 participant