From 5a5f060b029af98b9740758be87134b72b480dfd Mon Sep 17 00:00:00 2001 From: bymyself Date: Sat, 31 Jan 2026 15:35:19 -0800 Subject: [PATCH] refactor: deduplicate skill docs, add idiomatic retry patterns Amp-Thread-ID: https://ampcode.com/threads/T-019c165d-cebf-7393-847a-914e5858bd9a Co-authored-by: Amp --- .../skills/writing-playwright-tests/SKILL.md | 1 + .../writing-playwright-tests/core/canvas.md | 18 +------ .../writing-playwright-tests/core/setup.md | 32 +++--------- .../core/workflows.md | 13 +---- .../features/vue-nodes.md | 9 ++-- .../reference/cheatsheet.md | 13 +---- .../reference/debugging.md | 50 +++++++++++++++++-- .../testing/advanced.md | 15 +----- .../testing/file-upload.md | 16 +----- .../testing/widgets.md | 13 ++--- 10 files changed, 74 insertions(+), 106 deletions(-) diff --git a/.claude/skills/writing-playwright-tests/SKILL.md b/.claude/skills/writing-playwright-tests/SKILL.md index 6e1f1206d..a4a29601f 100644 --- a/.claude/skills/writing-playwright-tests/SKILL.md +++ b/.claude/skills/writing-playwright-tests/SKILL.md @@ -57,6 +57,7 @@ const slot = node.getOutputSlot('MODEL') | **API mocking** | [testing/mocking.md](testing/mocking.md) | | **Test assets** | [testing/assets.md](testing/assets.md) | | **Debug flaky tests** | [reference/debugging.md](reference/debugging.md) | +| **Async retry patterns** | [reference/debugging.md](reference/debugging.md#retry-patterns) | | **All fixture methods** | [reference/fixtures.md](reference/fixtures.md) | | **Quick cheatsheet** | [reference/cheatsheet.md](reference/cheatsheet.md) | diff --git a/.claude/skills/writing-playwright-tests/core/canvas.md b/.claude/skills/writing-playwright-tests/core/canvas.md index f1c00e5d3..178a68e59 100644 --- a/.claude/skills/writing-playwright-tests/core/canvas.md +++ b/.claude/skills/writing-playwright-tests/core/canvas.md @@ -82,23 +82,7 @@ await comfyPage.nextFrame() ## Connecting Nodes -```typescript -// Get output slot -const outputNode = comfyPage.getNodeRefByTitle('CLIP Loader') -const outputSlot = outputNode.getOutputSlot('CLIP') - -// Get input slot -const inputNode = comfyPage.getNodeRefByTitle('CLIP Text Encode') -const inputSlot = inputNode.getInputSlot('clip') - -// Connect via drag -await comfyMouse.dragFromTo( - await outputSlot.getPosition(), - await inputSlot.getPosition(), - { steps: 10 } -) -await comfyPage.nextFrame() -``` +See [nodes.md](nodes.md#connect-slots) for node connection patterns. ## Screenshot Testing diff --git a/.claude/skills/writing-playwright-tests/core/setup.md b/.claude/skills/writing-playwright-tests/core/setup.md index 9afe45f0b..a6e870204 100644 --- a/.claude/skills/writing-playwright-tests/core/setup.md +++ b/.claude/skills/writing-playwright-tests/core/setup.md @@ -85,31 +85,15 @@ await comfyPage.nextFrame() ## Common Gotchas -### 1. Missing `nextFrame()` +See [debugging.md](../reference/debugging.md) for detailed fixes. -Canvas changes don't render immediately: - -```typescript -await comfyPage.canvas.click(100, 200) -await comfyPage.nextFrame() // ← Required! -``` - -### 2. Double-Click Reliability - -```typescript -await element.dblclick({ delay: 5 }) -``` - -### 3. Screenshot Tests Are Linux-Only - -Don't commit local screenshots. Use `New Browser Test Expectations` PR label. - -### 4. Focus Before Keyboard - -```typescript -await comfyPage.canvas.focus() -await comfyPage.page.keyboard.press('Delete') -``` +| Issue | Solution | Details | +|-------|----------|---------| +| Canvas not updating | Add `nextFrame()` after canvas ops | [canvas.md](canvas.md#critical-always-use-nextframe) | +| Double-click unreliable | Use `{ delay: 5 }` option | [canvas.md](canvas.md#click-operations) | +| Screenshot mismatch | Linux-only, use PR label | [debugging.md](../reference/debugging.md#debugging-screenshots) | +| Keyboard not working | Focus canvas first | [canvas.md](canvas.md#focus-before-keyboard) | +| Flaky async assertions | Use `expect.poll()` or `toPass()` | [debugging.md](../reference/debugging.md#retry-patterns) | ## Fresh Page Setup diff --git a/.claude/skills/writing-playwright-tests/core/workflows.md b/.claude/skills/writing-playwright-tests/core/workflows.md index ea9cdb57b..36660b7e8 100644 --- a/.claude/skills/writing-playwright-tests/core/workflows.md +++ b/.claude/skills/writing-playwright-tests/core/workflows.md @@ -14,18 +14,9 @@ await comfyPage.loadWorkflow('nodes/reroute') await comfyPage.loadWorkflow('canvas/pan_zoom') ``` -### Asset Directory Structure +### Asset Organization -``` -browser_tests/assets/ -├── default.json # Basic starting workflow -├── canvas/ # Canvas state tests -├── groups/ # Group-related workflows -├── nodes/ # Node-specific workflows -├── widgets/ # Widget test workflows -├── workflows/ # Complex workflow scenarios -└── images/ # Image assets for drag-drop -``` +See [assets.md](../testing/assets.md) for full directory structure and best practices. ### Creating New Assets diff --git a/.claude/skills/writing-playwright-tests/features/vue-nodes.md b/.claude/skills/writing-playwright-tests/features/vue-nodes.md index 2a202ece3..69377e70d 100644 --- a/.claude/skills/writing-playwright-tests/features/vue-nodes.md +++ b/.claude/skills/writing-playwright-tests/features/vue-nodes.md @@ -99,15 +99,16 @@ const fixture = await comfyPage.vueNodes.getFixtureByTitle('Load Checkpoint') // Fixture maintains stable reference even if title changes ``` -## Common Vue Nodes Settings +## Vue Nodes Settings + +The essential setting for Vue Nodes: ```typescript await comfyPage.setSetting('Comfy.VueNodes.Enabled', true) -await comfyPage.setSetting('Comfy.UseNewMenu', 'Top') -await comfyPage.setSetting('Comfy.Minimap.Visible', false) -await comfyPage.setSetting('Comfy.Graph.CanvasMenu', true) ``` +See [setup.md](../core/setup.md#common-settings) for other common settings. + ## Example: Complete Vue Node Test ```typescript diff --git a/.claude/skills/writing-playwright-tests/reference/cheatsheet.md b/.claude/skills/writing-playwright-tests/reference/cheatsheet.md index 74d86347d..46f9ebb18 100644 --- a/.claude/skills/writing-playwright-tests/reference/cheatsheet.md +++ b/.claude/skills/writing-playwright-tests/reference/cheatsheet.md @@ -210,18 +210,9 @@ await comfyPage.page.waitForFunction( ) ``` -## Tags Quick Reference +## Tags -| Tag | Purpose | Run with | -| ------------- | -------------------- | ------------------------- | -| `@smoke` | Fast essential tests | `--grep @smoke` | -| `@slow` | Long-running tests | `--grep-invert @slow` | -| `@screenshot` | Visual regression | `--grep @screenshot` | -| `@mobile` | Mobile viewport | `--project=mobile-chrome` | -| `@2x` | HiDPI scale | `--project=chromium-2x` | -| `@canvas` | Canvas tests | `--grep @canvas` | -| `@node` | Node tests | `--grep @node` | -| `@widget` | Widget tests | `--grep @widget` | +See [setup.md](../core/setup.md#test-tags) for tag definitions. ## Run Commands diff --git a/.claude/skills/writing-playwright-tests/reference/debugging.md b/.claude/skills/writing-playwright-tests/reference/debugging.md index 2c8aeaa22..2ae5e8565 100644 --- a/.claude/skills/writing-playwright-tests/reference/debugging.md +++ b/.claude/skills/writing-playwright-tests/reference/debugging.md @@ -115,15 +115,55 @@ DEBUG=pw:api npx playwright test pnpm exec playwright test --ui ``` -## Retry Pattern +## Retry Patterns -For inherently async operations: +Playwright provides two idiomatic approaches for async assertions. + +### expect.poll() - Preferred for Single Values + +Use when polling a single value until it matches: + +```typescript +// Poll until node count matches +await expect + .poll(() => comfyPage.getGraphNodesCount(), { timeout: 3000 }) + .toBe(5) + +// Poll with custom intervals +await expect + .poll(() => widget.getValue(), { intervals: [100, 200, 500], timeout: 2000 }) + .toBe('expected') +``` + +### expect().toPass() - For Multiple Assertions + +Use when multiple conditions must be true together: ```typescript await expect(async () => { - const value = await widget.getValue() - expect(value).toBe(100) -}).toPass({ timeout: 2000 }) + expect(await input.getWidget(0).getValue()).toBe('foo') + expect(await output1.getWidget(0).getValue()).toBe('foo') + expect(await output2.getWidget(0).getValue()).toBe('') +}).toPass({ timeout: 2_000 }) +``` + +### When to Use Each + +| Pattern | Use Case | +|---------|----------| +| `expect.poll()` | Single value polling, cleaner syntax | +| `expect().toPass()` | Multiple assertions that must all pass | +| `locator.waitFor()` | Waiting for element state changes | +| Auto-retrying assertions | `toBeVisible()`, `toHaveText()`, etc. | + +### ❌ Never Use waitForTimeout + +```typescript +// ❌ Bad - arbitrary delay, flaky +await page.waitForTimeout(500) + +// ✅ Good - wait for actual condition +await expect.poll(() => getData()).toBe(expected) ``` ## Debugging Screenshots diff --git a/.claude/skills/writing-playwright-tests/testing/advanced.md b/.claude/skills/writing-playwright-tests/testing/advanced.md index 04822e3a5..7b6c6c788 100644 --- a/.claude/skills/writing-playwright-tests/testing/advanced.md +++ b/.claude/skills/writing-playwright-tests/testing/advanced.md @@ -118,20 +118,7 @@ const result = await comfyPage.page.evaluate(() => { ## WebSocket Mocking -Mock WebSocket status updates: - -```typescript -// Get WebSocket fixture -const { websocket } = await comfyPage.getWebSocket() - -// Send mock status -await websocket.send( - JSON.stringify({ - type: 'status', - data: { exec_info: { queue_remaining: 0 } } - }) -) -``` +See [mocking.md](mocking.md#websocket-mocking) for WebSocket patterns. ## Vue Node Testing diff --git a/.claude/skills/writing-playwright-tests/testing/file-upload.md b/.claude/skills/writing-playwright-tests/testing/file-upload.md index e84c1c06d..7f992ef6b 100644 --- a/.claude/skills/writing-playwright-tests/testing/file-upload.md +++ b/.claude/skills/writing-playwright-tests/testing/file-upload.md @@ -124,18 +124,6 @@ test('should upload image file', async ({ comfyPage }) => { }) ``` -## Organizing Test Assets +## Asset Organization -Assets should be organized by feature: - -``` -browser_tests/assets/ -├── widgets/ # Widget-specific workflows -│ ├── load_image_widget.json -│ └── boolean_widget.json -├── workflowInMedia/ # Files with embedded workflows -├── nodes/ # Node-specific workflows -└── image32x32.webp # Shared image assets -``` - -See [patterns/assets.md](assets.md) for full asset organization guide. +See [assets.md](assets.md) for directory structure and best practices. diff --git a/.claude/skills/writing-playwright-tests/testing/widgets.md b/.claude/skills/writing-playwright-tests/testing/widgets.md index ea4f0a2dd..6526ad00c 100644 --- a/.claude/skills/writing-playwright-tests/testing/widgets.md +++ b/.claude/skills/writing-playwright-tests/testing/widgets.md @@ -77,19 +77,20 @@ await expect(widget.locator).toBeVisible() ### 1. Wait for Value Change -Widget values may not update instantly: +Widget values may not update instantly. Use retry patterns: ```typescript await widget.setValue(100) await comfyPage.nextFrame() -// Retry assertion -await expect(async () => { - const value = await widget.getValue() - expect(value).toBe(100) -}).toPass({ timeout: 2000 }) +// Use poll for single value +await expect + .poll(() => widget.getValue(), { timeout: 2000 }) + .toBe(100) ``` +See [debugging.md](../reference/debugging.md#retry-patterns) for more retry patterns. + ### 2. Combo Widget Selection Click-based selection is more reliable than setValue for combos: