Skip to content

test(python-sdk): regression tests for template upload Content-Length#1293

Closed
mishushakov wants to merge 1 commit into
mainfrom
mishushakov/py-upload-tests
Closed

test(python-sdk): regression tests for template upload Content-Length#1293
mishushakov wants to merge 1 commit into
mainfrom
mishushakov/py-upload-tests

Conversation

@mishushakov
Copy link
Copy Markdown
Member

Summary

No Python SDK code changes — the current implementation already passes bytes to httpx.put(..., content=...), so this is purely a regression guard.

Test plan

  • poetry run pytest tests/sync/template_sync/test_upload_file.py tests/async/template_async/test_upload_file.py -v — both pass locally
  • poetry run make format / make lint / make typecheck clean

🤖 Generated with Claude Code

…ngth

Guards against future changes swapping the buffered bytes for a
generator/stream, which would trigger Transfer-Encoding: chunked and
break S3 presigned PUT uploads with 501 NotImplemented.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@cursor
Copy link
Copy Markdown

cursor Bot commented Apr 24, 2026

PR Summary

Low Risk
Low risk because changes are test-only and don’t modify runtime SDK behavior; they primarily guard against a future regression that would break presigned PUT uploads.

Overview
Adds sync and async regression tests for template_* upload_file that PUT to a local HTTP server and assert the request includes a non-zero Content-Length matching the received body.

The tests also assert the upload does not use Transfer-Encoding: chunked, protecting against future refactors that might switch the request body to a stream/generator and break S3 presigned PUT URLs.

Reviewed by Cursor Bugbot for commit bc3f865. Bugbot is set up for automated code reviews on this repo. Configure here.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 24, 2026

⚠️ No Changeset found

Latest commit: bc3f865

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

Package Artifacts

Built from 3155e27. Download artifacts from this workflow run.

JS SDK (e2b@2.19.1-mishushakov-py-upload-tests.0):

npm install ./e2b-2.19.1-mishushakov-py-upload-tests.0.tgz

CLI (@e2b/cli@2.9.1-mishushakov-py-upload-tests.0):

npm install ./e2b-cli-2.9.1-mishushakov-py-upload-tests.0.tgz

Python SDK (e2b==2.20.0+mishushakov-py-upload-tests):

pip install ./e2b-2.20.0+mishushakov.py.upload.tests-py3-none-any.whl

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test-only regression guard for upload_file Content-Length behavior — straightforward and self-contained.

Extended reasoning...

Overview

This PR adds two new test files (sync + async) under packages/python-sdk/tests/ that spin up a local HTTPServer on 127.0.0.1:0, invoke upload_file against it, and assert the request carries a matching Content-Length and no Transfer-Encoding: chunked. No runtime SDK code is modified.

Security risks

None. Tests bind to loopback only, use ephemeral ports, and run in daemon threads that are torn down in a finally block. No credentials, network egress, or filesystem writes outside pytest's tmp_path.

Level of scrutiny

Low. This is a pure test addition guarding against a previously-seen class of bug (JS SDK #1285/#1243 where presigned S3 PUTs failed under chunked encoding). The tests exercise the public function signature and assert on observable HTTP behavior — no mocking of internals, no brittle coupling.

Other factors

The mock server pattern is simple and correct: server.shutdown() + server_close() + thread.join(timeout=5) in finally ensures cleanup even on failure. The async test correctly relies on the fact that the sync HTTPServer in a daemon thread does not block the asyncio event loop, since httpx.AsyncClient uses asyncio sockets to talk to it. The assertions are precise (Content-Length > 0, matches received body length, no chunked TE) and fail-loud.

mishushakov added a commit that referenced this pull request Apr 24, 2026
…n tests (#1294)

## Summary
Consolidates the fix and tests from #1285 and #1293 into a single PR.

- **js-sdk**: `uploadFile` used to pass a Node `Readable` directly to
`fetch`, causing undici to fall back to `Transfer-Encoding: chunked`. S3
presigned PUT URLs reject chunked with 501 NotImplemented. Fix buffers
the archive first so `Content-Length` is set. Includes:
- Regression test that spins up a local HTTP server and asserts
`Content-Length` is set and matches the body, and `Transfer-Encoding` is
not chunked.
- Type-fix for the CLI's typecheck (cast `Pack` →
`AsyncIterable<Buffer>`).
- Dynamic import of `node:stream/consumers` so the browser bundle
doesn't pull it in.
- **python-sdk**: Adds sync + async regression tests for `upload_file`
that guard against the same class of bug (someone swapping
`tar_buffer.getvalue()` for a stream/generator). No Python code change —
the current implementation already passes bytes to `httpx.put(...,
content=...)`.

Authorship of the original JS fix commit preserved (truffle-dev).

Closes #1243.

## Test plan
- [x] `pnpm run test tests/template/uploadFile.test.ts` — passes
- [x] `pnpm run typecheck` / `lint` clean across js-sdk and cli
- [x] `poetry run pytest tests/sync/template_sync/test_upload_file.py
tests/async/template_async/test_upload_file.py -v` — both pass
- [x] `poetry run make format` / `make lint` / `make typecheck` clean

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: truffle <truffleagent@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@ValentaTomas ValentaTomas deleted the mishushakov/py-upload-tests branch May 13, 2026 08:40
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