Skip to content

test(sdk): add pty.kill coverage for JS and Python sync/async#1412

Open
mishushakov wants to merge 2 commits into
mainfrom
mishushakov/sdk-test-coverage-gaps
Open

test(sdk): add pty.kill coverage for JS and Python sync/async#1412
mishushakov wants to merge 2 commits into
mainfrom
mishushakov/sdk-test-coverage-gaps

Conversation

@mishushakov

Copy link
Copy Markdown
Member

Adds the previously-missing test coverage for pty.kill(), which was untested across all three SDK implementations despite the rest of the PTY API (create/connect/sendInput/resize) being well covered.

Each suite gets two tests covering both return paths of kill():

  • Kill a live PTY — asserts kill() returns true, then confirms the process is gone via kill -0 <pid> (throws ProcessExitError/CommandExitException).
  • Kill a non-existent PID — asserts kill() returns false, matching the documented not-found behavior.

The tests mirror the style of the existing commands.kill tests and were verified to lint cleanly and be discovered by vitest/pytest (full runs require a live sandbox).

Files

  • packages/js-sdk/tests/sandbox/pty/kill.test.ts
  • packages/python-sdk/tests/sync/sandbox_sync/pty/test_pty_kill.py
  • packages/python-sdk/tests/async/sandbox_async/pty/test_pty_kill.py

🤖 Generated with Claude Code

Covers both return paths of pty.kill(): killing a live PTY (returns
true, process confirmed gone) and killing a non-existent PID (returns
false).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Jun 9, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 1517a2c

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

@cursor

cursor Bot commented Jun 9, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Test-only changes with no production code paths affected.

Overview
Adds sandbox integration tests for pty.kill in the JS SDK and in Python sync/async, covering a successful kill (expects true and the PID gone via kill -0) and killing a missing PID (expects false). No SDK implementation changes.

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

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Package Artifacts

Built from 9398e60. Download artifacts from this workflow run.

JS SDK (e2b@2.28.2-mishushakov-sdk-test-coverage-gaps.0):

npm install ./e2b-2.28.2-mishushakov-sdk-test-coverage-gaps.0.tgz

CLI (@e2b/cli@2.11.1-mishushakov-sdk-test-coverage-gaps.0):

npm install ./e2b-cli-2.11.1-mishushakov-sdk-test-coverage-gaps.0.tgz

Python SDK (e2b==2.27.0+mishushakov-sdk-test-coverage-gaps):

pip install ./e2b-2.27.0+mishushakov.sdk.test.coverage.gaps-py3-none-any.whl

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6695754e75

ℹ️ 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".

Comment thread packages/python-sdk/tests/async/sandbox_async/pty/test_pty_kill.py Outdated
Async Pty.create requires on_data as a positional argument; without it
the test raised TypeError before exercising pty.kill.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants