test: verify comfyui.db creation after install #1552
Conversation
📝 WalkthroughWalkthroughThe PR modifies an installation integration test to enhance post-install validation. It updates the test signature to access the window object and adds verification for database file creation and live endpoint response validation alongside existing UI state checks. Changes
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
dosubot
bot
added
size:S
|
This test succeeded, CI is red from an existing bad expectation, to be fixed in #1536 |
Logs are green as well, API check succeeded. |
There was a problem hiding this comment.
Pull request overview
This PR adds integration testing to verify that the ComfyUI database (comfyui.db) is properly created during installation. The test helps catch regressions where the database creation fails (e.g., due to read-only path issues), which would cause the /object_info endpoint to fail and leave the UI stuck on a loading screen.
Changes:
- Add database file existence and size validation to the install integration test
- Add API health check for the
/object_infoendpoint after installation completes
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await expect(installedApp.templatesGrid).toBeVisible({ timeout: 30 * 1000 }); | ||
|
|
||
| const dbPath = path.join(testEnvironment.installLocation.path, 'user', 'comfyui.db'); | ||
| await expect.poll(async () => await pathExists(dbPath), { timeout: 30 * 1000 }).toBe(true); |
There was a problem hiding this comment.
The double await is redundant. Since pathExists already returns a Promise, the inner await is unnecessary. Consider simplifying to: async () => pathExists(dbPath)
| await expect.poll(async () => await pathExists(dbPath), { timeout: 30 * 1000 }).toBe(true); | |
| await expect.poll(async () => pathExists(dbPath), { timeout: 30 * 1000 }).toBe(true); |
| .toBeGreaterThan(0); | ||
| const appUrl = new URL(window.url()); | ||
| const response = await window.request.get(`${appUrl.origin}/object_info`); | ||
| expect(response.ok()).toBe(true); |
There was a problem hiding this comment.
Playwright provides a more idiomatic assertion for checking response status. Consider using expect(response).toBeOK() instead of expect(response.ok()).toBe(true) for better readability and consistency with Playwright best practices.
| expect(response.ok()).toBe(true); | |
| await expect(response).toBeOK(); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/integration/install/installApp.spec.ts`:
- Around line 42-59: The test currently calls
window.request.get(`${appUrl.origin}/object_info`) once which can race with
backend readiness; replace that single request with a polling check using
expect.poll to repeatedly call window.request.get or a small helper that fetches
`${appUrl.origin}/object_info` and returns response.ok(), waiting until it
returns true (or a timeout) before asserting; update the block that follows the
DB size check (references: dbPath, pathExists, stat, appUrl, window.request.get,
/object_info) so the test only proceeds when the endpoint responds successfully.
| const dbPath = path.join(testEnvironment.installLocation.path, 'user', 'comfyui.db'); | ||
| await expect.poll(async () => await pathExists(dbPath), { timeout: 30 * 1000 }).toBe(true); | ||
| await expect | ||
| .poll( | ||
| async () => { | ||
| try { | ||
| const fileStat = await stat(dbPath); | ||
| return fileStat.size; | ||
| } catch { | ||
| return 0; | ||
| } | ||
| }, | ||
| { timeout: 30 * 1000 } | ||
| ) | ||
| .toBeGreaterThan(0); | ||
| const appUrl = new URL(window.url()); | ||
| const response = await window.request.get(`${appUrl.origin}/object_info`); | ||
| expect(response.ok()).toBe(true); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find tests/integration -name "installApp.spec.ts" -type fRepository: Comfy-Org/desktop
Length of output: 105
🏁 Script executed:
fd installApp.spec.ts tests/integrationRepository: Comfy-Org/desktop
Length of output: 161
🏁 Script executed:
git ls-files tests/integration | head -20Repository: Comfy-Org/desktop
Length of output: 1566
🏁 Script executed:
cat -n tests/integration/install/installApp.spec.ts | sed -n '35,70p'Repository: Comfy-Org/desktop
Length of output: 1213
🏁 Script executed:
wc -l tests/integration/install/installApp.spec.tsRepository: Comfy-Org/desktop
Length of output: 108
🏁 Script executed:
grep -r "object_info" tests/integration --include="*.ts" -B 2 -A 2Repository: Comfy-Org/desktop
Length of output: 477
🏁 Script executed:
grep -r "window.request.get" tests/integration --include="*.ts" -B 2 -A 2 | head -40Repository: Comfy-Org/desktop
Length of output: 477
🏁 Script executed:
grep -r "expect.poll" tests/integration --include="*.ts" -A 5 | head -60Repository: Comfy-Org/desktop
Length of output: 543
🏁 Script executed:
cat -n tests/integration/install/installApp.spec.ts | sed -n '1,40p'Repository: Comfy-Org/desktop
Length of output: 1978
🏁 Script executed:
grep -r "response.dispose()" tests/integration --include="*.ts" -B 2 -A 2 | head -30Repository: Comfy-Org/desktop
Length of output: 43
🏁 Script executed:
grep -r "window.request" tests/integration --include="*.ts" | wc -lRepository: Comfy-Org/desktop
Length of output: 62
🏁 Script executed:
grep -r "\.dispose()" tests/integration --include="*.ts"Repository: Comfy-Org/desktop
Length of output: 43
🏁 Script executed:
grep -r "request\\.get\\|request\\.post" tests/integration --include="*.ts" -A 3Repository: Comfy-Org/desktop
Length of output: 367
🏁 Script executed:
cat -n tests/integration/testExtensions.ts | head -50Repository: Comfy-Org/desktop
Length of output: 2317
🏁 Script executed:
cd tests/integration && grep -r "await response" --include="*.ts"Repository: Comfy-Org/desktop
Length of output: 43
🏁 Script executed:
grep -r "APIResponse" tests/integration --include="*.ts"Repository: Comfy-Org/desktop
Length of output: 43
🏁 Script executed:
cat -n tests/integration/testExtensions.ts | sed -n '30,100p'Repository: Comfy-Org/desktop
Length of output: 3172
🏁 Script executed:
grep -r "APIResponse\|response\\.ok\|window\\.request" tests --include="*.ts" --include="*.js"Repository: Comfy-Org/desktop
Length of output: 267
🏁 Script executed:
grep -r "waitUntilLoaded" tests/integration --include="*.ts" -A 3 -B 1Repository: Comfy-Org/desktop
Length of output: 2038
🏁 Script executed:
cat -n tests/integration/testInstalledApp.ts | head -100Repository: Comfy-Org/desktop
Length of output: 1333
🏁 Script executed:
grep -r "Playwright\|APIResponse" tests --include="*.ts" --include="*.json" | head -20Repository: Comfy-Org/desktop
Length of output: 922
🏁 Script executed:
grep -r "response\\.dispose\|response\\.status\|response\\.text\|response\\.json" tests --include="*.ts"Repository: Comfy-Org/desktop
Length of output: 43
🏁 Script executed:
cat -n tests/integration/install/installApp.spec.ts | sed -n '55,61p'Repository: Comfy-Org/desktop
Length of output: 313
🏁 Script executed:
grep -r "expectLoaded\|graphCanvas" tests/integration/testGraphCanvas.ts -A 5 -B 1Repository: Comfy-Org/desktop
Length of output: 350
🏁 Script executed:
git log --oneline tests/integration/install/installApp.spec.ts | head -10Repository: Comfy-Org/desktop
Length of output: 115
🏁 Script executed:
grep -r "object_info\|backend\|server.*ready" tests/integration --include="*.md"Repository: Comfy-Org/desktop
Length of output: 43
Stabilize /object_info check with polling to avoid flakiness.
UI readiness (canvas visible, spinner hidden) doesn't guarantee the backend endpoint is accepting requests yet, creating a potential race condition.
🔧 Suggested stabilization using expect.poll
const dbPath = path.join(testEnvironment.installLocation.path, 'user', 'comfyui.db');
await expect.poll(async () => await pathExists(dbPath), { timeout: 30 * 1000 }).toBe(true);
await expect
.poll(
async () => {
try {
const fileStat = await stat(dbPath);
return fileStat.size;
} catch {
return 0;
}
},
{ timeout: 30 * 1000 }
)
.toBeGreaterThan(0);
const appUrl = new URL(window.url());
- const response = await window.request.get(`${appUrl.origin}/object_info`);
- expect(response.ok()).toBe(true);
+ await expect
+ .poll(
+ async () => {
+ const response = await window.request.get(`${appUrl.origin}/object_info`);
+ const ok = response.ok();
+ await response.dispose();
+ return ok;
+ },
+ { timeout: 30 * 1000 }
+ )
+ .toBe(true);🤖 Prompt for AI Agents
In `@tests/integration/install/installApp.spec.ts` around lines 42 - 59, The test
currently calls window.request.get(`${appUrl.origin}/object_info`) once which
can race with backend readiness; replace that single request with a polling
check using expect.poll to repeatedly call window.request.get or a small helper
that fetches `${appUrl.origin}/object_info` and returns response.ok(), waiting
until it returns true (or a timeout) before asserting; update the block that
follows the DB size check (references: dbPath, pathExists, stat, appUrl,
window.request.get, /object_info) so the test only proceeds when the endpoint
responds successfully.
Verify ComfyUI creates its database after install
Desktop relies on the ComfyUI backend to initialize
comfyui.db; when this fails (e.g., read-only path regressions like #1532),/object_infocan 500 and the UI stays on the loading screen. Add an install e2e check that waits for the DB file in the installuserdirectory, confirms it is non-empty, and then requests/object_infoafter the app loads. This is an integration-level guardrail; it does not validate schema contents, only that the DB is created and the endpoint is healthy.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.