Test new error handling indicating when the sandbox is not found #540
Conversation
|
Co-authored-by: Jakub Novák <[email protected]>
This is more of an integration test rather than a unit test, do you think it's necessary? |
0div
force-pushed
the
return-error-indicating-that-the-sandbox-is-not-running-when-e2b-1327-test
branch
from
f0def1b to
a9c283e
Compare
…not-running-when-e2b-1327-test
…or-indicating-that-the-sandbox-is-not-running-when-e2b-1327-test
|
After we resolve the conflict this should be good to go—it is deployed on prod, right @jakubno ? |
|
Btw, what was the reason for not adding the Python version? |
ValentaTomas
added
Improvement
@ValentaTomas felt like we are doing more of an integration test here, asserting that the error handling on the backend works as expected, rather than how the SDKs handle this themselves. |
…not-running-when-e2b-1327-test
| sandboxTest.skipIf(isDebug)( | ||
| 'ping server in non-running sandbox', | ||
| async ({ sandbox }) => { | ||
| const host = sandbox.getHost(49983) |
There was a problem hiding this comment.
the port would be better in a constant
There was a problem hiding this comment.
why exactly? it's only used here.
There was a problem hiding this comment.
The port 49983 is the envd port on which every sandbox can be accessed. I understand that you know it, but I don't consider it being quite obvious just from the number itself if you do have the whole infra context in mind
There was a problem hiding this comment.
i see, i'd rather not have to worry about this special port and explaining it; i went ahead and updated the test to make it independent of this.
There was a problem hiding this comment.
It looks like you are still using it
|
@0div Is this still blocked by PR/deploy of main/prod? |
…not-running-when-e2b-1327-test
ValentaTomas
requested review from
dobrac
and removed request for
ValentaTomas and
mishushakov
…not-running-when-e2b-1327-test
|
@0div Let's just resolve the comments and merge—we can move to integration tests later. |
…or-indicating-that-the-sandbox-is-not-running-when-e2b-1327-test
| const host = sandbox.getHost(49983) | ||
| const url = `${isDebug ? 'http' : 'https'}://${host}/health` | ||
|
|
||
| const res = await fetch(url) | ||
|
|
||
| assert.equal(res.status, 204) | ||
|
|
||
| await sandbox.kill() | ||
|
|
||
| const res2 = await fetch(url) | ||
| assert.equal(res2.status, 502) | ||
|
|
||
| const text = await res2.text() |
There was a problem hiding this comment.
you can simplify here (there's already test for running sandbox)
| const host = sandbox.getHost(49983) | |
| const url = `${isDebug ? 'http' : 'https'}://${host}/health` | |
| const res = await fetch(url) | |
| assert.equal(res.status, 204) | |
| await sandbox.kill() | |
| const res2 = await fetch(url) | |
| assert.equal(res2.status, 502) | |
| const text = await res2.text() | |
| const host = sandbox.getHost(3000) | |
| const url = `https://{host}` | |
| await sandbox.kill() | |
| const res = await fetch(url) | |
| assert.equal(res.status, 502) | |
| const text = await res.text() |
|
|
||
| test('connect to non-running sandbox', async () => { | ||
| const sbx = await Sandbox.create(template, { timeoutMs: 10_000 }) | ||
| let isKilled = false | ||
|
|
||
| try { | ||
| const isRunning = await sbx.isRunning() | ||
| assert.isTrue(isRunning) | ||
| await sbx.kill() | ||
| isKilled = true | ||
|
|
||
| const sbxConnection = await Sandbox.connect(sbx.sandboxId) | ||
| const isRunning2 = await sbxConnection.isRunning() | ||
| assert.isFalse(isRunning2) | ||
| } finally { | ||
| if (!isKilled) { | ||
| await sbx.kill() | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
| test('connect to non-running sandbox', async () => { | |
| const sbx = await Sandbox.create(template, { timeoutMs: 10_000 }) | |
| let isKilled = false | |
| try { | |
| const isRunning = await sbx.isRunning() | |
| assert.isTrue(isRunning) | |
| await sbx.kill() | |
| isKilled = true | |
| const sbxConnection = await Sandbox.connect(sbx.sandboxId) | |
| const isRunning2 = await sbxConnection.isRunning() | |
| assert.isFalse(isRunning2) | |
| } finally { | |
| if (!isKilled) { | |
| await sbx.kill() | |
| } | |
| } | |
| }) | |
| sandboxTest('connect to non-running sandbox', async ({ sandbox }) => { | |
| const isRunning = await sbx.isRunning() | |
| assert.isTrue(isRunning) | |
| await sbx.kill() | |
| const sbxConnection = await Sandbox.connect(sbx.sandboxId) | |
| const isRunning2 = await sbxConnection.isRunning() | |
| assert.isFalse(isRunning2) | |
| }) |
| @@ -2,30 +2,63 @@ import { assert } from 'vitest' | |||
|
|
|||
| import { isDebug, sandboxTest, wait } from '../setup.js' | |||
|
|
|||
| sandboxTest('ping server in sandbox', async ({ sandbox }) => { | |||
| const cmd = await sandbox.commands.run('python -m http.server 8000', { background: true }) | |||
| sandboxTest.skipIf(isDebug)( | |||
There was a problem hiding this comment.
this should work in debug right? maybe you could use nonstandard random port to prevent some port conflict
| sandboxTest.skipIf(isDebug)( | |
| sandboxTest( |
Added a test in
js-sdkfor e2b-dev/infra#231