feat(js-sdk): run the vitest unit suite on Deno - #1585
Conversation
|
PR SummaryLow Risk Overview CI installs Deno only on ubuntu-22.04, upgrades Reviewed by Cursor Bugbot for commit b0d838d. Bugbot is set up for automated code reviews on this repo. Configure here. |
Package ArtifactsBuilt from 1aea10c. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.35.3-add-deno-test-suite.0.tgzCLI ( npm install ./e2b-cli-2.14.1-add-deno-test-suite.0.tgzPython SDK ( pip install ./e2b-2.34.0+add.deno.test.suite-py3-none-any.whl |
782d7d3 to
58042a3
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58042a3c1b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
58042a3 to
e424eea
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
test:deno now runs the vitest unit and connectionConfig projects under Deno (deno run -A npm:vitest run --project unit --project connectionConfig), replacing the previous single dist-based smoke test (superseded - the suite covers the SDK under Deno far more thoroughly). CI runs it on ubuntu only, matching the Bun suite step. Bump the Deno pin to 2.8.1 (setup-deno@v2) since vitest needs Deno 2's Node compat. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
e424eea to
4778135
Compare
tests/runtimes/edge/ no longer exists, so the project matched zero files and only added noise to vitest run output. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
## What Extends the Deno vitest run (#1585) with the `template` project and fixes the real runtime bug the suite surfaced. Split out of #1594 (Bun counterpart: #1596). ```jsonc // packages/js-sdk/package.json "test:deno": "deno run -A npm:vitest run --project unit --project connectionConfig --project template", ``` ## Bug — Deno: template uploads used chunked transfer encoding Deno's native `fetch` ignores an explicit `Content-Length` header on stream bodies and falls back to `Transfer-Encoding: chunked` — exactly the failure #1243 fixed for Node, since S3-compatible presigned PUT URLs reject chunked uploads with 501. `uploadFile` now streams the spooled archive through **undici's `fetch`** (via the existing `loadUndici()` helper — undici 8 where it imports, undici 7 on Bun, global `fetch` where undici isn't resolvable, e.g. bundled apps), which honors the `Content-Length` header on stream bodies on every runtime. One upload path, no runtime sniffing. Approaches rejected along the way, all verified empirically with 1GB uploads + RSS sampling: - **File-backed `Blob` body (`fs.openAsBlob`)** — lazy on Node/Bun, but Deno's shim reads the whole file into memory eagerly (denoland/deno#32316), and Bun infers an unstrippable MIME type from the extension whose `Content-Type` breaks presigned signatures (403 against production storage). - **`node:http(s)` on Deno** — works (and is memory-bounded), but can't be unified: Bun's `node:http` ignores abort signals, and it's a second code path. Known caveat: Deno's `Readable.toWeb` shim has no backpressure, so the archive is buffered in memory during upload on Deno (Node and Bun stream in lockstep with the socket). Filed upstream as denoland/deno#36275 — accepted as Deno's to fix rather than worked around here. As part of this, `tarFileStream` became `spoolTarArchive`, returning `{ path, size, cleanup }` with caller-owned cleanup instead of a self-deleting read stream. `tests/template/uploadFile.test.ts` also asserts no `Content-Type` header is sent. ## Python SDK parity Intentionally none: `upload_file` already sends a sized file body via httpx. ## Testing - Real template builds (`tests/template/build.test.ts`, against prod S3 presigned URLs) green under **Node, Deno, and Bun** - `uploadFile` + `spoolTarArchive` suites green under Node, Deno, and Bun - `tests/template/abortSignal.test.ts` green under Deno - `pnpm build`, `lint`, `typecheck`, `prettier --check` clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2 participants
Description
pnpm test:denonow runs the full vitest suite — theunitandconnectionConfigprojects, 421 sandbox/files/commands/pty/git/api/config tests — under the Deno runtime viadeno run -A npm:vitest run --project unit --project connectionConfig, replacing the previous single dist-based smoke test (superseded — the suite covers the SDK under Deno far more thoroughly). The CI step runs on ubuntu only and covers the same projects as the Bun suite step from #1584, and the Deno pin is bumped from 1.46.3 to 2.8.1 (setup-deno@v2) since vitest needs Deno 2's Node compat.Also drops the
edgevitest project:tests/runtimes/edge/no longer exists, so it matched zero files.Rebased on main after #1584: the off-Node fetch-caching fix originally in this PR was superseded by #1584's late-binding fix, which also makes the whole suite (including the per-proxy cache tests) pass under Deno with no test changes — so this PR is pure test/CI wiring.
Verified locally on Deno 2.8.1: unit project green (349 passed, 0 failed, 29 skipped — same skips as Node), connectionConfig project green (43 passed), and Node suite green.
Usage
cd packages/js-sdk pnpm test:deno🤖 Generated with Claude Code