Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 12 additions & 16 deletions tests/smoketests/blueprints.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { BlueprintView } from '@runloop/api-client/resources/blueprints';
import { makeClient, SHORT_TIMEOUT, uniqueName } from './utils';
import { cleanUpBlueprintsByName, LONG_TIMEOUT, makeClient, MEDIUM_TIMEOUT, uniqueName } from './utils';
import { DevboxView } from '@runloop/api-client/resources/devboxes';

const client = makeClient();
Expand All @@ -13,7 +12,7 @@ describe('smoketest: blueprints', () => {
let blueprintName = uniqueName('bp');

afterAll(async () => {
await client.blueprints.delete(blueprintId!);
await cleanUpBlueprintsByName(client, blueprintName);
});

test(
Expand All @@ -24,13 +23,13 @@ describe('smoketest: blueprints', () => {
name: blueprintName,
},
{
longPoll: { timeoutMs: 30 * 60 * 1000 },
longPoll: { timeoutMs: MEDIUM_TIMEOUT },
},
);
expect(created.status).toBe('build_complete');
blueprintId = created.id;
},
SHORT_TIMEOUT,
LONG_TIMEOUT,
);

test(
Expand All @@ -44,7 +43,7 @@ describe('smoketest: blueprints', () => {
launch_parameters: { resource_size_request: 'X_SMALL', keep_alive_time_seconds: 60 * 5 }, // 5 minutes
},
{
longPoll: { timeoutMs: 20 * 60 * 1000 },
longPoll: { timeoutMs: MEDIUM_TIMEOUT },

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: createAndAwaitRunning creates the devbox before it begins polling and only returns the created view after the devbox reaches the target state. If this new 300-second polling timeout fires, devbox is still undefined, so the finally block cannot shut down the server-side devbox and the smoketest leaks it. [resource leak]

Severity Level: Major ⚠️
- ❌ Blueprint lifecycle tests can leak created devboxes.
- ⚠️ Leaked devboxes consume smoke-test environment resources.
- ⚠️ Both ID and name devbox paths are affected.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** tests/smoketests/blueprints.test.ts
**Line:** 46:46
**Comment:**
	*Resource Leak: `createAndAwaitRunning` creates the devbox before it begins polling and only returns the created view after the devbox reaches the target state. If this new 300-second polling timeout fires, `devbox` is still undefined, so the `finally` block cannot shut down the server-side devbox and the smoketest leaks it.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

},
);
expect(devbox.blueprint_id).toBe(blueprintId);
Expand All @@ -54,7 +53,7 @@ describe('smoketest: blueprints', () => {
}
}
},
SHORT_TIMEOUT,
LONG_TIMEOUT,
);

test(
Expand All @@ -68,7 +67,7 @@ describe('smoketest: blueprints', () => {
launch_parameters: { resource_size_request: 'X_SMALL', keep_alive_time_seconds: 60 * 5 }, // 5 minutes
},
{
longPoll: { timeoutMs: 20 * 60 * 1000 },
longPoll: { timeoutMs: MEDIUM_TIMEOUT },
},
);
expect(devbox.blueprint_id).toBeTruthy();
Expand All @@ -78,7 +77,7 @@ describe('smoketest: blueprints', () => {
}
}
},
SHORT_TIMEOUT,
LONG_TIMEOUT,
);
});

Expand All @@ -89,9 +88,8 @@ describe('smoketest: blueprints', () => {
test.concurrent(
'create blueprint with secret in Dockerfile and await build',
async () => {
let bpt: BlueprintView | undefined;
try {
bpt = await client.blueprints.createAndAwaitBuildCompleted(
const bpt = await client.blueprints.createAndAwaitBuildCompleted(
{
name: secretsBlueprintName,
dockerfile:
Expand All @@ -101,19 +99,17 @@ describe('smoketest: blueprints', () => {
},
},
{
longPoll: { timeoutMs: 30 * 60 * 1000 },
longPoll: { timeoutMs: MEDIUM_TIMEOUT },
},
);

expect(bpt.status).toBe('build_complete');
expect(bpt.parameters.secrets?.['GITHUB_TOKEN']).toBe('GITHUB_TOKEN_FOR_SMOKETESTS');
} finally {
if (bpt) {
await client.blueprints.delete(bpt.id);
}
await cleanUpBlueprintsByName(client, secretsBlueprintName);
}
},
SHORT_TIMEOUT,
LONG_TIMEOUT,
);
});
});
16 changes: 16 additions & 0 deletions tests/smoketests/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,22 @@ export const SHORT_TIMEOUT = 120_000;
export const MEDIUM_TIMEOUT = 300_000;
export const LONG_TIMEOUT = 600_000;

/**
* Helper to clean up every blueprint with the given name, ignoring errors if
* already deleted. Looks blueprints up by name rather than taking an ID so that
* cleanup still runs when a create-and-await call timed out before returning one.
*/
export async function cleanUpBlueprintsByName(client: Runloop, name: string): Promise<void> {
try {
const page = await client.blueprints.list({ name });
for (const blueprint of page.blueprints) {
await client.blueprints.delete(blueprint.id);
}
} catch {
// Already deleted or never created, ignore
}
}

/**
* Helper to clean up a network policy, ignoring errors if already deleted.
*/
Expand Down