Clean up the sandbox errors test through a class fixture

[devin-ai-integration]

Fixes ENG-5030

Summary

Two review findings on the sandbox.errors integration test merged in #223:

• Cleanup was inline (try/finally around a single test), so an interruption leaked a sandbox. Now an autouse class-scoped cleanup fixture deletes everything the class created, like test_drive_acl.py.
• test_listing_does_not_carry_the_error_history asserted len(page.data) > 0 while creating no sandbox of its own, so it depended on pre-existing workspace state. It now creates its own labeled sandbox first.

Assertions are unchanged: errors is always a list, empty on a healthy sandbox, and empty on a listing (the projection drops the field).

Link to Devin session: https://app.devin.ai/sessions/37e2b2b146c04e8daf2812d81fa0e2e4
Requested by: @drappier-charles

---

Note

Refactors the sandbox errors integration test to use a class-scoped autouse fixture for cleanup (preventing sandbox leaks on test interruption) and makes test_listing_does_not_carry_the_error_history self-contained by creating its own sandbox.

^{Written by Mendral for commit 8dcc8a6.}

[devin-ai-integration]

🤖 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 that start with 'DevinAI' or '@devin'.
• 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

[mendral-app]

🧪 Testing Guide

What this PR addresses

This is a refactor of the test_sandbox_errors.py integration test with two improvements:

1. Resource leak prevention: Replaces inline try/finally cleanup with a class-scoped cleanup fixture that deletes all sandboxes created during the test class, even if a test is interrupted mid-execution.
2. Test isolation fix: test_listing_does_not_carry_the_error_history previously relied on pre-existing workspace state (sandboxes created by other tests). It now creates its own sandbox before asserting.

Steps to verify

Since this is a test refactor with no production code changes, there's nothing to reproduce as a "bug." Instead, verify the following:

1. Run the integration test in isolation:

   pytest tests/integration/core/sandbox/test_sandbox_errors.py -v

2. Confirm both tests pass:
   • test_reads_empty_error_history_on_healthy_sandbox
   • test_listing_does_not_carry_the_error_history
3. Run just the listing test alone to confirm it no longer depends on external state:

   pytest tests/integration/core/sandbox/test_sandbox_errors.py::TestSandboxErrors::test_listing_does_not_carry_the_error_history -v

What to verify (expected behavior)

• ✅ Both tests pass in isolation (no ordering dependency).
• ✅ No sandbox resources are leaked after the test class completes (the cleanup fixture deletes all created sandboxes).
• ✅ Assertions are unchanged: errors is always a list, empty on a healthy sandbox, and listings still don't expose the error history field.
• ✅ No regressions in other sandbox integration tests.

Note

Posted by PR Testing Guide · Tag @mendral-app with feedback.

[mendral-app]

Interaction Flow

sequenceDiagram
    participant Test as TestSandboxErrors
    participant Fixture as cleanup fixture (autouse, class-scoped)
    participant Helper as _create_sandbox()
    participant SDK as SandboxInstance
    participant Infra as Compute Plane

    Note over Fixture: Before tests
    Fixture->>Test: Initialize created_sandboxes = []

    Note over Test: test_reads_empty_error_history_on_healthy_sandbox
    Test->>Helper: _create_sandbox()
    Helper->>Helper: Append name to created_sandboxes
    Helper->>SDK: SandboxInstance.create(name, image, region, labels)
    SDK->>Infra: Provision sandbox
    Helper-->>Test: return name
    Test->>SDK: SandboxInstance.get(name)
    SDK-->>Test: sandbox instance
    Test->>Test: assert errors == []

    Note over Test: test_listing_does_not_carry_the_error_history
    Test->>Helper: _create_sandbox()
    Helper->>SDK: SandboxInstance.create(...)
    SDK->>Infra: Provision sandbox
    Test->>SDK: SandboxInstance.list(limit=1)
    SDK-->>Test: page
    Test->>Test: assert len(page.data) > 0, no .errors attr

    Note over Fixture: After all tests (yield)
    Fixture->>Helper: _safe_delete_sandbox(name) for each
    Helper->>SDK: SandboxInstance.delete(name)
    SDK->>Infra: Tear down sandbox

Loading

Summary

This PR refactors the sandbox.errors integration test to use a class-scoped autouse fixture for cleanup instead of inline try/finally. The key changes to the interaction flow:

1. Centralized lifecycle: Sandbox creation is extracted into _create_sandbox() which tracks names in a class-level list. After all tests complete, the cleanup fixture deletes all created sandboxes via asyncio.gather.
2. Self-contained tests: test_listing_does_not_carry_the_error_history now creates its own sandbox before asserting, removing the implicit dependency on pre-existing workspace state.
3. Resilient teardown: _safe_delete_sandbox swallows exceptions so one failed deletion doesn't block others.

Note

Posted by PR Sequence Diagram · Tag @mendral-app with feedback.

[mendral-app]

LGTM

Clean refactoring. The mutable class-variable pattern (created_sandboxes: list[str] = []) works correctly here because the fixture resets it via request.cls.created_sandboxes = [] and test methods mutate the same object through self.created_sandboxes.append(...). The riskiest hunk is the listing test (line 65–70): it creates a sandbox then asserts page.data[0].errors == [] on a limit=1 result that may return a different sandbox, but the assertion is valid regardless since the listing projection drops errors for all sandboxes.

_{Tag @mendral-app with feedback or questions. View session}

[mendral-app]

📋 Created Linear issue ENG-5030 — status: In Progress

• Assignee: Charles Drappier (reviewer — bot PR)
• Labels: Improvement, SDK, Sandbox
• Estimate: S (1 file, ~46 lines)
• Related: ENG-5029 (the original sandbox errors integration test issue)
• PR linked: ✅ Issue will auto-close when this PR merges

Auto-created because no Linear reference was found in the PR title, description, or branch name.

Note

Posted by Linear Issue Enforcer · Tag @mendral-app with feedback.