Skip to content

test(s3): cover empty optional cors headers#111

Draft
overtrue wants to merge 1 commit intomainfrom
codex/cors-empty-optional-headers-gap
Draft

test(s3): cover empty optional cors headers#111
overtrue wants to merge 1 commit intomainfrom
codex/cors-empty-optional-headers-gap

Conversation

@overtrue
Copy link
Copy Markdown
Contributor

Summary

Recent bucket CORS support added helper conversions between SDK CORS rules and the core CorsRule type. Those helpers intentionally normalize empty optional header lists away, but the existing tests only covered populated header fields and method normalization.

This change adds focused unit coverage for the missing normalization paths in crates/s3/src/client.rs:

  • sdk_cors_rule_to_core_drops_empty_optional_headers verifies SDK rules without optional headers become None in the core type.
  • core_cors_rule_to_sdk_drops_empty_optional_headers verifies empty optional header vectors do not leak meaningful values back into the SDK rule representation.

User Impact

Without these tests, a regression in empty optional header normalization could slip through future refactors in the new bucket CORS code path. That would risk inconsistent behavior between SDK responses and CLI-managed CORS configs, especially when optional header fields are omitted.

Root Cause

The recent CORS feature introduced normalization helpers for optional header collections, but only the populated-field path was explicitly covered. The empty-field path remained implicit and unverified.

Validation

I could not run make pre-commit because this repository checkout does not contain a Makefile or pre-commit target. I validated with the repo's documented Rust checks instead:

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

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