Skip to content

test(s3): cover delete wrapper defaults#116

Draft
overtrue wants to merge 1 commit intomainfrom
automation/test-gap-delete-wrapper-delegation-followup
Draft

test(s3): cover delete wrapper defaults#116
overtrue wants to merge 1 commit intomainfrom
automation/test-gap-delete-wrapper-delegation-followup

Conversation

@overtrue
Copy link
Copy Markdown
Contributor

Summary

This PR adds focused unit coverage for the legacy ObjectStore delete wrappers in the S3 client. The recent force-delete work introduced options-aware delete helpers and tests around *_with_options, but the wrapper methods that existing call sites still use did not have direct coverage proving they delegate through the default option path.

Problem

Without a wrapper-level assertion, a future refactor could accidentally bypass the default options path and start sending the RustFS force-delete header for normal deletes. That would silently change ordinary delete behavior from delete-marker creation to permanent deletion on versioned buckets.

Root Cause

The original test additions validated explicit option handling, including force-delete header emission and omission on the *_with_options methods. The ObjectStore trait implementation methods delete_object and delete_objects now delegate into those helpers, but there was no regression test pinning that delegation.

Fix

The change adds two S3 client unit tests that call the wrapper methods directly and capture the generated HTTP requests. Each test asserts that the x-rustfs-force-delete header is absent when the wrapper uses default request options.

Validation

I first ran the two new unit tests directly:

  • cargo test -p rc-s3 delete_object_wrapper_uses_default_options_without_rustfs_header
  • cargo test -p rc-s3 delete_objects_wrapper_uses_default_options_without_rustfs_header

I then ran the repo's CI-equivalent checks:

  • cargo fmt --all --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test --workspace

make pre-commit was also requested by the automation prompt, but this checkout does not contain a Makefile or a pre-commit target, so I used the commands defined in .github/workflows/ci.yml instead.

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