test: serve S3 tests from a local moto server - #1695
Merged
Conversation
The S3 tests read pythia_ppZee_run17emb.picoDst.root (872 MB) out of the `pivarski-princeton` bucket on every CI run, which is running up data transfer charges for its owner; the file is being moved to Zenodo and will be deleted from AWS on August 31, 2026 (#1693). Rather than repoint the tests at another remote copy, run them against an in-process moto S3 server populated with a small skhep_testdata file. This removes the last use of that bucket, keeps coverage of the s3:// protocol (open, storage-option pass-through, and globbing), and makes the tests local, so they no longer need the `network` marker. Closes #1693 Assisted-by: claude-code:claude-opus-5[1m]
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the test suite’s S3 coverage to run against a local in-process moto S3 server instead of reading a large object from a public AWS bucket, reducing CI network dependence and avoiding ongoing S3 egress charges.
Changes:
- Add a session-scoped
s3_serverfixture that boots aThreadedMotoServer, creates a bucket, and uploads twouproot-HZZ.rootcopies for deterministic S3 test inputs. - Update S3-related tests to use the local moto-backed bucket and adjust assertions (including globbing expecting exactly two matches).
- Add
moto[s3,server]to thetestdependency group.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/test_0916_read_from_s3.py | Switch S3 read/fail tests from remote AWS bucket to the local s3_server fixture. |
| tests/test_0692_fsspec_reading.py | Update fsspec S3 open/globbing tests to target the moto-backed bucket and assert deterministic results. |
| tests/conftest.py | Introduce s3_server session fixture that provisions and populates a local moto S3 service. |
| pyproject.toml | Add moto server/S3 extra to the test dependency group. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
On free-threaded Windows, moto[server] -> docker -> pywin32 has no wheels, so uv backtracks to moto 4.1.4, which has no ThreadedMotoServer.get_host_and_port and errored out the S3 tests. Skip them there instead. Assisted-by: claude-code:claude-opus-5[1m]
ariostas
force-pushed
the
ariostas/update-s3-test-file-1693
branch
from
August 4, 2026 18:36
ba7c387 to
53791cf
Compare
Only the ThreadedMotoServer construction and start() are outside the try/finally now, so a failure in port discovery or bucket setup can no longer leave the server thread running. Assisted-by: claude-code:claude-opus-5[1m]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1693.
🤖 AI text below 🤖
The S3 tests were reading
pythia_ppZee_run17emb.picoDst.root(872 MB) out of thepivarski-princetonbucket on every CI run, which is running up data transfer charges for its owner. That file is moving to Zenodo and will be deleted from AWS on August 31, 2026.It was the only object in that bucket, and all three tests using it exist to exercise the
s3://protocol, so the new Zenodo HTTPS URL isn't a substitute. Instead of repointing them at another remote copy, this runs them against an in-process moto S3 server.s3_serverfixture intests/conftest.py(alongside the existinghttp_server/xrootd_serverfixtures): startsThreadedMotoServeron an ephemeral port, creates a bucket, and uploads two copies ofuproot-HZZ.rootfromskhep_testdata. It yields(bucket_url, storage_options), with fake key/secret passed explicitly so a developer's real AWS credentials are never picked up.test_s3_fail,test_read_s3,test_open_fsspec_s3, andtest_fsspec_globbing_s3now use the fixture. The globbing test asserts 2 matches in a bucket the fixture controls, rather than depending on the remote bucket's contents not changing.@pytest.mark.networkfrom these tests — they're local now, and run in ~2.5s.moto[s3,server]to thetestdependency group.No reference to the
pivarski-princetonbucket remains in the repo.The one trade-off: we no longer read a real remote S3 object anywhere in the test suite. If that's worth keeping, the Zenodo URL could be added as an
https://fsspec test, but it can't cover thes3://path.🤖 Generated with Claude Code