test(e2e): align test_sdk_tunnel.py with the rest of the suite's box lifecycle - #1187
Merged
DorianZheng merged 2 commits intoAug 11, 2026
Merged
Conversation
…lifecycle Every other file in apps/e2e/cases uses rt.create() + try/finally: rt.remove() for box lifecycle. test_sdk_tunnel.py was the sole outlier on SimpleBox's async-with convenience wrapper - which is how the auto_remove leak fixed in boxlite-ai#1186 stayed invisible here for so long: the one file with a different lifecycle pattern was the one nobody thought to check. network.tunnel() isn't SimpleBox-specific - the raw Box returned by rt.create() exposes the same handle - so there's no functional reason to keep the indirection. Switching means this file no longer depends on SimpleBox's internal cleanup logic at all, and gets the same explicit, auditable cleanup path as everything else. test_python_sdk_tunnel_rejects_stopped_box now calls box.stop() directly instead of relying on `async with` exit - it's testing tunnel access to a stopped-but-not-deleted box, and after boxlite-ai#1186 exiting a SimpleBox with auto_remove=True deletes the box outright, which would have silently changed what this test exercises. _start_service/_stop_service also move off SimpleBox.exec()'s convenience signature (single ExecResult return) onto the raw Box.exec() -> Execution contract (exec + drain + wait), matching test_lifecycle_comprehensive.py and friends. Verified against the dev cloud API: same pass/xfail/flake pattern as the pre-refactor file (test_python_sdk_tunnel_rejects_stopped_box's XPASS(strict) is a pre-existing, unrelated flake - reproduces identically before and after this change), and dev cloud box count returns to 0 whether the run passes or fails. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe tunnel E2E tests now create boxes through ChangesTunnel E2E lifecycle updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
G4614
marked this pull request as ready for review
August 10, 2026 06:06
📦 BoxLite review — couldn't completepowered by BoxLite |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/e2e/cases/test_sdk_tunnel.py`:
- Around line 272-276: Update the box setup around the boxes collection and
cleanup try/finally block: initialize boxes before try, create each box within
the try scope, and append it immediately after successful creation so cleanup
also runs when a later rt.create call fails. Preserve cleanup for every
successfully created box.
- Around line 51-53: Apply the 30-second asyncio.wait_for timeout to both
drain(ex) calls in apps/e2e/cases/test_sdk_tunnel.py at lines 51-53 and 59-61,
ensuring stream draining cannot block indefinitely before waiting for process
completion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2d55d823-3d60-42f0-b34a-20e16916e877
📒 Files selected for processing (1)
apps/e2e/cases/test_sdk_tunnel.py
4 tasks
CodeRabbit on boxlite-ai#1187: - _start_service/_stop_service: drain(ex) had no timeout of its own, so a hung remote exec would block forever before ever reaching ex.wait()'s 30s timeout. Wrapped both in the same asyncio.wait_for(..., timeout=30). - test_python_sdk_tunnel_keeps_boxes_isolated: `boxes = [await rt.create() for _ in range(2)]` never assigns `boxes` if the second create() raises, so a first box that succeeded remotely would have no reference for `finally` to clean up. Now appended one at a time inside try. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
test_sdk_tunnel.pynow manages box lifecycle withrt.create()/rt.remove()like the rest ofapps/e2e, instead ofSimpleBox.Test plan:
apps/e2e/cases/test_sdk_tunnel.pyagainst the dev cloud API — same pass/xfail pattern before and after this change, dev box count returns to 0 either waySummary by CodeRabbit
Tests
Refactor