chore(cli): tighten up sandbox exec piping tests#1129
chore(cli): tighten up sandbox exec piping tests#1129matthewlouisbrockman wants to merge 14 commits intomainfrom
Conversation
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 045cf19a96
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
packages/cli/tests/commands/sandbox/backend_integration.test.ts
Outdated
Show resolved
Hide resolved
|
uh oh |
Package ArtifactsBuilt from 6e64cad. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.16.1-piping-tone-down-the-tests-configs-eng-3497.0.tgzCLI ( npm install ./e2b-cli-2.9.1-piping-tone-down-the-tests-configs-eng-3497.0.tgzPython SDK ( pip install ./e2b-2.17.0+piping.tone.down.the.tests.configs.eng.3497-py3-none-any.whl |
…he-tests-configs-eng-3497 # Conflicts: # packages/cli/tests/commands/sandbox/backend_integration.test.ts
…e-down-the-tests-configs-eng-3497" This reverts commit cab6388.
…tests-configs-eng-3497 # Conflicts: # packages/cli/tests/commands/sandbox/backend_integration.test.ts
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| userConfig?.domain || | ||
| 'e2b.app' | ||
| const apiKey = process.env.E2B_API_KEY || userConfig?.teamApiKey | ||
| const integrationTest = test.skipIf(isDebug) |
There was a problem hiding this comment.
Missing API key skip guard in exec_pipe tests
Medium Severity
The refactor removed the API key availability check from the test skip guard. Previously, shouldSkip was !hasCreds || isDebug, but now integrationTest only checks isDebug. In environments without E2B_API_KEY set, the test will attempt Sandbox.create (which also lost its explicit apiKey and domain options) and fail with an auth error instead of being gracefully skipped. This is inconsistent with backend_integration.test.ts, which still guards on !apiKey || isDebug.


following up from comments on #1127
Note
Low Risk
Low risk: behavior change is limited to error-handling cleanup when
closeStdinfails, and the rest is test-only refactoring via shared helpers.Overview
Tightens
sandbox execstdin EOF failure handling by inlining a best-effort remotekillwhencloseStdinerrors (to avoid leaving a process blocked on stdin).Refactors CLI integration tests to use shared helpers in
tests/setup.ts(runCli,runCliWithPipedStdin,bufferToText,parseEnvInt,isDebug), centralizing env setup/timeout behavior and adding a probe step inexec_pipe.test.tsto gracefully skip strict byte-count assertions in environments where piped stdin isn’t detected.Written by Cursor Bugbot for commit 05b85e7. This will update automatically on new commits. Configure here.