-
Notifications
You must be signed in to change notification settings - Fork 1k
test: add Volume mount payload unit and live mount e2e coverage (3/4) #1744
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| import { assert } from 'vitest' | ||
|
|
||
| import { Sandbox, Volume } from '../../src' | ||
| import { hostedTest, template } from '../setup' | ||
|
|
||
| /** | ||
| * Volume content persisting across sandboxes is server-side behavior — the | ||
| * mount happens on real compute, so this is the one volume test that can't be | ||
| * mocked. Everything else about volumes (CRUD, pagination, error mapping, the | ||
| * content API) is asserted against a mocked transport in the unit tier. | ||
| */ | ||
| hostedTest('a mounted volume persists content across sandboxes', async () => { | ||
| const volume = await Volume.create(`test-mount-${Date.now()}`) | ||
|
|
||
| try { | ||
| const writer = await Sandbox.create(template, { | ||
| volumeMounts: { '/mnt/data': volume }, | ||
| }) | ||
| try { | ||
| await writer.files.write('/mnt/data/hello.txt', 'written by the writer') | ||
| } finally { | ||
| await writer.kill() | ||
| } | ||
|
|
||
| const reader = await Sandbox.create(template, { | ||
| volumeMounts: { '/mnt/data': volume }, | ||
| }) | ||
| try { | ||
| assert.equal( | ||
| await reader.files.read('/mnt/data/hello.txt'), | ||
| 'written by the writer' | ||
| ) | ||
| } finally { | ||
| await reader.kill() | ||
| } | ||
| } finally { | ||
| await Volume.destroy(volume.volumeId) | ||
| } | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import { afterAll, afterEach, beforeAll, expect, test } from 'vitest' | ||
| import { http, HttpResponse } from 'msw' | ||
| import { setupServer } from 'msw/node' | ||
|
|
||
| import { Sandbox, Volume } from '../../src' | ||
| import { apiUrl, TEST_API_KEY } from '../setup' | ||
|
|
||
| let lastCreateBody: Record<string, unknown> | undefined | ||
|
|
||
| const server = setupServer( | ||
| http.post(apiUrl('/sandboxes'), async ({ request }) => { | ||
| lastCreateBody = (await request.json()) as Record<string, unknown> | ||
| return HttpResponse.json({ | ||
| sandboxID: 'test-sandbox-id', | ||
| templateID: 'base', | ||
| envdVersion: '0.2.4', | ||
| }) | ||
| }) | ||
| ) | ||
|
|
||
| beforeAll(() => server.listen({ onUnhandledRequest: 'error' })) | ||
| afterAll(() => server.close()) | ||
| afterEach(() => { | ||
| lastCreateBody = undefined | ||
| server.resetHandlers() | ||
| }) | ||
|
|
||
| test('Sandbox.create omits volumeMounts when none are requested', async () => { | ||
| await Sandbox.create('base', { apiKey: TEST_API_KEY }) | ||
|
|
||
| expect(lastCreateBody).not.toHaveProperty('volumeMounts') | ||
| }) | ||
|
|
||
| test('Sandbox.create maps mount paths to named volume mounts', async () => { | ||
| await Sandbox.create('base', { | ||
| apiKey: TEST_API_KEY, | ||
| volumeMounts: { '/mnt/data': 'my-volume' }, | ||
| }) | ||
|
|
||
| expect(lastCreateBody?.volumeMounts).toEqual([ | ||
| { name: 'my-volume', path: '/mnt/data' }, | ||
| ]) | ||
| }) | ||
|
|
||
| test('Sandbox.create accepts a Volume instance as the mount source', async () => { | ||
| const volume = new Volume('vol-1', 'my-volume', 'volume-token') | ||
|
|
||
| await Sandbox.create('base', { | ||
| apiKey: TEST_API_KEY, | ||
| volumeMounts: { '/mnt/data': volume }, | ||
| }) | ||
|
|
||
| expect(lastCreateBody?.volumeMounts).toEqual([ | ||
| { name: 'my-volume', path: '/mnt/data' }, | ||
| ]) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| """Async counterpart of `tests/sync/volume_sync/test_mount.py`.""" | ||
|
|
||
| from uuid import uuid4 | ||
|
|
||
| import pytest | ||
|
|
||
| from e2b import AsyncSandbox, AsyncVolume | ||
|
|
||
|
|
||
| @pytest.mark.e2e | ||
| @pytest.mark.skip_debug() | ||
| async def test_mounted_volume_persists_content_across_sandboxes(template): | ||
| volume = await AsyncVolume.create(f"test-mount-{uuid4()}") | ||
|
|
||
| try: | ||
| writer = await AsyncSandbox.create( | ||
| template, volume_mounts={"/mnt/data": volume} | ||
| ) | ||
| try: | ||
| await writer.files.write("/mnt/data/hello.txt", "written by the writer") | ||
| finally: | ||
| await writer.kill() | ||
|
|
||
| reader = await AsyncSandbox.create( | ||
| template, volume_mounts={"/mnt/data": volume} | ||
| ) | ||
| try: | ||
| assert ( | ||
| await reader.files.read("/mnt/data/hello.txt") | ||
| == "written by the writer" | ||
| ) | ||
| finally: | ||
| await reader.kill() | ||
| finally: | ||
| await AsyncVolume.destroy(volume.volume_id) |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,77 @@ | ||||||
| """Volume mounts in the sandbox create request — pure client-side shaping. | ||||||
|
|
||||||
| Mirrors `tests/volume/mountPayload.test.ts` in the JS SDK. The mount itself is | ||||||
| server-side behavior and is covered by the e2e tier (`test_mount.py`). | ||||||
| """ | ||||||
|
|
||||||
| from types import SimpleNamespace | ||||||
| from typing import Any, Dict | ||||||
| from unittest.mock import AsyncMock, Mock | ||||||
|
|
||||||
| from e2b import AsyncSandbox, Sandbox, Volume | ||||||
| from e2b.api.client.api.sandboxes import post_sandboxes | ||||||
| from e2b.api.client.models import Sandbox as SandboxModel | ||||||
|
|
||||||
|
|
||||||
| def _created_sandbox(): | ||||||
| return SimpleNamespace( | ||||||
| status_code=200, | ||||||
| parsed=SandboxModel( | ||||||
| client_id="client-id", | ||||||
| envd_version="0.2.4", | ||||||
| sandbox_id="sbx-test", | ||||||
| template_id="template-id", | ||||||
| ), | ||||||
| ) | ||||||
|
|
||||||
|
|
||||||
| def _sync_request_body(monkeypatch, api_key: str, volume_mounts) -> Dict[str, Any]: | ||||||
| request = Mock(return_value=_created_sandbox()) | ||||||
| monkeypatch.setattr(post_sandboxes, "sync_detailed", request) | ||||||
|
|
||||||
| Sandbox.create(api_key=api_key, volume_mounts=volume_mounts) | ||||||
|
|
||||||
| return request.call_args.kwargs["body"].to_dict() | ||||||
|
|
||||||
|
|
||||||
| async def _async_request_body( | ||||||
| monkeypatch, api_key: str, volume_mounts | ||||||
| ) -> Dict[str, Any]: | ||||||
| request = AsyncMock(return_value=_created_sandbox()) | ||||||
| monkeypatch.setattr(post_sandboxes, "asyncio_detailed", request) | ||||||
|
|
||||||
| await AsyncSandbox.create(api_key=api_key, volume_mounts=volume_mounts) | ||||||
|
|
||||||
| return request.call_args.kwargs["body"].to_dict() | ||||||
|
|
||||||
|
|
||||||
| def test_create_omits_volume_mounts_when_none_are_requested(monkeypatch, test_api_key): | ||||||
| body = _sync_request_body(monkeypatch, test_api_key, None) | ||||||
|
|
||||||
| assert "volumeMounts" not in body | ||||||
|
|
||||||
|
|
||||||
| def test_create_maps_mount_paths_to_named_volume_mounts(monkeypatch, test_api_key): | ||||||
| body = _sync_request_body(monkeypatch, test_api_key, {"/mnt/data": "my-volume"}) | ||||||
|
|
||||||
| assert body["volumeMounts"] == [{"name": "my-volume", "path": "/mnt/data"}] | ||||||
|
|
||||||
|
|
||||||
| def test_create_accepts_a_volume_instance_as_the_mount_source( | ||||||
| monkeypatch, test_api_key | ||||||
| ): | ||||||
| volume = Volume("vol-1", "my-volume", "volume-token") | ||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. T-4 / T-3 — same constructor point as the JS side, plus a local inconsistency: At minimum match the existing convention:
Suggested change
Better, mirror whatever the JS test ends up doing and obtain the instance from |
||||||
|
|
||||||
| body = _sync_request_body(monkeypatch, test_api_key, {"/mnt/data": volume}) | ||||||
|
|
||||||
| assert body["volumeMounts"] == [{"name": "my-volume", "path": "/mnt/data"}] | ||||||
|
|
||||||
|
|
||||||
| async def test_async_create_maps_mount_paths_to_named_volume_mounts( | ||||||
| monkeypatch, test_api_key | ||||||
| ): | ||||||
| body = await _async_request_body( | ||||||
| monkeypatch, test_api_key, {"/mnt/data": "my-volume"} | ||||||
| ) | ||||||
|
|
||||||
| assert body["volumeMounts"] == [{"name": "my-volume", "path": "/mnt/data"}] | ||||||
|
Comment on lines
+70
to
+77
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. T-1 / T-2 — sync and async are full mirrors with the same method names and semantics, so their coverage shouldn't diverge either. Sync gets three cases (omitted, string name, Add the two missing async cases ( |
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| """Volume content persisting across sandboxes — real mounts, real compute. | ||
|
|
||
| Everything else about volumes (CRUD, pagination, error mapping, the content | ||
| API) is asserted against a mocked transport in the default tier; only the mount | ||
| behavior needs live infrastructure. Mirrors `tests/volume/mount.test.ts`. | ||
| """ | ||
|
|
||
| from uuid import uuid4 | ||
|
|
||
| import pytest | ||
|
|
||
| from e2b import Sandbox, Volume | ||
|
|
||
|
|
||
| @pytest.mark.e2e | ||
| @pytest.mark.skip_debug() | ||
| def test_mounted_volume_persists_content_across_sandboxes(template): | ||
| volume = Volume.create(f"test-mount-{uuid4()}") | ||
|
|
||
| try: | ||
| writer = Sandbox.create(template, volume_mounts={"/mnt/data": volume}) | ||
| try: | ||
| writer.files.write("/mnt/data/hello.txt", "written by the writer") | ||
| finally: | ||
| writer.kill() | ||
|
|
||
| reader = Sandbox.create(template, volume_mounts={"/mnt/data": volume}) | ||
| try: | ||
| assert reader.files.read("/mnt/data/hello.txt") == "written by the writer" | ||
| finally: | ||
| reader.kill() | ||
| finally: | ||
| Volume.destroy(volume.volume_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
T-4 — objects with remote lifecycles are obtained through async static factories (
Volume.create,Volume.list); constructors are internal wiring. This is the firstnew Volume(...)call site in the JS SDK, and a test is documentation: it cements a construction path TASTE says users never take, and it also spells out the optionaltokenpositionally, which is exactly the chain T-3 rules out.msw is already intercepting here, so the compliant form is to get the volume the way a caller would — add a
http.post(apiUrl('/volumes'), …)handler returning{ volumeID: 'vol-1', name: 'my-volume', token: 'volume-token' }and thenconst volume = await Volume.create('my-volume', { apiKey: TEST_API_KEY }). That keeps the test asserting the public path end to end instead of a hand-assembled object.