test: write test fixtures to temp dirs instead of the repo tree - #1665
test: write test fixtures to temp dirs instead of the repo tree#1665mishushakov wants to merge 1 commit into
Conversation
Eight test files created their fixtures outside a temporary destination, so
running the suite left directories behind in the working tree.
The five CLI template tests called `fs.mkdtemp` with a bare relative prefix.
`mkdtemp` does not imply `os.tmpdir()` — a relative prefix resolves against
`process.cwd()`, so each run created `packages/cli/e2b-<name>-testXXXXXX/`.
They now join the prefix onto `os.tmpdir()`.
On the JS SDK side, `getAllFilesInPath` and `spoolTarArchive` wrote into
`__dirname` and now `mkdtemp` under `os.tmpdir()`. `build.test.ts` built its
file context at `tests/template/folder`, relying on the implicit
caller-directory context; it now creates the context under `os.tmpdir()` and
passes it explicitly via `Template({ fileContextPath })`, matching what the
Python mirror in `test_build.py` already does with `tempfile.mkdtemp` and
`file_context_path`.
The Python suite needed no changes: every host-side write already goes
through `tmp_path`, `tempfile.mkdtemp`, or `TemporaryDirectory`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
PR SummaryLow Risk Overview Reviewed by Cursor Bugbot for commit 4217015. Bugbot is set up for automated code reviews on this repo. Configure here. |
Package ArtifactsBuilt from 3ead754. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.38.4-bangui.0.tgzCLI ( npm install ./e2b-cli-2.16.2-bangui.0.tgzPython SDK ( pip install ./e2b-2.38.0+bangui-py3-none-any.whl |
![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: 4217015d5f
ℹ️ 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".
| const folderPath = path.join(__dirname, 'folder') | ||
| // The file context lives in a temp directory so a test run never writes into | ||
| // the repository tree. | ||
| const contextPath = fs.mkdtempSync(path.join(os.tmpdir(), 'js-build-test-')) |
There was a problem hiding this comment.
When this commit updates package-scoped files under both packages/cli and packages/js-sdk, the repository instructions provided for this review require generating a changeset with pnpm changeset; this diff only changes the test files and does not add or update any .changeset entry for those packages, leaving the required release metadata absent for these package updates.
Useful? React with 👍 / 👎.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good, straightforward test-only fix.
Extended reasoning...
Overview
Test-only change across 5 CLI test files and 3 JS SDK test files. All changes replace fixture/temp directory creation that either used a relative fs.mkdtemp prefix (resolving against process.cwd()) or wrote directly under __dirname, with directories created under os.tmpdir(). build.test.ts additionally passes the temp context explicitly via Template({ fileContextPath }), an option that already exists in the SDK (packages/js-sdk/src/template/types.ts, index.ts) and mirrors the pattern already used by the Python test suite.
Security risks
None. This only affects local test execution paths (temp directory locations) and does not touch production code, auth, or data handling.
Level of scrutiny
Low. This is a mechanical, low-risk change confined to test setup/teardown code with no changes to src/ in either package. The diff is easy to verify by inspection: each site now joins the same prefix onto os.tmpdir() (or uses mkdtemp(join(tmpdir(), ...))) instead of a bare/relative prefix, and cleanup (afterAll/afterEach rm) is updated consistently to remove the new temp directory.
Other factors
No other reviewer or bot flagged concerns beyond the informational Cursor summary agreeing this is low risk. No changeset is needed since test-only changes don't ship. The bug-hunting system found no issues, and the candidate quality nits about duplicated mkdtemp boilerplate across files are pre-existing style, not something introduced or worsened by this PR.
1 participant
Eight test files created their fixtures outside a temporary destination, so running the suite left directories behind in the working tree. The five CLI template tests called
fs.mkdtempwith a bare relative prefix —mkdtempdoes not implyos.tmpdir(), so a relative prefix resolves againstprocess.cwd()and each run createdpackages/cli/e2b-<name>-testXXXXXX/; they now join the prefix ontoos.tmpdir(). On the JS SDK sidegetAllFilesInPathandspoolTarArchivewrote into__dirnameand nowmkdtempunderos.tmpdir(), whilebuild.test.tsbuilt its file context attests/template/folderrelying on the implicit caller-directory context — it now creates the context underos.tmpdir()and passes it explicitly viaTemplate({ fileContextPath }), matching what the Python mirror intest_build.pyalready does withtempfile.mkdtempandfile_context_path. The Python suite needed no changes: every host-side write already goes throughtmp_path,tempfile.mkdtemp, orTemporaryDirectory, verified both by inspection of all 46 write sites and by running the filesystem-touching unit tests and confirming a cleangit statusafterwards.No usage examples apply — this is a test-only change with no user-facing surface.
Verification
packages/cli: full suite, 108 passed / 1 skipped. The one failing file iscreate.test.ts, which throws becauseE2B_API_KEYis not set locally — pre-existing and unrelated.packages/js-sdk: the two util test files, 24 passed.packages/python-sdk: 88 passed / 1 skipped across the template utils, upload-file, and io-utils suites.pnpm run format,pnpm run lint,pnpm run typecheckall clean; no leftover directories anywhere after the runs.No changeset: test-only changes do not ship in either published package.
Fixes SDK-334
🤖 Generated with Claude Code