Repository navigation
Conversation
localstub 0.0.3 decoded incoming header values to text, so tests had no way to check the exact bytes the CLI sent for a non-ASCII header. That left `test_content_disposition_non_ascii` skipped. 0.1.1 keeps the original header bytes and exposes them through `Headers.raw`, so the test now runs again. The test checks `Headers.raw` rather than `headers.get()`. Lookups decode values as Latin-1, so the UTF-8 encoded `×` the CLI sends would otherwise appear as mojibake and never match the expected string.
`cli_env` set `AWS_MAX_ATTEMPTS=1`, which is outside the range the CRT transfer client supports, so every s3 transfer in these tests fell back to the classic client even though CRT is now the default. Removing it puts the tests on the default path, and also turns on the default retry behavior. The CLI only auto-resolves to CRT when no other `aws` process holds the CRT process lock, so under pytest-xdist each invocation can get either client. The assertions now check what S3 accepts rather than one client's wire format. A checksum may be named by `x-amz-sdk-checksum-algorithm` or only by the `x-amz-trailer` that CRT sends, `x-amz-checksum-mode` is compared case-insensitively since CRT sends `enabled`, and the checksum mismatch tests accept either client's error. With retries on, a single scripted error no longer fails a command: the retry falls through to localstub's default 200 response. Tests that expect a failure now fail every attempt and check which requests were made instead of a count tied to the retry policy. The content-length mismatch tests had been failing since botocore took over Content-Length validation and made a short body retryable. They now truncate every GET and check that no partial file is left and that mv keeps its source. CRT rejects a 204 with no headers with AWS_ERROR_S3_INVALID_RESPONSE_STATUS, which broke rm, rb --force and sync --delete, so the 204 helpers now send `x-amz-request-id` the way S3 does. The ranged download test served the same 5 MiB body to every request for a 10 MiB object; CRT's own HeadObject took that as the object size, so the test now serves byte ranges of a single object.
hssyoo
approved these changes
Nov 4, 2026
This branch has not been deployed
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.
This refreshes the current blackbox tests to test against the default S3 transfer client, which is now the CRT. Previously, the test fixtures set
AWS_MAX_ATTEMPTS=1which forced a fallback to the Python transfer manager. This is removed (and tests updated to properly account for this) so that we test the CRT codepath. The version of localstub is also updated to the latest 0.1.1 which provides additional APIs for additional testing capabilities and to unblock test scenarios such as non-ascii headers.There a few changes needed to the existing tests to account for the CRT differences. While behaviorally similar, there are some wire protocol differences:
x-amz-trailerinstead ofx-amz-sdk-checksum-algorithmenabledvs.ENABLED)x-amz-request-id)One thing you'll see here is that we have to handle both assertion cases for the Python and CRT transfer manager. This is because of the CRT process lock, which if already taken will trigger a fallback to Python. This can happen with the default set of options in CI via
pytest-xdistwhich runs things in parallel. Once the constraints around the CRT locks are resolved we should be able to simplify these tests to always expect the CRT code path. There's also the quirk about the double HeadObject needed on the python side for sizing and the CRT size for checksums, which we should eventually be able to resolve when we pull in newer CRT versions, but for now these tests need to account for that.