Skip to content

test(s3): cover cors fallback url builder#99

Draft
overtrue wants to merge 1 commit intomainfrom
codex/cors-fallback-url-gap
Draft

test(s3): cover cors fallback url builder#99
overtrue wants to merge 1 commit intomainfrom
codex/cors-fallback-url-gap

Conversation

@overtrue
Copy link
Copy Markdown
Contributor

@overtrue overtrue commented Apr 9, 2026

Summary

This PR adds focused unit coverage for the S3 CORS XML fallback URL builder introduced with recent bucket CORS support.

The recent CORS implementation added a dedicated cors_url helper so the client can fall back to path-style XML requests when the AWS SDK path does not work against RustFS-compatible responses. That helper is part of the new behavior, but it did not have direct tests on main. Without coverage here, a future change to URL construction or endpoint handling could silently break the fallback path even while the existing parser tests still pass.

This change keeps scope tight to that untested path. It extracts a small test-only helper so the unit tests can build an S3Client with a custom endpoint, then adds assertions for the two critical branches in cors_url: the normal path-style URL shape (https://example.com/bucket-name?cors=) and the invalid-endpoint error branch for endpoints that cannot support path-style bucket operations.

Validation

I first ran the two targeted unit tests for the new coverage:

  • cargo test -p rc-s3 cors_url_uses_path_style_bucket_and_query
  • cargo test -p rc-s3 cors_url_rejects_endpoints_without_path_segments

The automation requested make pre-commit, but this repository currently has no pre-commit make target or local pre-commit configuration. To match the repository's documented CI gates, I ran the equivalent checks directly:

  • cargo fmt --all --check
  • cargo clippy --workspace --all-targets -- -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