Repository navigation
test: remove duplicate, dead and private-state SDK tests - #2048
devin-ai-integration[bot] wants to merge 15 commits into
Conversation
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Package ArtifactsBuilt from 9920b32. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-dockerfile-utils-0.1.3-devin-1793224550-remove-irrelevant-sdk-tests.0.tgz ./e2b-2.64.1-devin-1793224550-remove-irrelevant-sdk-tests.0.tgzCLI ( npm install ./e2b-cli-2.24.1-devin-1793224550-remove-irrelevant-sdk-tests.0.tgzCode Interpreter JS SDK ( npm install ./e2b-code-interpreter-2.9.2-devin-1793224550-remove-irrelevant-sdk-tests.0.tgzDesktop JS SDK ( npm install ./e2b-desktop-2.5.1-devin-1793224550-remove-irrelevant-sdk-tests.0.tgzPython SDK ( pip install ./e2b_dockerfile_utils-0.1.2+devin.1793224550.remove.irrelevant.sdk.tests-py3-none-any.whl ./e2b-2.64.0+devin.1793224550.remove.irrelevant.sdk.tests-py3-none-any.whlCode Interpreter Python SDK ( pip install ./e2b_code_interpreter-2.11.10+devin.1793224550.remove.irrelevant.sdk.tests-py3-none-any.whlDesktop Python SDK ( pip install ./e2b_desktop-2.7.2+devin.1793224550.remove.irrelevant.sdk.tests-py3-none-any.whl |
ed7c834 to
76cc85a
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.
Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Note
Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.
🔍 Devin Review: 1 flag
Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
Drop the duplicated second block in api/list.test.ts (keeping one static Sandbox.betaPause call), redundant POST retry cases, the bounded RPC dispatcher case, duplicate handleRpcError Unavailable cases (folding their notInstanceOf checks into the surviving tests), the uncleared handshake timer case and the trivial undici load check. Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Drop duplicate/shape-only tests in client, connection config, rpc exception, logging option and idempotency suites; fold the not-SandboxNotRunning assertion into the port-closed mapping test. Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Drop tests duplicated by stronger counterparts (secure-mode create/connect/write, command envs, pty send_input), a no-assertion watch test, a test of the test module's own wait_for_status helper, and implementation-coupled sync client lifecycle tests. Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
…e payload tests Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
…ync and async Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
…w list test Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
531df79 to
96a4761
Compare
Summary
Removes SDK tests that can't catch a regression the rest of the suite wouldn't already catch. This is the high/medium-confidence delete/merge part of a TDD-style audit of all JS and Python SDK tests (321 files). Rewrites of implementation-coupled tests are in #2049; the gated git suites are removed separately in their own PR. Test-only: no
src/change and no changeset.What's removed:
api/list.test.tshad its whole live-sandbox suite twice, andfiles/write.test.tsdefined'write file'twice. Also duplicate ptysendInputtests in JS/sync/async, and duplicate volume tests.secureoption: it's ignored (envd access is always secured), so thetest_secure.py/secure.test.ts/*_with_secured_envdtests just repeated the default-sandbox tests. One of them had no assertion.test_sync_client_lifecycle.pyonly asserted private attributes. The private_create_sandboxidempotency tests duplicated the public-API ones. Some transport/retry tests asserted class internals or test helpers (e.g.cleanupBaseDir,test_wait_for_status_retries_transport_errors).list with depth(JS, sync, async).Secret.filltests now live intests/shared/secret/test_fill.py. The asyncfrom_dockerfile/to_dockerfiletests were copies oftests/shared/template/methods/*, so they're removed.Every deletion was checked against the remaining suite for any unique behavior it covered. Findings that turned out to be real SDK bugs were left alone (see the rewrite PR).
Verification
pnpm run format,pnpm run lint,pnpm run typecheckpass.934 passed, 1 skippedin touched files.tests/api/list.test.ts(empty running/metadata listings), which fails the same way onmain(3b0f0fedd).Linear: SDK-410
Link to Devin session: https://app.devin.ai/sessions/98def51a61574e17908881f74f2f7ded
Open in Devin Desktop: https://app.devin.ai/desktop/session/98def51a61574e17908881f74f2f7ded?variant=devin
Requested by: @mishushakov
🟢 Devin Review found nothing to fix · View findings