Compare commits
29 Commits
extension-
...
v1.42.6
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
736f0fa416 | ||
|
|
d8d8aeb99e | ||
|
|
f4868894fa | ||
|
|
a96c61d2c2 | ||
|
|
c420cff4f0 | ||
|
|
8ccfe852b4 | ||
|
|
e2ef041170 | ||
|
|
a4952e9c45 | ||
|
|
64c852bf82 | ||
|
|
697087fdca | ||
|
|
a7218b2922 | ||
|
|
39a774bc15 | ||
|
|
74dedc314d | ||
|
|
5ae747e7c1 | ||
|
|
21069d54e7 | ||
|
|
8a456043e8 | ||
|
|
585e6f87fa | ||
|
|
4781775a78 | ||
|
|
74a48ab2aa | ||
|
|
0875e2f50f | ||
|
|
63442d2fb0 | ||
|
|
bb6f00dc68 | ||
|
|
11fc09220c | ||
|
|
16f4f3f3ed | ||
|
|
4c5a49860c | ||
|
|
db5e8961e0 | ||
|
|
9f9fa60137 | ||
|
|
7131c274f3 | ||
|
|
82556f02a9 |
221
.claude/skills/red-green-fix/SKILL.md
Normal file
@@ -0,0 +1,221 @@
|
||||
---
|
||||
name: red-green-fix
|
||||
description: 'Bug fix workflow that proves test validity with a red-then-green CI sequence. Commits a failing test first (CI red), then the minimal fix (CI green). Use when fixing a bug, writing a regression test, or when asked to prove a fix works.'
|
||||
---
|
||||
|
||||
# Red-Green Fix
|
||||
|
||||
Fixes bugs as two commits so CI automatically proves the test catches the bug.
|
||||
|
||||
## Why Two Commits
|
||||
|
||||
If you commit the test and fix together, the test always passes — reviewers cannot tell whether the test actually detects the bug or is a no-op. Splitting into two commits creates a verifiable CI trail:
|
||||
|
||||
1. **Commit 1 (test-only)** — adds a test that exercises the bug. CI runs it → test fails → red X.
|
||||
2. **Commit 2 (fix)** — adds the minimal fix. CI runs the same test → test passes → green check.
|
||||
|
||||
The red-then-green sequence in the commit history proves the test is valid.
|
||||
|
||||
## Input
|
||||
|
||||
The user provides a bug description as an argument. If no description is given, ask the user to describe the bug before proceeding.
|
||||
|
||||
Bug description: $ARGUMENTS
|
||||
|
||||
## Step 0 — Setup
|
||||
|
||||
Create an isolated branch from main:
|
||||
|
||||
```bash
|
||||
git fetch origin main
|
||||
git checkout -b fix/<bug-name> origin/main
|
||||
```
|
||||
|
||||
## Step 1 — Red: Failing Test Only
|
||||
|
||||
Write a test that reproduces the bug. **Do NOT write any fix code.**
|
||||
|
||||
### Choosing the Test Framework
|
||||
|
||||
| Bug type | Framework | File location |
|
||||
| --------------------------------- | ---------- | ------------------------------- |
|
||||
| Logic, utils, stores, composables | Vitest | `src/**/*.test.ts` (colocated) |
|
||||
| UI interaction, canvas, workflows | Playwright | `browser_tests/tests/*.spec.ts` |
|
||||
|
||||
For Playwright tests, follow the `/writing-playwright-tests` skill for patterns, fixtures, and tags.
|
||||
|
||||
### Rules
|
||||
|
||||
- The test MUST fail against the current codebase (this is the whole point)
|
||||
- Do NOT modify any source code outside of test files
|
||||
- Do NOT include any fix, workaround, or behavioral change
|
||||
- Do NOT add unrelated tests or refactor existing tests
|
||||
- Keep the test minimal — only what is needed to reproduce the bug
|
||||
- Avoid common anti-patterns — see `reference/testing-anti-patterns.md`
|
||||
|
||||
### Vitest Example
|
||||
|
||||
```typescript
|
||||
// src/utils/pathUtil.test.ts
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { resolveModelPath } from './pathUtil'
|
||||
|
||||
describe('resolveModelPath', () => {
|
||||
it('handles absolute paths from folder_paths API', () => {
|
||||
const result = resolveModelPath(
|
||||
'/absolute/models',
|
||||
'/absolute/models/checkpoints'
|
||||
)
|
||||
expect(result).toBe('/absolute/models/checkpoints')
|
||||
})
|
||||
})
|
||||
```
|
||||
|
||||
### Playwright Example
|
||||
|
||||
```typescript
|
||||
import {
|
||||
comfyPageFixture as test,
|
||||
comfyExpect as expect
|
||||
} from '../fixtures/ComfyPage'
|
||||
|
||||
test.describe('Model Download', { tag: ['@smoke'] }, () => {
|
||||
test('downloads model when path is absolute', async ({ comfyPage }) => {
|
||||
await comfyPage.workflow.loadWorkflow('missing-model')
|
||||
const downloadBtn = comfyPage.page.getByTestId('download-model-button')
|
||||
await downloadBtn.click()
|
||||
await expect(comfyPage.page.getByText('Download complete')).toBeVisible()
|
||||
})
|
||||
})
|
||||
```
|
||||
|
||||
### Verify Locally First
|
||||
|
||||
Run the test locally before pushing to confirm it fails for the right reason:
|
||||
|
||||
```bash
|
||||
# Vitest
|
||||
pnpm test:unit -- <test-file>
|
||||
|
||||
# Playwright
|
||||
pnpm test:browser:local -- --grep "<test name>"
|
||||
```
|
||||
|
||||
If the test passes locally, it does not reproduce the bug — revisit your test before pushing.
|
||||
|
||||
### Quality Checks and Commit
|
||||
|
||||
```bash
|
||||
pnpm typecheck
|
||||
pnpm lint
|
||||
pnpm format:check
|
||||
|
||||
git add <test-files-only>
|
||||
git commit -m "test: add failing test for <concise bug description>"
|
||||
git push -u origin HEAD
|
||||
```
|
||||
|
||||
### Verify CI Failure
|
||||
|
||||
```bash
|
||||
gh run list --branch $(git branch --show-current) --limit 1
|
||||
```
|
||||
|
||||
**STOP HERE.** Inform the user of the CI status and wait for confirmation before proceeding to Step 2.
|
||||
|
||||
- If CI passes: the test does not catch the bug. Revisit the test.
|
||||
- If CI fails for unrelated reasons: investigate and fix the test setup, not the bug.
|
||||
- If CI fails because the test correctly catches the bug: proceed to Step 2.
|
||||
|
||||
## Step 2 — Green: Minimal Fix
|
||||
|
||||
Write the minimum code change needed to make the failing test pass.
|
||||
|
||||
### Rules
|
||||
|
||||
- Do NOT modify, weaken, or delete the test from Step 1 — it is immutable. If the test needs changes, restart from Step 1 and re-prove the red.
|
||||
- Do NOT add new tests (tests were finalized in Step 1)
|
||||
- Do NOT refactor, clean up, or make "drive-by" improvements
|
||||
- Do NOT modify code unrelated to the bug
|
||||
- The fix should be the smallest correct change
|
||||
|
||||
### Quality Checks and Commit
|
||||
|
||||
```bash
|
||||
pnpm typecheck
|
||||
pnpm lint
|
||||
pnpm format
|
||||
|
||||
git add <fix-files-only>
|
||||
git commit -m "fix: <concise bug description>"
|
||||
git push
|
||||
```
|
||||
|
||||
### Verify CI Pass
|
||||
|
||||
```bash
|
||||
gh run list --branch $(git branch --show-current) --limit 1
|
||||
```
|
||||
|
||||
- If CI passes: the fix is verified. Proceed to PR creation.
|
||||
- If CI fails: investigate and fix. Do NOT change the test from Step 1.
|
||||
|
||||
## Step 3 — Open Pull Request
|
||||
|
||||
```bash
|
||||
gh pr create --title "fix: <description>" --body "$(cat <<'EOF'
|
||||
## Summary
|
||||
|
||||
<Brief explanation of the bug and root cause>
|
||||
|
||||
- Fixes #<issue-number>
|
||||
|
||||
## Red-Green Verification
|
||||
|
||||
| Commit | CI Status | Purpose |
|
||||
|--------|-----------|---------|
|
||||
| `test: ...` | :red_circle: Red | Proves the test catches the bug |
|
||||
| `fix: ...` | :green_circle: Green | Proves the fix resolves the bug |
|
||||
|
||||
## Test Plan
|
||||
|
||||
- [ ] CI red on test-only commit
|
||||
- [ ] CI green on fix commit
|
||||
- [ ] Added/updated E2E regression under `browser_tests/` or explained why not applicable
|
||||
- [ ] Manual verification (if applicable)
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
## Gotchas
|
||||
|
||||
### CI fails on test commit for unrelated reasons
|
||||
|
||||
Lint, typecheck, or other tests may fail — not just your new test. Check the CI logs carefully. If the failure is unrelated, fix it in a separate commit before the `test:` commit so the red X is clearly attributable to your test.
|
||||
|
||||
### Test passes when it should fail
|
||||
|
||||
The bug may only manifest under specific conditions (e.g., Windows paths, external model directories, certain workflow structures). Make sure your test setup matches the actual bug scenario. Check that you're not accidentally testing the happy path.
|
||||
|
||||
### Flaky Playwright tests
|
||||
|
||||
If your e2e test is intermittent, it doesn't prove anything. Use retrying assertions (`toBeVisible`, `toHaveText`) instead of `waitForTimeout`. See the `/writing-playwright-tests` skill for anti-patterns.
|
||||
|
||||
### Pre-existing CI failures on main
|
||||
|
||||
If main itself is red, branch from the last green commit or fix the pre-existing failure first. A red-green proof is meaningless if the baseline is already red.
|
||||
|
||||
## Reference
|
||||
|
||||
| Resource | Path |
|
||||
| --------------------- | -------------------------------------------------- |
|
||||
| Unit test framework | Vitest (`src/**/*.test.ts`) |
|
||||
| E2E test framework | Playwright (`browser_tests/tests/*.spec.ts`) |
|
||||
| E2E fixtures | `browser_tests/fixtures/` |
|
||||
| E2E assets | `browser_tests/assets/` |
|
||||
| Playwright skill | `.claude/skills/writing-playwright-tests/SKILL.md` |
|
||||
| Unit CI | `.github/workflows/ci-tests-unit.yaml` |
|
||||
| E2E CI | `.github/workflows/ci-tests-e2e.yaml` |
|
||||
| Lint CI | `.github/workflows/ci-lint-format.yaml` |
|
||||
| Testing anti-patterns | `reference/testing-anti-patterns.md` |
|
||||
| Related skill | `.claude/skills/perf-fix-with-proof/SKILL.md` |
|
||||
214
.claude/skills/red-green-fix/reference/testing-anti-patterns.md
Normal file
@@ -0,0 +1,214 @@
|
||||
# Testing Anti-Patterns for Red-Green Fixes
|
||||
|
||||
Common mistakes that undermine the red-green proof. Avoid these when writing the test commit (Step 1).
|
||||
|
||||
## Testing Implementation Details
|
||||
|
||||
Test observable behavior, not internal state.
|
||||
|
||||
**Bad** — coupling to internals:
|
||||
|
||||
```typescript
|
||||
it('uses cache internally', () => {
|
||||
const service = new UserService()
|
||||
service.getUser(1)
|
||||
expect(service._cache.has(1)).toBe(true) // Implementation detail
|
||||
})
|
||||
```
|
||||
|
||||
**Good** — testing through the public interface:
|
||||
|
||||
```typescript
|
||||
it('returns same user on repeated calls', async () => {
|
||||
const service = new UserService()
|
||||
const user1 = await service.getUser(1)
|
||||
const user2 = await service.getUser(1)
|
||||
expect(user1).toBe(user2) // Behavior, not implementation
|
||||
})
|
||||
```
|
||||
|
||||
Why this matters for red-green: if your test is coupled to internals, a valid fix that changes the implementation may break the test — even though the bug is fixed. The green commit should only require changing source code, not rewriting the test.
|
||||
|
||||
## Assertion-Free Tests
|
||||
|
||||
Every test must assert something meaningful. A test without assertions always passes — it cannot produce the red X needed in Step 1.
|
||||
|
||||
**Bad**:
|
||||
|
||||
```typescript
|
||||
it('processes the download', () => {
|
||||
processDownload('/models/checkpoints', 'model.safetensors')
|
||||
// No expect()!
|
||||
})
|
||||
```
|
||||
|
||||
**Good**:
|
||||
|
||||
```typescript
|
||||
it('processes the download to correct path', () => {
|
||||
const result = processDownload('/models/checkpoints', 'model.safetensors')
|
||||
expect(result.savePath).toBe('/models/checkpoints/model.safetensors')
|
||||
})
|
||||
```
|
||||
|
||||
## Over-Mocking
|
||||
|
||||
Mock only system boundaries (network, filesystem, Electron APIs). If you mock the module under test, you are testing your mocks — the test will not detect the real bug.
|
||||
|
||||
**Bad** — mocking everything:
|
||||
|
||||
```typescript
|
||||
vi.mock('./pathResolver')
|
||||
vi.mock('./validator')
|
||||
vi.mock('./downloader')
|
||||
|
||||
it('downloads model', () => {
|
||||
// This only tests that mocks were called, not that the bug exists
|
||||
})
|
||||
```
|
||||
|
||||
**Good** — mock only the boundary:
|
||||
|
||||
```typescript
|
||||
vi.mock('./electronAPI') // Boundary: Electron IPC
|
||||
|
||||
it('resolves absolute path correctly', () => {
|
||||
const result = resolveModelPath('/root/models', '/root/models/checkpoints')
|
||||
expect(result).toBe('/root/models/checkpoints')
|
||||
})
|
||||
```
|
||||
|
||||
See also: [Don't Mock What You Don't Own](https://hynek.me/articles/what-to-mock-in-5-mins/)
|
||||
|
||||
## Giant Tests
|
||||
|
||||
A test that covers the entire flow makes it hard to pinpoint which part catches the bug. Keep it focused — one concept per test.
|
||||
|
||||
**Bad**:
|
||||
|
||||
```typescript
|
||||
it('full model download flow', async () => {
|
||||
// 80 lines: load workflow, open dialog, select model,
|
||||
// click download, verify path, check progress, confirm completion
|
||||
})
|
||||
```
|
||||
|
||||
**Good**:
|
||||
|
||||
```typescript
|
||||
it('resolves absolute savePath without nesting under modelsDirectory', () => {
|
||||
const result = getLocalSavePath(
|
||||
'/models',
|
||||
'/models/checkpoints',
|
||||
'file.safetensors'
|
||||
)
|
||||
expect(result).toBe('/models/checkpoints/file.safetensors')
|
||||
})
|
||||
```
|
||||
|
||||
If the bug is in path resolution, test path resolution — not the entire download flow.
|
||||
|
||||
## Test Duplication
|
||||
|
||||
Duplicated test code hides what actually differs between cases. Use parameterized tests.
|
||||
|
||||
**Bad**:
|
||||
|
||||
```typescript
|
||||
it('resolves checkpoints path', () => {
|
||||
expect(resolve('/models', '/models/checkpoints', 'a.safetensors')).toBe(
|
||||
'/models/checkpoints/a.safetensors'
|
||||
)
|
||||
})
|
||||
it('resolves loras path', () => {
|
||||
expect(resolve('/models', '/models/loras', 'b.safetensors')).toBe(
|
||||
'/models/loras/b.safetensors'
|
||||
)
|
||||
})
|
||||
```
|
||||
|
||||
**Good**:
|
||||
|
||||
```typescript
|
||||
it.each([
|
||||
['/models/checkpoints', 'a.safetensors', '/models/checkpoints/a.safetensors'],
|
||||
['/models/loras', 'b.safetensors', '/models/loras/b.safetensors']
|
||||
])('resolves %s/%s to %s', (dir, file, expected) => {
|
||||
expect(resolve('/models', dir, file)).toBe(expected)
|
||||
})
|
||||
```
|
||||
|
||||
## Flaky Tests
|
||||
|
||||
A flaky test cannot prove anything — it may show red for reasons unrelated to the bug, or green despite the bug still existing.
|
||||
|
||||
**Common causes in this codebase:**
|
||||
|
||||
| Cause | Fix |
|
||||
| -------------------------------------- | --------------------------------------- |
|
||||
| Missing `nextFrame()` after canvas ops | Add `await comfyPage.nextFrame()` |
|
||||
| `waitForTimeout` instead of assertions | Use `toBeVisible()`, `toHaveText()` |
|
||||
| Shared state between tests | Isolate with `afterEach` / `beforeEach` |
|
||||
| Timing-dependent logic | Use `expect.poll()` or `toPass()` |
|
||||
|
||||
## Gaming the Red-Green Process
|
||||
|
||||
These are ways the red-green proof gets invalidated during Step 2 (the fix commit). The test from Step 1 is immutable — if any of these happen, restart from Step 1.
|
||||
|
||||
**Weakening the assertion to make it pass:**
|
||||
|
||||
```typescript
|
||||
// Step 1 (red) — strict assertion
|
||||
expect(result).toBe('/external/drive/models/checkpoints/file.safetensors')
|
||||
|
||||
// Step 2 (green) — weakened to pass without a real fix
|
||||
expect(result).toBeDefined() // This proves nothing
|
||||
```
|
||||
|
||||
**Updating snapshots to bless the bug:**
|
||||
|
||||
```bash
|
||||
# Instead of fixing the code, just updating the snapshot to match buggy output
|
||||
pnpm test:unit -- --update
|
||||
```
|
||||
|
||||
If a snapshot needs updating, the fix should change the code behavior, not the expected output.
|
||||
|
||||
**Adding mocks in Step 2 that hide the failure:**
|
||||
|
||||
```typescript
|
||||
// Step 2 adds a mock that didn't exist in Step 1
|
||||
vi.mock('./pathResolver', () => ({
|
||||
resolve: () => '/expected/path' // Hardcoded to pass
|
||||
}))
|
||||
```
|
||||
|
||||
Step 2 should only change source code — not test infrastructure.
|
||||
|
||||
## Testing the Happy Path Only
|
||||
|
||||
The red-green pattern specifically requires the test to exercise the **broken path**. If you only test the case that already works, the test will pass (green) on Step 1 — defeating the purpose.
|
||||
|
||||
**Bad** — testing the default case that works:
|
||||
|
||||
```typescript
|
||||
it('downloads to default models directory', () => {
|
||||
// This already works — it won't produce a red X
|
||||
const result = resolve('/models', 'checkpoints', 'file.safetensors')
|
||||
expect(result).toBe('/models/checkpoints/file.safetensors')
|
||||
})
|
||||
```
|
||||
|
||||
**Good** — testing the case that is actually broken:
|
||||
|
||||
```typescript
|
||||
it('downloads to external models directory configured via extra_model_paths', () => {
|
||||
// This is the broken case — absolute path from folder_paths API
|
||||
const result = resolve(
|
||||
'/models',
|
||||
'/external/drive/models/checkpoints',
|
||||
'file.safetensors'
|
||||
)
|
||||
expect(result).toBe('/external/drive/models/checkpoints/file.safetensors')
|
||||
})
|
||||
```
|
||||
2
.gitattributes
vendored
@@ -3,4 +3,6 @@
|
||||
|
||||
# Generated files
|
||||
packages/registry-types/src/comfyRegistryTypes.ts linguist-generated=true
|
||||
packages/ingest-types/src/types.gen.ts linguist-generated=true
|
||||
packages/ingest-types/src/zod.gen.ts linguist-generated=true
|
||||
src/workbench/extensions/manager/types/generatedManagerTypes.ts linguist-generated=true
|
||||
|
||||
48
.github/workflows/ci-perf-report.yaml
vendored
@@ -10,7 +10,7 @@ on:
|
||||
|
||||
concurrency:
|
||||
group: perf-${{ github.ref }}
|
||||
cancel-in-progress: true
|
||||
cancel-in-progress: false
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
@@ -26,12 +26,15 @@ jobs:
|
||||
username: ${{ github.actor }}
|
||||
password: ${{ secrets.GITHUB_TOKEN }}
|
||||
permissions:
|
||||
contents: read
|
||||
contents: write
|
||||
packages: read
|
||||
actions: read
|
||||
|
||||
steps:
|
||||
- name: Checkout repository
|
||||
uses: actions/checkout@v6
|
||||
with:
|
||||
persist-credentials: false
|
||||
|
||||
- name: Setup frontend
|
||||
uses: ./.github/actions/setup-frontend
|
||||
@@ -68,3 +71,44 @@ jobs:
|
||||
with:
|
||||
name: perf-meta
|
||||
path: temp/perf-meta/
|
||||
|
||||
- name: Save perf baseline to perf-data branch
|
||||
if: github.event_name == 'push' && github.ref == 'refs/heads/main' && steps.perf.outcome == 'success'
|
||||
continue-on-error: true
|
||||
run: |
|
||||
git config user.name "github-actions[bot]"
|
||||
git config user.email "github-actions[bot]@users.noreply.github.com"
|
||||
git config url."https://x-access-token:${GH_TOKEN}@github.com/".insteadOf "https://github.com/"
|
||||
|
||||
cp test-results/perf-metrics.json /tmp/perf-metrics.json
|
||||
|
||||
git fetch origin perf-data || {
|
||||
echo "Creating perf-data branch"
|
||||
git checkout --orphan perf-data
|
||||
git rm -rf . 2>/dev/null || true
|
||||
echo "# Performance Baselines" > README.md
|
||||
mkdir -p baselines
|
||||
git add README.md baselines
|
||||
git commit -m "Initialize perf-data branch"
|
||||
git push origin perf-data
|
||||
git fetch origin perf-data
|
||||
}
|
||||
|
||||
git worktree add /tmp/perf-data origin/perf-data
|
||||
|
||||
TIMESTAMP=$(date -u +%Y%m%dT%H%M%SZ)
|
||||
SHA=$(echo "${{ github.sha }}" | cut -c1-8)
|
||||
mkdir -p /tmp/perf-data/baselines
|
||||
cp /tmp/perf-metrics.json "/tmp/perf-data/baselines/perf-${TIMESTAMP}-${SHA}.json"
|
||||
|
||||
# Keep only last 20 baselines
|
||||
cd /tmp/perf-data
|
||||
ls -t baselines/perf-*.json 2>/dev/null | tail -n +21 | xargs -r rm
|
||||
|
||||
git -C /tmp/perf-data add baselines/
|
||||
git -C /tmp/perf-data commit -m "perf: add baseline for ${SHA}" || echo "No changes to commit"
|
||||
git -C /tmp/perf-data push origin HEAD:perf-data
|
||||
|
||||
git worktree remove /tmp/perf-data --force 2>/dev/null || true
|
||||
env:
|
||||
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
|
||||
46
.github/workflows/pr-perf-report.yaml
vendored
@@ -10,6 +10,7 @@ permissions:
|
||||
contents: read
|
||||
pull-requests: write
|
||||
issues: write
|
||||
actions: read
|
||||
|
||||
jobs:
|
||||
comment:
|
||||
@@ -73,7 +74,28 @@ jobs:
|
||||
core.setOutput('number', String(pr.number));
|
||||
core.setOutput('base', trustedBase);
|
||||
|
||||
- name: Check if results are still current
|
||||
id: sha-check
|
||||
uses: actions/github-script@v8
|
||||
with:
|
||||
script: |
|
||||
const prNumber = Number('${{ steps.pr-meta.outputs.number }}');
|
||||
const { data: pr } = await github.rest.pulls.get({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
pull_number: prNumber,
|
||||
});
|
||||
const runSha = context.payload.workflow_run.head_sha;
|
||||
const currentSha = pr.head.sha;
|
||||
if (runSha !== currentSha) {
|
||||
core.info(`Skipping stale report: run SHA ${runSha} != current PR SHA ${currentSha}`);
|
||||
core.setOutput('stale', 'true');
|
||||
} else {
|
||||
core.setOutput('stale', 'false');
|
||||
}
|
||||
|
||||
- name: Download PR perf metrics
|
||||
if: steps.sha-check.outputs.stale != 'true'
|
||||
uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12
|
||||
with:
|
||||
name: perf-metrics
|
||||
@@ -81,6 +103,7 @@ jobs:
|
||||
path: test-results/
|
||||
|
||||
- name: Download baseline perf metrics
|
||||
if: steps.sha-check.outputs.stale != 'true'
|
||||
uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12
|
||||
with:
|
||||
branch: ${{ steps.pr-meta.outputs.base }}
|
||||
@@ -90,10 +113,33 @@ jobs:
|
||||
path: temp/perf-baseline/
|
||||
if_no_artifact_found: warn
|
||||
|
||||
- name: Load historical baselines from perf-data branch
|
||||
if: steps.sha-check.outputs.stale != 'true'
|
||||
continue-on-error: true
|
||||
run: |
|
||||
mkdir -p temp/perf-history
|
||||
|
||||
git fetch origin perf-data 2>/dev/null || {
|
||||
echo "perf-data branch not found, skipping historical data"
|
||||
exit 0
|
||||
}
|
||||
|
||||
INDEX=0
|
||||
for file in $(git ls-tree --name-only origin/perf-data baselines/ 2>/dev/null | sort -r | head -5); do
|
||||
DIR="temp/perf-history/$INDEX"
|
||||
mkdir -p "$DIR"
|
||||
git show "origin/perf-data:${file}" > "$DIR/perf-metrics.json" 2>/dev/null || true
|
||||
INDEX=$((INDEX + 1))
|
||||
done
|
||||
|
||||
echo "Loaded $INDEX historical baselines"
|
||||
|
||||
- name: Generate perf report
|
||||
if: steps.sha-check.outputs.stale != 'true'
|
||||
run: npx --yes tsx scripts/perf-report.ts > perf-report.md
|
||||
|
||||
- name: Post PR comment
|
||||
if: steps.sha-check.outputs.stale != 'true'
|
||||
uses: ./.github/actions/post-pr-report-comment
|
||||
with:
|
||||
pr-number: ${{ steps.pr-meta.outputs.number }}
|
||||
|
||||
4876
browser_tests/assets/large-graph-workflow.json
Normal file
@@ -8,7 +8,7 @@
|
||||
"id": 11,
|
||||
"type": "422723e8-4bf6-438c-823f-881ca81acead",
|
||||
"pos": [791.59912109375, 386.13336181640625],
|
||||
"size": [140, 26],
|
||||
"size": [400, 200],
|
||||
"flags": {},
|
||||
"order": 0,
|
||||
"mode": 0,
|
||||
|
||||
@@ -431,9 +431,9 @@ export const comfyPageFixture = base.extend<{
|
||||
// Disable toast warning about version compatibility, as they may or
|
||||
// may not appear - depending on upstream ComfyUI dependencies
|
||||
'Comfy.VersionCompatibility.DisableWarnings': true,
|
||||
// Browser tests should opt into missing-model warnings explicitly so
|
||||
// workflows do not render differently based on models present on disk.
|
||||
'Comfy.Workflow.ShowMissingModelsWarning': false
|
||||
// Disable errors tab to prevent missing model detection from
|
||||
// rendering error indicators on nodes during unrelated tests.
|
||||
'Comfy.RightSidePanel.ShowErrorsTab': false
|
||||
})
|
||||
} catch (e) {
|
||||
console.error(e)
|
||||
|
||||
@@ -8,6 +8,10 @@ interface PerfSnapshot {
|
||||
TaskDuration: number
|
||||
JSHeapUsedSize: number
|
||||
Timestamp: number
|
||||
Nodes: number
|
||||
JSHeapTotalSize: number
|
||||
ScriptDuration: number
|
||||
JSEventListeners: number
|
||||
}
|
||||
|
||||
export interface PerfMeasurement {
|
||||
@@ -19,6 +23,12 @@ export interface PerfMeasurement {
|
||||
layoutDurationMs: number
|
||||
taskDurationMs: number
|
||||
heapDeltaBytes: number
|
||||
domNodes: number
|
||||
jsHeapTotalBytes: number
|
||||
scriptDurationMs: number
|
||||
eventListeners: number
|
||||
totalBlockingTimeMs: number
|
||||
frameDurationMs: number
|
||||
}
|
||||
|
||||
export class PerformanceHelper {
|
||||
@@ -59,16 +69,100 @@ export class PerformanceHelper {
|
||||
LayoutDuration: get('LayoutDuration'),
|
||||
TaskDuration: get('TaskDuration'),
|
||||
JSHeapUsedSize: get('JSHeapUsedSize'),
|
||||
Timestamp: get('Timestamp')
|
||||
Timestamp: get('Timestamp'),
|
||||
Nodes: get('Nodes'),
|
||||
JSHeapTotalSize: get('JSHeapTotalSize'),
|
||||
ScriptDuration: get('ScriptDuration'),
|
||||
JSEventListeners: get('JSEventListeners')
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Collect longtask entries from PerformanceObserver and compute TBT.
|
||||
* TBT = sum of (duration - 50ms) for every task longer than 50ms.
|
||||
*/
|
||||
private async collectTBT(): Promise<number> {
|
||||
return this.page.evaluate(() => {
|
||||
const state = (window as unknown as Record<string, unknown>)
|
||||
.__perfLongtaskState as
|
||||
| { observer: PerformanceObserver; tbtMs: number }
|
||||
| undefined
|
||||
if (!state) return 0
|
||||
|
||||
// Flush any queued-but-undelivered entries into our accumulator
|
||||
for (const entry of state.observer.takeRecords()) {
|
||||
if (entry.duration > 50) state.tbtMs += entry.duration - 50
|
||||
}
|
||||
const result = state.tbtMs
|
||||
state.tbtMs = 0
|
||||
return result
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Measure average frame duration via rAF timing over a sample window.
|
||||
* Returns average ms per frame (lower = better, 16.67 = 60fps).
|
||||
*/
|
||||
private async measureFrameDuration(sampleFrames = 10): Promise<number> {
|
||||
return this.page.evaluate((frames) => {
|
||||
return new Promise<number>((resolve) => {
|
||||
const timeout = setTimeout(() => resolve(0), 5000)
|
||||
const timestamps: number[] = []
|
||||
let count = 0
|
||||
function tick(ts: number) {
|
||||
timestamps.push(ts)
|
||||
count++
|
||||
if (count <= frames) {
|
||||
requestAnimationFrame(tick)
|
||||
} else {
|
||||
clearTimeout(timeout)
|
||||
if (timestamps.length < 2) {
|
||||
resolve(0)
|
||||
return
|
||||
}
|
||||
const total = timestamps[timestamps.length - 1] - timestamps[0]
|
||||
resolve(total / (timestamps.length - 1))
|
||||
}
|
||||
}
|
||||
requestAnimationFrame(tick)
|
||||
})
|
||||
}, sampleFrames)
|
||||
}
|
||||
|
||||
async startMeasuring(): Promise<void> {
|
||||
if (this.snapshot) {
|
||||
throw new Error(
|
||||
'Measurement already in progress — call stopMeasuring() first'
|
||||
)
|
||||
}
|
||||
// Install longtask observer if not already present, then reset the
|
||||
// accumulator so old longtasks don't bleed into the new measurement window.
|
||||
await this.page.evaluate(() => {
|
||||
const win = window as unknown as Record<string, unknown>
|
||||
if (!win.__perfLongtaskState) {
|
||||
const state: { observer: PerformanceObserver; tbtMs: number } = {
|
||||
observer: new PerformanceObserver((list) => {
|
||||
const self = (window as unknown as Record<string, unknown>)
|
||||
.__perfLongtaskState as {
|
||||
observer: PerformanceObserver
|
||||
tbtMs: number
|
||||
}
|
||||
for (const entry of list.getEntries()) {
|
||||
if (entry.duration > 50) self.tbtMs += entry.duration - 50
|
||||
}
|
||||
}),
|
||||
tbtMs: 0
|
||||
}
|
||||
state.observer.observe({ type: 'longtask', buffered: true })
|
||||
win.__perfLongtaskState = state
|
||||
}
|
||||
const state = win.__perfLongtaskState as {
|
||||
observer: PerformanceObserver
|
||||
tbtMs: number
|
||||
}
|
||||
state.tbtMs = 0
|
||||
state.observer.takeRecords()
|
||||
})
|
||||
this.snapshot = await this.getSnapshot()
|
||||
}
|
||||
|
||||
@@ -82,6 +176,11 @@ export class PerformanceHelper {
|
||||
return after[key] - before[key]
|
||||
}
|
||||
|
||||
const [totalBlockingTimeMs, frameDurationMs] = await Promise.all([
|
||||
this.collectTBT(),
|
||||
this.measureFrameDuration()
|
||||
])
|
||||
|
||||
return {
|
||||
name,
|
||||
durationMs: delta('Timestamp') * 1000,
|
||||
@@ -90,7 +189,13 @@ export class PerformanceHelper {
|
||||
layouts: delta('LayoutCount'),
|
||||
layoutDurationMs: delta('LayoutDuration') * 1000,
|
||||
taskDurationMs: delta('TaskDuration') * 1000,
|
||||
heapDeltaBytes: delta('JSHeapUsedSize')
|
||||
heapDeltaBytes: delta('JSHeapUsedSize'),
|
||||
domNodes: delta('Nodes'),
|
||||
jsHeapTotalBytes: delta('JSHeapTotalSize'),
|
||||
scriptDurationMs: delta('ScriptDuration') * 1000,
|
||||
eventListeners: delta('JSEventListeners'),
|
||||
totalBlockingTimeMs,
|
||||
frameDurationMs
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
Before Width: | Height: | Size: 138 KiB After Width: | Height: | Size: 130 KiB |
|
Before Width: | Height: | Size: 132 KiB After Width: | Height: | Size: 126 KiB |
|
Before Width: | Height: | Size: 99 KiB After Width: | Height: | Size: 93 KiB |
|
Before Width: | Height: | Size: 133 KiB After Width: | Height: | Size: 126 KiB |
|
Before Width: | Height: | Size: 138 KiB After Width: | Height: | Size: 130 KiB |
|
Before Width: | Height: | Size: 158 KiB After Width: | Height: | Size: 154 KiB |
|
Before Width: | Height: | Size: 151 KiB After Width: | Height: | Size: 142 KiB |
|
Before Width: | Height: | Size: 150 KiB After Width: | Height: | Size: 141 KiB |
|
Before Width: | Height: | Size: 147 KiB After Width: | Height: | Size: 140 KiB |
|
Before Width: | Height: | Size: 133 KiB After Width: | Height: | Size: 126 KiB |
|
Before Width: | Height: | Size: 94 KiB After Width: | Height: | Size: 94 KiB |
|
Before Width: | Height: | Size: 115 KiB After Width: | Height: | Size: 107 KiB |
|
Before Width: | Height: | Size: 97 KiB After Width: | Height: | Size: 91 KiB |
|
Before Width: | Height: | Size: 104 KiB After Width: | Height: | Size: 97 KiB |
|
Before Width: | Height: | Size: 97 KiB After Width: | Height: | Size: 91 KiB |
|
Before Width: | Height: | Size: 100 KiB After Width: | Height: | Size: 100 KiB |
@@ -1,4 +1,3 @@
|
||||
import type { Locator } from '@playwright/test'
|
||||
import { expect } from '@playwright/test'
|
||||
|
||||
import type { Keybinding } from '../../src/platform/keybindings/types'
|
||||
@@ -72,6 +71,10 @@ test('Does not report warning on undo/redo', async ({ comfyPage }) => {
|
||||
test.describe('Execution error', () => {
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Top')
|
||||
await comfyPage.settings.setSetting(
|
||||
'Comfy.RightSidePanel.ShowErrorsTab',
|
||||
true
|
||||
)
|
||||
await comfyPage.setup()
|
||||
})
|
||||
|
||||
@@ -88,117 +91,58 @@ test.describe('Execution error', () => {
|
||||
})
|
||||
})
|
||||
|
||||
test.describe('Missing models warning', () => {
|
||||
test('Should be disabled by default in browser tests', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
await comfyPage.workflow.loadWorkflow('missing/missing_models')
|
||||
|
||||
const dialogTitle = comfyPage.page.getByText(
|
||||
'This workflow is missing models'
|
||||
)
|
||||
await expect(dialogTitle).not.toBeVisible()
|
||||
})
|
||||
|
||||
test.describe('Missing models in Error Tab', () => {
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Top')
|
||||
await comfyPage.settings.setSetting(
|
||||
'Comfy.Workflow.ShowMissingModelsWarning',
|
||||
'Comfy.RightSidePanel.ShowErrorsTab',
|
||||
true
|
||||
)
|
||||
await comfyPage.page.evaluate((url: string) => {
|
||||
return fetch(`${url}/api/devtools/cleanup_fake_model`)
|
||||
const cleanupOk = await comfyPage.page.evaluate(async (url: string) => {
|
||||
const response = await fetch(`${url}/api/devtools/cleanup_fake_model`)
|
||||
return response.ok
|
||||
}, comfyPage.url)
|
||||
expect(cleanupOk).toBeTruthy()
|
||||
})
|
||||
|
||||
test('Should display a warning when missing models are found', async ({
|
||||
test('Should show error overlay with missing models when workflow has missing models', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
await comfyPage.workflow.loadWorkflow('missing/missing_models')
|
||||
|
||||
const dialogTitle = comfyPage.page.getByText(
|
||||
'This workflow is missing models'
|
||||
)
|
||||
await expect(dialogTitle).toBeVisible()
|
||||
const errorOverlay = comfyPage.page.locator('[data-testid="error-overlay"]')
|
||||
await expect(errorOverlay).toBeVisible()
|
||||
|
||||
const downloadAllButton = comfyPage.page.getByText('Download all')
|
||||
await expect(downloadAllButton).toBeVisible()
|
||||
const missingModelsTitle = comfyPage.page.getByText(/Missing Models/)
|
||||
await expect(missingModelsTitle).toBeVisible()
|
||||
})
|
||||
|
||||
test('Should display a warning when missing models are found in node properties', async ({
|
||||
test('Should show missing models from node properties', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
// Load workflow that has a node with models metadata at the node level
|
||||
await comfyPage.workflow.loadWorkflow(
|
||||
'missing/missing_models_from_node_properties'
|
||||
)
|
||||
|
||||
const dialogTitle = comfyPage.page.getByText(
|
||||
'This workflow is missing models'
|
||||
)
|
||||
await expect(dialogTitle).toBeVisible()
|
||||
const errorOverlay = comfyPage.page.locator('[data-testid="error-overlay"]')
|
||||
await expect(errorOverlay).toBeVisible()
|
||||
|
||||
const downloadAllButton = comfyPage.page.getByText('Download all')
|
||||
await expect(downloadAllButton).toBeVisible()
|
||||
const missingModelsTitle = comfyPage.page.getByText(/Missing Models/)
|
||||
await expect(missingModelsTitle).toBeVisible()
|
||||
})
|
||||
|
||||
test('Should not display a warning when no missing models are found', async ({
|
||||
test('Should not show missing models when widget values have changed', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
const modelFoldersRes = {
|
||||
status: 200,
|
||||
body: JSON.stringify([
|
||||
{
|
||||
name: 'text_encoders',
|
||||
folders: ['ComfyUI/models/text_encoders']
|
||||
}
|
||||
])
|
||||
}
|
||||
await comfyPage.page.route(
|
||||
'**/api/experiment/models',
|
||||
(route) => route.fulfill(modelFoldersRes),
|
||||
{ times: 1 }
|
||||
)
|
||||
|
||||
// Reload page to trigger indexing of model folders
|
||||
await comfyPage.setup()
|
||||
|
||||
const clipModelsRes = {
|
||||
status: 200,
|
||||
body: JSON.stringify([
|
||||
{
|
||||
name: 'fake_model.safetensors',
|
||||
pathIndex: 0
|
||||
}
|
||||
])
|
||||
}
|
||||
await comfyPage.page.route(
|
||||
'**/api/experiment/models/text_encoders',
|
||||
(route) => route.fulfill(clipModelsRes),
|
||||
{ times: 1 }
|
||||
)
|
||||
|
||||
await comfyPage.workflow.loadWorkflow('missing/missing_models')
|
||||
|
||||
const dialogTitle = comfyPage.page.getByText(
|
||||
'This workflow is missing models'
|
||||
)
|
||||
await expect(dialogTitle).not.toBeVisible()
|
||||
})
|
||||
|
||||
test('Should not display warning when model metadata exists but widget values have changed', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
// This tests the scenario where outdated model metadata exists in the workflow
|
||||
// but the actual selected models (widget values) have changed
|
||||
await comfyPage.workflow.loadWorkflow(
|
||||
'missing/model_metadata_widget_mismatch'
|
||||
)
|
||||
|
||||
// The missing models warning should NOT appear
|
||||
const dialogTitle = comfyPage.page.getByText(
|
||||
'This workflow is missing models'
|
||||
)
|
||||
await expect(dialogTitle).not.toBeVisible()
|
||||
const missingModelsTitle = comfyPage.page.getByText(/Missing Models/)
|
||||
await expect(missingModelsTitle).not.toBeVisible()
|
||||
|
||||
const errorOverlay = comfyPage.page.locator('[data-testid="error-overlay"]')
|
||||
await expect(errorOverlay).not.toBeVisible()
|
||||
})
|
||||
|
||||
// Flaky test after parallelization
|
||||
@@ -206,14 +150,10 @@ test.describe('Missing models warning', () => {
|
||||
test.skip('Should download missing model when clicking download button', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
// The fake_model.safetensors is served by
|
||||
// https://github.com/Comfy-Org/ComfyUI_devtools/blob/main/__init__.py
|
||||
await comfyPage.workflow.loadWorkflow('missing/missing_models')
|
||||
|
||||
const dialogTitle = comfyPage.page.getByText(
|
||||
'This workflow is missing models'
|
||||
)
|
||||
await expect(dialogTitle).toBeVisible()
|
||||
const errorOverlay = comfyPage.page.locator('[data-testid="error-overlay"]')
|
||||
await expect(errorOverlay).toBeVisible()
|
||||
|
||||
const downloadAllButton = comfyPage.page.getByText('Download all')
|
||||
await expect(downloadAllButton).toBeVisible()
|
||||
@@ -223,50 +163,6 @@ test.describe('Missing models warning', () => {
|
||||
const download = await downloadPromise
|
||||
expect(download.suggestedFilename()).toBe('fake_model.safetensors')
|
||||
})
|
||||
|
||||
test.describe('Do not show again checkbox', () => {
|
||||
let checkbox: Locator
|
||||
let closeButton: Locator
|
||||
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.settings.setSetting(
|
||||
'Comfy.Workflow.ShowMissingModelsWarning',
|
||||
true
|
||||
)
|
||||
await comfyPage.workflow.loadWorkflow('missing/missing_models')
|
||||
|
||||
checkbox = comfyPage.page.getByLabel("Don't show this again")
|
||||
closeButton = comfyPage.page.getByLabel('Close')
|
||||
})
|
||||
|
||||
test('Should disable warning dialog when checkbox is checked', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
const changeSettingPromise = comfyPage.page.waitForRequest(
|
||||
'**/api/settings/Comfy.Workflow.ShowMissingModelsWarning'
|
||||
)
|
||||
await checkbox.click()
|
||||
await changeSettingPromise
|
||||
|
||||
await closeButton.click()
|
||||
|
||||
const settingValue = await comfyPage.settings.getSetting(
|
||||
'Comfy.Workflow.ShowMissingModelsWarning'
|
||||
)
|
||||
expect(settingValue).toBe(false)
|
||||
})
|
||||
|
||||
test('Should keep warning dialog enabled when checkbox is unchecked', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
await closeButton.click()
|
||||
|
||||
const settingValue = await comfyPage.settings.getSetting(
|
||||
'Comfy.Workflow.ShowMissingModelsWarning'
|
||||
)
|
||||
expect(settingValue).toBe(true)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
test.describe('Settings', () => {
|
||||
|
||||
@@ -9,6 +9,10 @@ test.beforeEach(async ({ comfyPage }) => {
|
||||
test.describe('Execution', { tag: ['@smoke', '@workflow'] }, () => {
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Top')
|
||||
await comfyPage.settings.setSetting(
|
||||
'Comfy.RightSidePanel.ShowErrorsTab',
|
||||
true
|
||||
)
|
||||
await comfyPage.setup()
|
||||
})
|
||||
|
||||
|
||||
|
Before Width: | Height: | Size: 80 KiB After Width: | Height: | Size: 80 KiB |
|
Before Width: | Height: | Size: 95 KiB After Width: | Height: | Size: 95 KiB |
44
browser_tests/tests/groupCopyPaste.spec.ts
Normal file
@@ -0,0 +1,44 @@
|
||||
import { expect } from '@playwright/test'
|
||||
|
||||
import { comfyPageFixture as test } from '../fixtures/ComfyPage'
|
||||
|
||||
test.describe('Group Copy Paste', { tag: ['@canvas'] }, () => {
|
||||
test.afterEach(async ({ comfyPage }) => {
|
||||
await comfyPage.canvasOps.resetView()
|
||||
})
|
||||
|
||||
test('Pasted group is offset from original position', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
await comfyPage.workflow.loadWorkflow('groups/single_group_only')
|
||||
|
||||
const titlePos = await comfyPage.page.evaluate(() => {
|
||||
const app = window.app!
|
||||
const group = app.graph.groups[0]
|
||||
const clientPos = app.canvasPosToClientPos([
|
||||
group.pos[0] + 50,
|
||||
group.pos[1] + 15
|
||||
])
|
||||
return { x: clientPos[0], y: clientPos[1] }
|
||||
})
|
||||
await comfyPage.canvas.click({ position: titlePos })
|
||||
await comfyPage.nextFrame()
|
||||
|
||||
await comfyPage.clipboard.copy()
|
||||
await comfyPage.clipboard.paste()
|
||||
await comfyPage.nextFrame()
|
||||
|
||||
const positions = await comfyPage.page.evaluate(() =>
|
||||
window.app!.graph.groups.map((g: { pos: number[] }) => ({
|
||||
x: g.pos[0],
|
||||
y: g.pos[1]
|
||||
}))
|
||||
)
|
||||
|
||||
expect(positions).toHaveLength(2)
|
||||
const dx = Math.abs(positions[0].x - positions[1].x)
|
||||
const dy = Math.abs(positions[0].y - positions[1].y)
|
||||
expect(dx).toBeCloseTo(50, 0)
|
||||
expect(dy).toBeCloseTo(15, 0)
|
||||
})
|
||||
})
|
||||
|
Before Width: | Height: | Size: 97 KiB After Width: | Height: | Size: 91 KiB |
|
Before Width: | Height: | Size: 91 KiB After Width: | Height: | Size: 85 KiB |
|
Before Width: | Height: | Size: 75 KiB After Width: | Height: | Size: 70 KiB |
|
Before Width: | Height: | Size: 75 KiB After Width: | Height: | Size: 70 KiB |
|
Before Width: | Height: | Size: 98 KiB After Width: | Height: | Size: 93 KiB |
|
Before Width: | Height: | Size: 97 KiB After Width: | Height: | Size: 91 KiB |
|
Before Width: | Height: | Size: 93 KiB After Width: | Height: | Size: 87 KiB |
|
Before Width: | Height: | Size: 96 KiB After Width: | Height: | Size: 91 KiB |
|
Before Width: | Height: | Size: 92 KiB After Width: | Height: | Size: 87 KiB |
|
Before Width: | Height: | Size: 107 KiB After Width: | Height: | Size: 101 KiB |
|
Before Width: | Height: | Size: 89 KiB After Width: | Height: | Size: 84 KiB |
|
Before Width: | Height: | Size: 98 KiB After Width: | Height: | Size: 92 KiB |
|
Before Width: | Height: | Size: 89 KiB After Width: | Height: | Size: 84 KiB |
|
Before Width: | Height: | Size: 89 KiB After Width: | Height: | Size: 84 KiB |
|
Before Width: | Height: | Size: 101 KiB After Width: | Height: | Size: 95 KiB |
|
Before Width: | Height: | Size: 97 KiB After Width: | Height: | Size: 91 KiB |
|
Before Width: | Height: | Size: 95 KiB After Width: | Height: | Size: 90 KiB |
|
Before Width: | Height: | Size: 113 KiB After Width: | Height: | Size: 106 KiB |
|
Before Width: | Height: | Size: 103 KiB After Width: | Height: | Size: 96 KiB |
|
Before Width: | Height: | Size: 102 KiB After Width: | Height: | Size: 95 KiB |
|
Before Width: | Height: | Size: 102 KiB After Width: | Height: | Size: 95 KiB |
|
Before Width: | Height: | Size: 97 KiB After Width: | Height: | Size: 91 KiB |
|
Before Width: | Height: | Size: 72 KiB After Width: | Height: | Size: 69 KiB |
|
Before Width: | Height: | Size: 89 KiB After Width: | Height: | Size: 84 KiB |
|
Before Width: | Height: | Size: 100 KiB After Width: | Height: | Size: 94 KiB |
|
Before Width: | Height: | Size: 98 KiB After Width: | Height: | Size: 92 KiB |
|
Before Width: | Height: | Size: 101 KiB After Width: | Height: | Size: 94 KiB |
|
Before Width: | Height: | Size: 98 KiB After Width: | Height: | Size: 92 KiB |
|
Before Width: | Height: | Size: 96 KiB After Width: | Height: | Size: 90 KiB |
|
Before Width: | Height: | Size: 98 KiB After Width: | Height: | Size: 92 KiB |
|
Before Width: | Height: | Size: 101 KiB After Width: | Height: | Size: 95 KiB |
|
Before Width: | Height: | Size: 97 KiB After Width: | Height: | Size: 91 KiB |
|
Before Width: | Height: | Size: 98 KiB After Width: | Height: | Size: 92 KiB |
|
Before Width: | Height: | Size: 102 KiB After Width: | Height: | Size: 95 KiB |
|
Before Width: | Height: | Size: 89 KiB After Width: | Height: | Size: 84 KiB |
|
Before Width: | Height: | Size: 98 KiB After Width: | Height: | Size: 92 KiB |
|
Before Width: | Height: | Size: 97 KiB After Width: | Height: | Size: 91 KiB |
|
Before Width: | Height: | Size: 102 KiB After Width: | Height: | Size: 96 KiB |
|
Before Width: | Height: | Size: 90 KiB After Width: | Height: | Size: 85 KiB |
|
Before Width: | Height: | Size: 96 KiB After Width: | Height: | Size: 90 KiB |
|
Before Width: | Height: | Size: 97 KiB After Width: | Height: | Size: 91 KiB |
|
Before Width: | Height: | Size: 100 KiB After Width: | Height: | Size: 94 KiB |
|
Before Width: | Height: | Size: 97 KiB After Width: | Height: | Size: 91 KiB |
|
Before Width: | Height: | Size: 66 KiB After Width: | Height: | Size: 62 KiB |
|
Before Width: | Height: | Size: 99 KiB After Width: | Height: | Size: 93 KiB |
|
Before Width: | Height: | Size: 68 KiB After Width: | Height: | Size: 65 KiB |
|
Before Width: | Height: | Size: 95 KiB After Width: | Height: | Size: 90 KiB |
|
Before Width: | Height: | Size: 97 KiB After Width: | Height: | Size: 91 KiB |
|
Before Width: | Height: | Size: 36 KiB After Width: | Height: | Size: 34 KiB |
|
Before Width: | Height: | Size: 34 KiB After Width: | Height: | Size: 32 KiB |
|
Before Width: | Height: | Size: 97 KiB After Width: | Height: | Size: 91 KiB |
|
Before Width: | Height: | Size: 35 KiB After Width: | Height: | Size: 33 KiB |
|
Before Width: | Height: | Size: 99 KiB After Width: | Height: | Size: 93 KiB |
|
Before Width: | Height: | Size: 104 KiB After Width: | Height: | Size: 99 KiB |
|
Before Width: | Height: | Size: 99 KiB After Width: | Height: | Size: 93 KiB |
|
Before Width: | Height: | Size: 99 KiB After Width: | Height: | Size: 93 KiB |
|
Before Width: | Height: | Size: 53 KiB After Width: | Height: | Size: 49 KiB |
|
Before Width: | Height: | Size: 36 KiB After Width: | Height: | Size: 34 KiB |
|
Before Width: | Height: | Size: 30 KiB After Width: | Height: | Size: 30 KiB |
|
Before Width: | Height: | Size: 18 KiB After Width: | Height: | Size: 18 KiB |
|
Before Width: | Height: | Size: 101 KiB After Width: | Height: | Size: 95 KiB |
|
Before Width: | Height: | Size: 101 KiB After Width: | Height: | Size: 95 KiB |
|
Before Width: | Height: | Size: 103 KiB After Width: | Height: | Size: 96 KiB |
|
Before Width: | Height: | Size: 104 KiB After Width: | Height: | Size: 98 KiB |
|
Before Width: | Height: | Size: 109 KiB After Width: | Height: | Size: 100 KiB |
|
Before Width: | Height: | Size: 82 KiB After Width: | Height: | Size: 77 KiB |
|
Before Width: | Height: | Size: 104 KiB After Width: | Height: | Size: 98 KiB |
@@ -1,3 +1,5 @@
|
||||
import { expect } from '@playwright/test'
|
||||
|
||||
import { comfyPageFixture as test } from '../fixtures/ComfyPage'
|
||||
import { recordMeasurement } from '../helpers/perfReporter'
|
||||
|
||||
@@ -107,6 +109,51 @@ test.describe('Performance', { tag: ['@perf'] }, () => {
|
||||
)
|
||||
})
|
||||
|
||||
test('large graph idle rendering', async ({ comfyPage }) => {
|
||||
await comfyPage.workflow.loadWorkflow('large-graph-workflow')
|
||||
await comfyPage.perf.startMeasuring()
|
||||
|
||||
// Let the large graph idle for 2 seconds — measures compositor and
|
||||
// style recalculation cost at scale (245 nodes).
|
||||
for (let i = 0; i < 120; i++) {
|
||||
await comfyPage.nextFrame()
|
||||
}
|
||||
|
||||
const m = await comfyPage.perf.stopMeasuring('large-graph-idle')
|
||||
recordMeasurement(m)
|
||||
console.log(
|
||||
`Large graph idle: ${m.styleRecalcs} style recalcs, ${m.layouts} layouts`
|
||||
)
|
||||
})
|
||||
|
||||
test('large graph pan interaction', async ({ comfyPage }) => {
|
||||
await comfyPage.workflow.loadWorkflow('large-graph-workflow')
|
||||
|
||||
const canvas = comfyPage.canvas
|
||||
const box = await canvas.boundingBox()
|
||||
if (!box) throw new Error('Canvas bounding box not available')
|
||||
|
||||
await comfyPage.perf.startMeasuring()
|
||||
|
||||
// Simulate panning across a large graph — stresses compositor
|
||||
// layer management and transform recalculation.
|
||||
const centerX = box.x + box.width / 2
|
||||
const centerY = box.y + box.height / 2
|
||||
await comfyPage.page.mouse.move(centerX, centerY)
|
||||
await comfyPage.page.mouse.down({ button: 'middle' })
|
||||
for (let i = 0; i < 60; i++) {
|
||||
await comfyPage.page.mouse.move(centerX + i * 5, centerY + i * 2)
|
||||
await comfyPage.nextFrame()
|
||||
}
|
||||
await comfyPage.page.mouse.up({ button: 'middle' })
|
||||
|
||||
const m = await comfyPage.perf.stopMeasuring('large-graph-pan')
|
||||
recordMeasurement(m)
|
||||
console.log(
|
||||
`Large graph pan: ${m.styleRecalcs} style recalcs, ${m.layouts} layouts, ${m.taskDurationMs.toFixed(1)}ms task`
|
||||
)
|
||||
})
|
||||
|
||||
test('subgraph DOM widget clipping during node selection', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
@@ -129,4 +176,70 @@ test.describe('Performance', { tag: ['@perf'] }, () => {
|
||||
recordMeasurement(m)
|
||||
console.log(`Subgraph clipping: ${m.layouts} forced layouts`)
|
||||
})
|
||||
|
||||
test('canvas zoom sweep', async ({ comfyPage }) => {
|
||||
await comfyPage.workflow.loadWorkflow('default')
|
||||
await comfyPage.perf.startMeasuring()
|
||||
|
||||
// Zoom in 10 steps, then zoom out 10 steps
|
||||
for (let i = 0; i < 10; i++) {
|
||||
await comfyPage.canvasOps.zoom(-100)
|
||||
await comfyPage.nextFrame()
|
||||
}
|
||||
for (let i = 0; i < 10; i++) {
|
||||
await comfyPage.canvasOps.zoom(100)
|
||||
await comfyPage.nextFrame()
|
||||
}
|
||||
|
||||
const m = await comfyPage.perf.stopMeasuring('canvas-zoom-sweep')
|
||||
recordMeasurement(m)
|
||||
console.log(
|
||||
`Zoom sweep: ${m.layouts} layouts, ${m.frameDurationMs.toFixed(1)}ms/frame, TBT=${m.totalBlockingTimeMs.toFixed(0)}ms`
|
||||
)
|
||||
})
|
||||
|
||||
test('minimap idle', async ({ comfyPage }) => {
|
||||
// Enable minimap via setting, load workflow, then measure idle cost
|
||||
await comfyPage.settings.setSetting('Comfy.Minimap.Visible', true)
|
||||
await comfyPage.workflow.loadWorkflow('large-graph-workflow')
|
||||
|
||||
// Wait for minimap to render
|
||||
await comfyPage.page
|
||||
.locator('.litegraph-minimap')
|
||||
.waitFor({ state: 'visible', timeout: 5000 })
|
||||
|
||||
await comfyPage.perf.startMeasuring()
|
||||
|
||||
// Idle for 2 seconds with minimap open and 245 nodes
|
||||
for (let i = 0; i < 120; i++) {
|
||||
await comfyPage.nextFrame()
|
||||
}
|
||||
|
||||
const m = await comfyPage.perf.stopMeasuring('minimap-idle')
|
||||
recordMeasurement(m)
|
||||
console.log(
|
||||
`Minimap idle: ${m.styleRecalcs} style recalcs, ${m.layouts} layouts, TBT=${m.totalBlockingTimeMs.toFixed(0)}ms`
|
||||
)
|
||||
})
|
||||
|
||||
test('workflow execution', async ({ comfyPage }) => {
|
||||
// Uses lightweight PrimitiveString → PreviewAny workflow (no GPU needed)
|
||||
await comfyPage.workflow.loadWorkflow('execution/partial_execution')
|
||||
await comfyPage.perf.startMeasuring()
|
||||
|
||||
// Queue the prompt and wait for execution to complete
|
||||
await comfyPage.command.executeCommand('Comfy.QueuePrompt')
|
||||
|
||||
// Wait for the output widget to populate (execution_success)
|
||||
const outputNode = await comfyPage.nodeOps.getNodeRefById(1)
|
||||
await expect(async () => {
|
||||
expect(await (await outputNode.getWidget(0)).getValue()).toBe('foo')
|
||||
}).toPass({ timeout: 10000 })
|
||||
|
||||
const m = await comfyPage.perf.stopMeasuring('workflow-execution')
|
||||
recordMeasurement(m)
|
||||
console.log(
|
||||
`Workflow execution: ${m.durationMs.toFixed(0)}ms total, ${m.layouts} layouts, TBT=${m.totalBlockingTimeMs.toFixed(0)}ms`
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
|
Before Width: | Height: | Size: 102 KiB After Width: | Height: | Size: 95 KiB |
|
Before Width: | Height: | Size: 107 KiB After Width: | Height: | Size: 103 KiB |