Enable shift-drop context menu test #7140
Conversation
dosubot
bot
added
the
size:M
📝 WalkthroughWalkthroughA previously skipped test in the Shift-drop interaction block is re-enabled with functional assertions verifying context menu visibility after drag-and-drop, including pin position state checks before and after the interaction. Changes
Possibly related PRs
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/03/2025, 08:47:42 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 12/03/2025, 08:59:23 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
|
the test failed because it dragged too far, which was canvas before, but after the QPO's button, it dragged onto the run button, which ate the action, and started to make it fail |
There was a problem hiding this comment.
Pull request overview
This PR re-enables a previously disabled Playwright test for the shift-drop context menu functionality in link interactions, with reduced drag distance to minimize test flakiness.
Key Changes:
- Removed
test.fixme()wrapper to re-enable the "Context menu opens and endpoint is pinned on Shift-drop" test - Reduced drag distance by approximately 50% (from +180/-140 to +90/-70) to decrease flake risk while maintaining test behavior
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.2 MB (baseline 3.2 MB) • ⚪ 0 BMain entry bundles and manifests
Graph Workspace — 972 kB (baseline 972 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
UI Components — 169 kB (baseline 169 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.81 MB (baseline 3.81 MB) • ⚪ 0 BBundles that do not match a named category
|
![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
🧹 Nitpick comments (1)
browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts (1)
865-878: Consider extracting internal state access into a helper.Accessing
window['app']?.canvas?.linkConnector?.state?.snapLinksPoscouples the test to internal implementation details. If the linkConnector structure changes, this test will break.Consider creating a helper function like
getSnapLinksPosition(page)to centralize this access pattern, making future refactoring easier:async function getSnapLinksPosition(page: Page) { return await page.evaluate(() => { const snap = window['app']?.canvas?.linkConnector?.state?.snapLinksPos return Array.isArray(snap) ? [snap[0], snap[1]] : null }) }Then use:
const before = await getSnapLinksPosition(comfyPage.page)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (browser_tests/CLAUDE.md)
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}: Test user workflows in browser tests
Use Playwright fixtures for browser tests
Follow naming conventions for browser tests
Check assets/ directory for test data when writing tests
Prefer specific selectors in browser tests
Test across multiple viewports
Files:
browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Unit and component tests should be located in
tests-ui/or co-located with components assrc/components/**/*.{test,spec}.ts; E2E tests should be inbrowser_tests/
Files:
browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
browser_tests/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Playwright E2E tests can use optional tags like
@mobileand@2xwhich are respected by the Playwright configuration
Files:
browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: test
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (1)
browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts (1)
831-879: LGTM! Test successfully re-enabled with proper assertions.The test correctly verifies that:
- Context menu opens on Shift-drop
- Pinned endpoint remains stable during mouse movement
The reduced drag distance (90, 70 vs. the 180, 140 used in Line 125) aligns with the PR objective to lower flakiness.
| // Context menu should be visible | ||
| const contextMenu = comfyPage.page.locator('.litecontextmenu') | ||
| await expect(contextMenu).toBeVisible() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -nP -A5 -B5 '\.litecontextmenu' browser_tests/tests/vueNodes/Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1374
🏁 Script executed:
cat -n browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts | sed -n '840,900p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2606
🏁 Script executed:
cat -n browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts | sed -n '1,50p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1969
🏁 Script executed:
grep -n "beforeEach\|afterEach\|beforeAll\|afterAll" browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 118
🏁 Script executed:
find browser_tests/fixtures -name "*ComfyPage*" -type fRepository: Comfy-Org/ComfyUI_frontend
Length of output: 105
🏁 Script executed:
cat -n browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts | sed -n '100,115p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 776
🏁 Script executed:
wc -l browser_tests/fixtures/ComfyPage.ts && head -100 browser_tests/fixtures/ComfyPage.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 3152
🏁 Script executed:
rg -n "litecontextmenu\|contextmenu" browser_tests/fixtures/ -A3 -B3Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
rg -n "keyboard.*Escape\|Escape.*key" browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
grep -n "escapeMenu\|closeMenu\|dismissMenu" browser_tests/fixtures/ComfyPage.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 96
🏁 Script executed:
cat -n browser_tests/fixtures/ComfyPage.ts | sed -n '1410,1425p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 582
🏁 Script executed:
rg -n "closeMenu\(\)" browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
rg -n "\.litecontextmenu\|page.keyboard.press.*Escape" browser_tests/tests/ -A2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
rg -n "test\('" browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1502
🏁 Script executed:
cat -n browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts | sed -n '873,890p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 842
🏁 Script executed:
grep -n "page.keyboard.press\|keyboard.down.*Escape\|click.*away" browser_tests/tests/vueNodes/interactions/links/*.spec.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
cat browser_tests/playwright.config.ts | grep -A10 -B10 "fullyParallel\|testDir\|beforeEach\|teardown"Repository: Comfy-Org/ComfyUI_frontend
Length of output: 136
🏁 Script executed:
find browser_tests -name "*.ts" -path "*fixtures*" -exec grep -l "beforeEach\|afterEach" {} \; | head -5Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
find . -name "playwright.config.*" -o -name "vitest.config.*" 2>/dev/null | head -5Repository: Comfy-Org/ComfyUI_frontend
Length of output: 111
🏁 Script executed:
cat -n browser_tests/fixtures/ComfyPage.ts | sed -n '1,30p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1553
🏁 Script executed:
grep -n "use\|fullyParallel\|testDir\|beforeEach\|afterEach\|cleanupTest" playwright.config.ts | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 894
Add explicit context menu dismissal to improve test cleanup.
The context menu is opened (line 862) but not explicitly closed before the test ends. While Playwright's page isolation between tests mitigates interference, explicitly dismissing the menu with await comfyPage.page.keyboard.press('Escape') or clicking elsewhere improves test clarity and resilience. Consider adding cleanup after the assertions on line 863.
🤖 Prompt for AI Agents
In browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts
around lines 861 to 863, the test asserts the context menu is visible but does
not close it; add an explicit dismissal after the assertion by sending Escape
(await comfyPage.page.keyboard.press('Escape')) or clicking outside (await
comfyPage.page.click('body', { position: { x: 0, y: 0 } })) to reliably close
the .litecontextmenu before the test ends and improve cleanup.
Summary
Testing
┆Issue is synchronized with this Notion page by Unito