Skip to content

test(s3): cover delete bucket NotFound fallback#168

Merged
overtrue merged 1 commit intomainfrom
codex/test-delete-bucket-notfound-code
May 5, 2026
Merged

test(s3): cover delete bucket NotFound fallback#168
overtrue merged 1 commit intomainfrom
codex/test-delete-bucket-notfound-code

Conversation

@overtrue
Copy link
Copy Markdown
Contributor

@overtrue overtrue commented May 4, 2026

Related issue

This PR does not resolve a user-facing bug by itself. It closes a remaining test gap around the recent delete_bucket error-classification changes landed in #163 and #166.

Background

crates/s3/src/client.rs::delete_bucket now has explicit classification for missing buckets, non-empty buckets, and generic backend failures. The existing tests already covered NoSuchBucket, BucketNotEmpty, and a generic InternalError path.

A nearby backend variant was still untested: some S3-compatible services return NotFound instead of NoSuchBucket for the same missing-bucket condition. The implementation already handles that code path, but the regression suite did not prove it.

Root cause

The recent focused test additions around delete_bucket stopped one branch short of the sibling NotFound mapping path, leaving the same user-visible behavior dependent on unverified backend wording.

Solution

Add one focused unit test that simulates a 404 NotFound delete-bucket response and asserts that rc still maps it to Error::NotFound("Bucket not found: <bucket>").

This keeps the scope limited to the changed helper and strengthens the recent error-mapping work without changing runtime behavior.

Validation

  • cargo test -p rc-s3 delete_bucket_maps_ --lib
  • make pre-commit

@overtrue overtrue marked this pull request as ready for review May 4, 2026 21:04
Copilot AI review requested due to automatic review settings May 4, 2026 21:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a focused regression test for crates/s3/src/client.rs::delete_bucket to ensure S3-compatible backends that return the NotFound error code (instead of NoSuchBucket) are still classified as Error::NotFound, preserving the intended CLI behavior and guarding the recent error-mapping changes.

Changes:

  • Added a new #[tokio::test] covering DeleteBucket responses with x-amz-error-code: NotFound.
  • Asserts the error is mapped to Error::NotFound("Bucket not found: <bucket>"), matching the existing missing-bucket behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@overtrue overtrue merged commit 51b4959 into main May 5, 2026
19 checks passed
@overtrue overtrue deleted the codex/test-delete-bucket-notfound-code branch May 5, 2026 02:55
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.

2 participants