-
Notifications
You must be signed in to change notification settings - Fork 1k
test: split Template suites into mocked unit and e2e tiers (4/4) #1745
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 |
|---|---|---|
| @@ -1,25 +1,30 @@ | ||
| import { randomUUID } from 'node:crypto' | ||
| import { expect, test } from 'vitest' | ||
| import { expect } from 'vitest' | ||
| import { Template, waitForTimeout } from '../../src' | ||
| import { e2eTest } from '../setup' | ||
|
|
||
| test('build template in background', async () => { | ||
| const template = Template() | ||
| .fromImage('ubuntu:22.04') | ||
| .skipCache() | ||
| .runCmd('sleep 5') // Add a delay to ensure build takes time | ||
| .setStartCmd('echo "Hello"', waitForTimeout(10_000)) | ||
| e2eTest( | ||
| 'build template in background', | ||
| async () => { | ||
| const template = Template() | ||
| .fromImage('ubuntu:22.04') | ||
| .skipCache() | ||
| .runCmd('sleep 5') // Add a delay to ensure build takes time | ||
| .setStartCmd('echo "Hello"', waitForTimeout(10_000)) | ||
|
|
||
| const name = `e2b-test:v1-${randomUUID()}` | ||
| const name = `e2b-test:v1-${randomUUID()}` | ||
|
|
||
| const buildInfo = await Template.buildInBackground(template, name, { | ||
| cpuCount: 1, | ||
| memoryMB: 1024, | ||
| }) | ||
| const buildInfo = await Template.buildInBackground(template, name, { | ||
| cpuCount: 1, | ||
| memoryMB: 1024, | ||
| }) | ||
|
|
||
| // Should return quickly (within a few seconds), not wait for the full build | ||
| expect(buildInfo).toBeDefined() | ||
| // Should return quickly (within a few seconds), not wait for the full build | ||
| expect(buildInfo).toBeDefined() | ||
|
|
||
| // Verify the build is actually running | ||
| const status = await Template.getBuildStatus(buildInfo) | ||
| expect(status.status).toEqual('building') | ||
| }, 10_000) | ||
| // Verify the build is actually running | ||
| const status = await Template.getBuildStatus(buildInfo) | ||
| expect(status.status).toEqual('building') | ||
| }, | ||
| 10_000 | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,15 @@ | ||
| import { randomUUID } from 'node:crypto' | ||
| import { expect, test } from 'vitest' | ||
| import { expect } from 'vitest' | ||
| import { Template } from '../../src' | ||
| import { e2eTest } from '../setup' | ||
|
|
||
| test('check if base template name exists', async () => { | ||
| e2eTest('check if base template name exists', async () => { | ||
| const exists = await Template.exists('base') | ||
| expect(exists).toBe(true) | ||
| }) | ||
|
|
||
| test('check non existing name', async () => { | ||
| e2eTest('check non existing name', async () => { | ||
| const nonExistingName = `nonexistent-${randomUUID()}` | ||
| const exists = await Template.exists(nonExistingName) | ||
| expect(exists).toBe(false) | ||
|
Check warning on line 14 in packages/js-sdk/tests/template/exists.test.ts
|
||
| }) | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,112 @@ | ||||||||||
| import { mkdtemp, rm, writeFile } from 'node:fs/promises' | ||||||||||
| import { tmpdir } from 'node:os' | ||||||||||
| import { join } from 'node:path' | ||||||||||
| import { afterAll, assert, beforeAll, expect, test } from 'vitest' | ||||||||||
|
|
||||||||||
| import { Template } from '../../src' | ||||||||||
| import { InstructionType } from '../../src/template/types' | ||||||||||
| import { calculateFilesHash } from '../../src/template/utils' | ||||||||||
|
Comment on lines
+7
to
+8
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-54 (one flat entry point per package — everything public is re-exported from |
||||||||||
|
|
||||||||||
| let contextPath: string | ||||||||||
|
|
||||||||||
| beforeAll(async () => { | ||||||||||
| contextPath = await mkdtemp(join(tmpdir(), 'template-serialization-')) | ||||||||||
| await writeFile(join(contextPath, 'app.txt'), 'hello') | ||||||||||
| await writeFile(join(contextPath, 'other.txt'), 'hello') | ||||||||||
| }) | ||||||||||
|
|
||||||||||
| afterAll(async () => { | ||||||||||
| await rm(contextPath, { recursive: true, force: true }) | ||||||||||
| }) | ||||||||||
|
|
||||||||||
| const filesHash = (src: string, dest: string) => | ||||||||||
| calculateFilesHash(src, dest, contextPath, [], false, undefined) | ||||||||||
|
|
||||||||||
| test('hash is stable and content-dependent', async () => { | ||||||||||
| const before = await filesHash('app.txt', '/app/') | ||||||||||
| assert.equal(await filesHash('app.txt', '/app/'), before) | ||||||||||
|
|
||||||||||
| await writeFile(join(contextPath, 'app.txt'), 'hello again') | ||||||||||
| const after = await filesHash('app.txt', '/app/') | ||||||||||
|
|
||||||||||
| assert.notEqual(after, before) | ||||||||||
| assert.match(after, /^[0-9a-f]{64}$/) | ||||||||||
| }) | ||||||||||
|
|
||||||||||
| test('hash covers the source and destination paths', async () => { | ||||||||||
| // Identical content, different instruction — the hash seeds on `COPY src dest`. | ||||||||||
| assert.notEqual( | ||||||||||
| await filesHash('app.txt', '/app/'), | ||||||||||
| await filesHash('other.txt', '/app/') | ||||||||||
| ) | ||||||||||
| assert.notEqual( | ||||||||||
| await filesHash('app.txt', '/app/'), | ||||||||||
| await filesHash('app.txt', '/srv/') | ||||||||||
| ) | ||||||||||
| }) | ||||||||||
|
|
||||||||||
| test('hashing a source that matches no file fails', async () => { | ||||||||||
| // TODO: should reject with TemplateError once calculateFilesHash stops | ||||||||||
| // throwing a bare Error. | ||||||||||
|
Comment on lines
+49
to
+50
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-42 (builder precondition and configuration failures raise
Suggested change
|
||||||||||
| await expect(filesHash('nope.txt', '/app/')).rejects.toThrow() | ||||||||||
| }) | ||||||||||
|
|
||||||||||
| test('serializes a build payload from the builder', async () => { | ||||||||||
| const template = Template({ fileContextPath: contextPath }) | ||||||||||
| .fromImage('ubuntu:22.04') | ||||||||||
| .runCmd('echo hello') | ||||||||||
| .setWorkdir('/app') | ||||||||||
| .setStartCmd('python main.py', 'curl -f http://localhost:8000') | ||||||||||
|
|
||||||||||
| const payload = JSON.parse(await Template.toJSON(template, false)) | ||||||||||
|
|
||||||||||
| assert.equal(payload.fromImage, 'ubuntu:22.04') | ||||||||||
| assert.equal(payload.startCmd, 'python main.py') | ||||||||||
| assert.equal(payload.readyCmd, 'curl -f http://localhost:8000') | ||||||||||
| assert.isUndefined(payload.fromTemplate) | ||||||||||
| assert.deepEqual( | ||||||||||
| payload.steps.map((step: { type: string }) => step.type), | ||||||||||
| [InstructionType.RUN, InstructionType.WORKDIR] | ||||||||||
| ) | ||||||||||
| }) | ||||||||||
|
|
||||||||||
| test('serializes fromTemplate instead of fromImage', async () => { | ||||||||||
| const payload = JSON.parse( | ||||||||||
| await Template.toJSON(Template().fromTemplate('base')) | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| assert.equal(payload.fromTemplate, 'base') | ||||||||||
| assert.isUndefined(payload.fromImage) | ||||||||||
| }) | ||||||||||
|
|
||||||||||
| test('serializes a registry config next to the image', async () => { | ||||||||||
| const template = Template().fromImage('registry.example.com/app:latest', { | ||||||||||
| username: 'user', | ||||||||||
| password: 'pass', | ||||||||||
| }) | ||||||||||
|
|
||||||||||
| const payload = JSON.parse(await Template.toJSON(template)) | ||||||||||
|
|
||||||||||
| assert.equal(payload.fromImage, 'registry.example.com/app:latest') | ||||||||||
| assert.equal(payload.fromImageRegistry.type, 'registry') | ||||||||||
| assert.equal(payload.fromImageRegistry.username, 'user') | ||||||||||
| }) | ||||||||||
|
|
||||||||||
| test('computeHashes adds the copy hash to the payload', async () => { | ||||||||||
| const template = Template({ fileContextPath: contextPath }) | ||||||||||
| .fromImage('ubuntu:22.04') | ||||||||||
| .copy('app.txt', '/app/') | ||||||||||
|
|
||||||||||
| const withoutHashes = JSON.parse(await Template.toJSON(template, false)) | ||||||||||
| const withHashes = JSON.parse(await Template.toJSON(template, true)) | ||||||||||
|
|
||||||||||
| const copyStep = (payload: { | ||||||||||
| steps: { type: string; filesHash?: string }[] | ||||||||||
| }) => payload.steps.find((step) => step.type === InstructionType.COPY) | ||||||||||
|
|
||||||||||
| assert.isUndefined(copyStep(withoutHashes)?.filesHash) | ||||||||||
| assert.equal( | ||||||||||
| copyStep(withHashes)?.filesHash, | ||||||||||
| await filesHash('app.txt', '/app/') | ||||||||||
| ) | ||||||||||
| }) | ||||||||||
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.
🟡 Use
hostedTestinstead ofe2eTestfor tests that hit control-plane/build-only APIs, soE2B_DEBUGstill skips them -- sweep:e2eTest\((seen in backgroundBuild.test.ts and exists.test.ts).hostedTest = e2eTest.skipIf(isDebug)is the variant meant for exactly this case; a local envd cannot serviceTemplate.buildInBackgroundorTemplate.exists, so underE2B_E2E=1+E2B_DEBUGthese tests would hit the wrong server and fail/hang instead of skipping, unlike their Python mirrors which carry@pytest.mark.skip_debug().Extended reasoning...
tests/setup.tsdefines three tiers relevant here:e2eTest = base.skipIf(!isE2E)(no debug guard),hostedTest = e2eTest.skipIf(isDebug), ande2eBuildTemplateTest = buildTemplateTest.skipIf(!isE2E || isDebug). ThehostedTest/e2eBuildTemplateTestvariants exist specifically for operations that a local envd (used underE2B_DEBUG) cannot stand in for: control-plane routes, real builds, snapshots, the traffic proxy.tests/README.mddocuments this convention explicitly, andsandbox/create.test.ts/connect.test.tsalready follow it.This PR wraps
backgroundBuild.test.ts(Template.buildInBackground, a real server-side build) andexists.test.ts(Template.exists->aliasExists->checkAliasExists, a control-planeGET /templates/aliases/{alias}route) in baree2eTestrather thanhostedTest. Neither operation has a local-envd fallback: underE2B_DEBUG,ConnectionConfig.apiUrlresolves tohttp://localhost:3000, which does not serve the builds API or the templates-alias control-plane route.Concrete walkthrough for
exists.test.ts: run withE2B_E2E=1 E2B_DEBUG=1.isE2Eis true soe2eTestdoes not skip.Template.exists(\"base\")callscheckAliasExists, which issuesclient.api.GET(\"/templates/aliases/{alias}\")againsthttp://localhost:3000(the local envd) instead of the real control plane. That route does not exist on envd, so the request fails or 404s, andexpect(exists).toBe(true)fails instead of the test being skipped -- exactly the outcomehostedTestexists to prevent. The same reasoning applies tobackgroundBuild.test.ts, whereTemplate.buildInBackgroundwould attempt a real server-side build against an envd that has no build endpoint.This is also a JS/Python parity gap (a rule from CLAUDE.md): the Python mirrors
test_exists.pyandtest_background_build.py(both sync and async) correctly carry both@pytest.mark.e2eand the pre-existing@pytest.mark.skip_debug(), so only the JS side regresses under the combinedE2B_E2E+E2B_DEBUGconfiguration.Fix is mechanical: replace
e2eTestwithhostedTest(or equivalentlye2eTest.skipIf(isDebug)) as the import and wrapper in both files. Severity is nit -- this only misfires in the niche opt-in combination ofE2B_E2E=1andE2B_DEBUGtogether; the default unit tier and the plain e2e-without-debug tier are unaffected, and it is test-only code with no production impact.