Skip to content

test(io): stop test_cloud_paths from resolving S3 region over network#7808

Open
wjones127 wants to merge 1 commit into
lance-format:mainfrom
wjones127:fix/test-cloud-paths-region
Open

test(io): stop test_cloud_paths from resolving S3 region over network#7808
wjones127 wants to merge 1 commit into
lance-format:mainfrom
wjones127:fix/test-cloud-paths-region

Conversation

@wjones127

@wjones127 wjones127 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

object_store::tests::test_cloud_paths is failing on main in the Rust CI job.

The test opens s3://bucket/foo.lance with no region configured. resolve_s3_region then makes a live HEAD request to bucket.s3.amazonaws.com to read the x-amz-bucket-region header. That request is flaky in CI: when the header is absent the call fails with RegionParse { bucket: "bucket" }, panicking the .unwrap(). In the same failing run, test_block_size_used_cloud::case_1 — which hits the identical code path — passed, confirming the failure is non-deterministic network behavior, not a logic regression.

The affected tests only verify URL/path parsing, so they have no reason to reach the network. This passes a static region storage option for the S3 cases, which short-circuits resolve_s3_region before any request is made. test_block_size_used_cloud's S3 case shares the same flaw and is fixed the same way.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved cloud storage testing reliability by using a fixed S3 region configuration.
    • Prevented tests from requiring live region resolution when validating S3 paths and block size behavior.

`test_cloud_paths` and the S3 case of `test_block_size_used_cloud` opened
`s3://bucket/foo.lance` with no region configured. `resolve_s3_region` then
makes a live HEAD request to `bucket.s3.amazonaws.com` to read the
`x-amz-bucket-region` header. That request is flaky in CI: when the header is
absent the call fails with `RegionParse`, panicking the test — while other
tests hitting the same path in the same run pass, confirming non-determinism.

These tests only verify URL/path parsing, so pass a static `region` storage
option, which short-circuits `resolve_s3_region` before any network call.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0a1715ed-7eb3-48af-9abc-fe3c8f23518d

📥 Commits

Reviewing files that changed from the base of the PR and between 234fc2a and 33c1025.

📒 Files selected for processing (1)
  • rust/lance-io/src/object_store.rs

📝 Walkthrough

Walkthrough

S3-related tests now pass static us-east-1 storage options when constructing object stores and testing block-size behavior, while preserving existing path and scheme assertions.

Changes

S3 test configuration

Layer / File(s) Summary
Static S3 region options
rust/lance-io/src/object_store.rs
test_cloud_paths and the S3 block-size test case use storage options containing the static us-east-1 region instead of relying on live region resolution.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: jackye1995, xuanwo

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: avoiding S3 region resolution during tests to prevent network calls.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added A-encoding Encoding, IO, file reader/writer chore labels Jul 15, 2026
@wjones127
wjones127 marked this pull request as ready for review July 15, 2026 19:24
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-encoding Encoding, IO, file reader/writer chore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant