Skip to content

test: skip the deprecated git suites unless ENABLE_GIT_TESTS is set - #1863

Merged
mishushakov merged 4 commits into
mainfrom
devin/1789047533-skip-git-tests
Sep 10, 2026
Merged

test: skip the deprecated git suites unless ENABLE_GIT_TESTS is set#1863
mishushakov merged 4 commits into
mainfrom
devin/1789047533-skip-git-tests

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

The sandbox git API is deprecated, so its test suites are now opt-in behind an ENABLE_GIT_TESTS env var — the same pattern the live volume tests used with ENABLE_VOLUME_TESTS (#1526) before they were mocked. Everything under packages/js-sdk/tests/sandbox/git/ and packages/python-sdk/tests/shared/git/ is skipped by default, including the offline unit tests (helpers.test.ts, validation.test.ts, test_args.py, test_parity.py).

  • JStests/setup.ts gains isGitTestsEnabled and a gitTest = sandboxTest.skipIf(!isGitTestsEnabled) fixture; the sandbox-backed git tests switch from sandboxTest to gitTest, and the two unit files use describe.skipIf / test.skipIf with the same flag.
  • Pythontests/shared/git/conftest.py adds an autouse fixture that calls pytest.skip("skipped because ENABLE_GIT_TESTS is not set"); autouse fixtures resolve before git_sandbox, so no sandbox is created for skipped tests.

Nothing in CI sets the flag, so the git suites no longer run there. To run them locally:

ENABLE_GIT_TESTS=1 pnpm run test              # js-sdk
ENABLE_GIT_TESTS=1 uv run pytest tests/shared/git  # python-sdk

Verified: without the flag vitest reports 14 files / 32 tests skipped and pytest 31 skipped (no E2B_API_KEY needed); with the flag the offline unit tests pass in both SDKs.

Tests only — no changeset.

Link to Devin session: https://app.devin.ai/sessions/6e7c9f24da3c4eeab560a687e913a7b6
Open in Devin Desktop: https://app.devin.ai/desktop/session/6e7c9f24da3c4eeab560a687e913a7b6?variant=devin
Requested by: @mishushakov

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@cla-bot cla-bot Bot added the cla-signed label Sep 10, 2026
@changeset-bot

changeset-bot Bot commented Sep 10, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 79d0322

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

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

TASTE.md review: no violations found. This PR touches only test fixtures and conftest (packages/js-sdk/tests/**, packages/python-sdk/tests/shared/git/conftest.py) — no public SDK surface changes, so the rules that could apply (naming/casing T-12, parity T-1/T-2, options shape T-3, env-var config T-49/T-50, deprecation T-65–T-68) are either not engaged or respected.

Checked and clean:

  • Naming/casing (T-12): isGitTestsEnabled, gitTest, skip_unless_git_tests_enabled case acronym-free words consistently; matches the existing isDebug / volumeTest fixtures.
  • Parity (T-1, T-2): the JS and Python halves gate on the same ENABLE_GIT_TESTS flag with the idiomatic mechanism on each side (skipIf fixture vs. autouse fixture), mirroring the ENABLE_VOLUME_TESTS precedent.
  • Deprecation (T-65–T-68): nothing in the deprecated git public surface is removed or renamed — only its test execution is made opt-in.

One non-blocking note, not a violation: ENABLE_GIT_TESTS is treated as set when it is the empty string (!== undefined / is None), which is the inverse of the SDK's own convention that an empty env var means unset (T-50). T-50 scopes itself to E2B_-prefixed SDK config and the existing isDebug fixture uses the same !== undefined check, so consistency with the surrounding test setup is the right call here.

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Package Artifacts

Built from 86f0e9e. Download artifacts from this workflow run.

JS SDK (e2b@2.49.1-devin-1789047533-skip-git-tests.0):

npm install ./e2b-2.49.1-devin-1789047533-skip-git-tests.0.tgz

CLI (@e2b/cli@2.19.1-devin-1789047533-skip-git-tests.0):

npm install ./e2b-cli-2.19.1-devin-1789047533-skip-git-tests.0.tgz

Code Interpreter JS SDK (@e2b/code-interpreter@2.8.1-devin-1789047533-skip-git-tests.0):

npm install ./e2b-code-interpreter-2.8.1-devin-1789047533-skip-git-tests.0.tgz

Desktop JS SDK (@e2b/desktop@2.4.1-devin-1789047533-skip-git-tests.0):

npm install ./e2b-desktop-2.4.1-devin-1789047533-skip-git-tests.0.tgz

Python SDK (e2b==2.49.0+devin.1789047533.skip.git.tests):

pip install ./e2b-2.49.0+devin.1789047533.skip.git.tests-py3-none-any.whl

Code Interpreter Python SDK (e2b-code-interpreter==2.10.0+devin.1789047533.skip.git.tests):

pip install ./e2b_code_interpreter-2.10.0+devin.1789047533.skip.git.tests-py3-none-any.whl

Desktop Python SDK (e2b-desktop==2.5.0+devin.1789047533.skip.git.tests):

pip install ./e2b_desktop-2.5.0+devin.1789047533.skip.git.tests-py3-none-any.whl

Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
@mishushakov
mishushakov marked this pull request as ready for review September 10, 2026 13:43
@mishushakov
mishushakov self-requested a review as a code owner September 10, 2026 13:43

@claude claude 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.

Looks good — a simple, test-only change that follows the existing ENABLE_VOLUME_TESTS pattern.

  • Checked packages/js-sdk/tests/setup.ts: isGitTestsEnabled/gitTest fixture wraps sandboxTest with .skipIf, mirroring the existing volume-test gate.
  • Checked every git/*.test.ts file switched from sandboxTest/bare test to the new gated fixture, plus unrelated reformatting hunks — no logic changes beyond the import/skip wiring.
  • Checked packages/python-sdk/tests/shared/git/conftest.py: autouse skip_unless_git_tests_enabled fixture applies the same gate to sync/async shared git tests, consistent with the JS-side change.
  • Confirmed the autouse fixture also skips the offline test_args.py/test_parity.py unit tests in that directory, which matches the PR description's stated intent rather than being an unintended side effect.
Extended reasoning...

Overview

This PR touches only test files: it gates the deprecated sandbox git API test suites (both JS and Python SDKs) behind an ENABLE_GIT_TESTS environment variable, following the same pattern previously used for ENABLE_VOLUME_TESTS. No product/source code is modified. The JS side adds an isGitTestsEnabled flag and gitTest fixture in tests/setup.ts, and every packages/js-sdk/tests/sandbox/git/*.test.ts file (plus validation.test.ts, which wraps vitest's own test) switches to use it. The Python side adds an autouse skip_unless_git_tests_enabled fixture in packages/python-sdk/tests/shared/git/conftest.py, applied to the whole shared/git directory (used by both sync and async suites).

Security risks

None. This is a test-infrastructure-only change; it does not touch authentication, authorization, request handling, or any runtime code path exposed to users.

Level of scrutiny

Low. The change is mechanical (import swap + .skipIf/pytest.skip gating), self-contained to test files, and directly mirrors an established, previously-reviewed pattern (ENABLE_VOLUME_TESTS). CLAUDE.md's requirement to mirror JS/Python sync+async changes is satisfied here since both SDKs' git suites are gated identically, and no changeset is required since only test files changed.

Other factors

The bug hunter reported no bugs. Two candidate issues were raised and ruled out as duplicates of each other: the autouse fixture in the Python conftest also disables the offline test_args.py/test_parity.py unit tests, not just live-sandbox tests. I independently verified this is true (both files live in packages/python-sdk/tests/shared/git/, so the autouse fixture does apply to them) — but the PR description explicitly states this is intentional ("Everything under ... is skipped by default, including the offline unit tests"), so it is a deliberate scope decision rather than an oversight. Given the mechanical nature of the diff, its narrow test-only scope, and the absence of any outstanding CHANGES_REQUESTED review in the timeline, I'm confident this does not need further human scrutiny.

@mishushakov
mishushakov enabled auto-merge (squash) September 10, 2026 16:39
@mishushakov
mishushakov merged commit 804021e into main Sep 10, 2026
74 of 77 checks passed
@mishushakov
mishushakov deleted the devin/1789047533-skip-git-tests branch September 10, 2026 17:22
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.

1 participant