Add queue overlay tests and stories #7342
Conversation
📝 WalkthroughWalkthroughThe changes add a new QueueList test component and update browser testing infrastructure with type-safe WebSocket messages. Waiting logic in dialog confirmation and workflow setup is removed. New queue UI tests and Storybook stories are introduced. The useJobList composable API renames a returned member. Changes
Possibly related PRs
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
🎭 Playwright Test Results❌ Some tests failed ⏰ Completed at: 12/11/2025, 04:17:03 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/11/2025, 04:04:46 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
dosubot
bot
added
the
size:L
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive test coverage and Storybook stories for the queue overlay functionality. It introduces Playwright end-to-end tests for queue list interactions, updates existing unit tests to align with refactored composable names, and provides visual documentation through Storybook stories.
- Adds Playwright tests for queue overlay toggle, count display, and job item interactions
- Creates Storybook stories for QueueInlineProgressSummary and updates QueueJobItem stories
- Refactors test expectations to use
orderedTasksinstead ofallTasksSortedfor consistency
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests-ui/tests/composables/useJobList.test.ts | Updates test descriptions and variable references to align with refactored composable using orderedTasks |
| src/components/queue/job/QueueJobItem.stories.ts | Adds runningNodeName prop to the RunningWithCurrent story for better demonstration |
| src/components/queue/QueueOverlayHeader.test.ts | Adds test coverage for close button click event emission |
| src/components/queue/QueueInlineProgressSummary.stories.ts | Creates comprehensive Storybook stories showcasing different progress states |
| browser_tests/tests/queue/queueList.spec.ts | Implements Playwright tests for queue overlay toggle, count updates, and job display |
| browser_tests/tests/actionbar.spec.ts | Refactors to use typed WsMessage for improved type safety |
| browser_tests/fixtures/ws.ts | Adds WsMessage type definition to improve WebSocket fixture type safety |
| browser_tests/fixtures/components/QueueList.ts | Introduces new page object model fixture for queue list interactions |
| browser_tests/fixtures/ComfyPage.ts | Integrates QueueList fixture and removes unnecessary wait logic from ConfirmDialog |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
browser_tests/fixtures/ComfyPage.ts(3 hunks)browser_tests/fixtures/components/QueueList.ts(1 hunks)browser_tests/fixtures/ws.ts(1 hunks)browser_tests/tests/actionbar.spec.ts(3 hunks)browser_tests/tests/queue/queueList.spec.ts(1 hunks)src/components/queue/QueueInlineProgressSummary.stories.ts(1 hunks)src/components/queue/QueueOverlayHeader.test.ts(2 hunks)src/components/queue/job/QueueJobItem.stories.ts(1 hunks)tests-ui/tests/composables/useJobList.test.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/components/queue/job/QueueJobItem.stories.tssrc/components/queue/QueueOverlayHeader.test.tssrc/components/queue/QueueInlineProgressSummary.stories.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safetyMinimize the surface area (exported values) of each module and composable
Files:
src/components/queue/job/QueueJobItem.stories.tssrc/components/queue/QueueOverlayHeader.test.tssrc/components/queue/QueueInlineProgressSummary.stories.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/components/queue/job/QueueJobItem.stories.tssrc/components/queue/QueueOverlayHeader.test.tssrc/components/queue/QueueInlineProgressSummary.stories.ts
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/components/queue/job/QueueJobItem.stories.tssrc/components/queue/QueueOverlayHeader.test.tssrc/components/queue/QueueInlineProgressSummary.stories.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/components/queue/job/QueueJobItem.stories.tssrc/components/queue/QueueOverlayHeader.test.tssrc/components/queue/QueueInlineProgressSummary.stories.ts
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/components/queue/job/QueueJobItem.stories.tssrc/components/queue/QueueOverlayHeader.test.tssrc/components/queue/QueueInlineProgressSummary.stories.ts
src/components/**/*.{vue,ts,js}
📄 CodeRabbit inference engine (src/components/CLAUDE.md)
src/components/**/*.{vue,ts,js}: Use existing VueUse composables (such as useElementHover) instead of manually managing event listeners
Use useIntersectionObserver for visibility detection instead of custom scroll handlers
Use vue-i18n for ALL UI strings
Files:
src/components/queue/job/QueueJobItem.stories.tssrc/components/queue/QueueOverlayHeader.test.tssrc/components/queue/QueueInlineProgressSummary.stories.ts
**/*.{ts,tsx,js,jsx,vue,json}
📄 CodeRabbit inference engine (AGENTS.md)
Code style: Use 2-space indentation, single quotes, no trailing semicolons, and 80-character line width (see
.prettierrc)
Files:
src/components/queue/job/QueueJobItem.stories.tsbrowser_tests/tests/queue/queueList.spec.tsbrowser_tests/fixtures/ws.tstests-ui/tests/composables/useJobList.test.tssrc/components/queue/QueueOverlayHeader.test.tssrc/components/queue/QueueInlineProgressSummary.stories.tsbrowser_tests/tests/actionbar.spec.tsbrowser_tests/fixtures/ComfyPage.tsbrowser_tests/fixtures/components/QueueList.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Imports must be sorted and grouped by plugin; runpnpm formatbefore committing
Use TypeScript for type safety; never useanytype - use proper TypeScript types
Never useas anytype assertions; fix the underlying type issue instead
Use es-toolkit for utility functions
Write code that is expressive and self-documenting; avoid comments unless absolutely necessary; do not add or retain redundant comments
Keep functions short and functional
Minimize nesting in code (e.g., deeply nestediforforstatements); apply the Arrow Anti-Pattern principle
Avoid mutable state; prefer immutability and assignment at point of declaration
Favor pure functions, especially testable ones
Files:
src/components/queue/job/QueueJobItem.stories.tsbrowser_tests/tests/queue/queueList.spec.tsbrowser_tests/fixtures/ws.tstests-ui/tests/composables/useJobList.test.tssrc/components/queue/QueueOverlayHeader.test.tssrc/components/queue/QueueInlineProgressSummary.stories.tsbrowser_tests/tests/actionbar.spec.tsbrowser_tests/fixtures/ComfyPage.tsbrowser_tests/fixtures/components/QueueList.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/queue/queueList.spec.tsbrowser_tests/tests/actionbar.spec.ts
browser_tests/**/*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
browser_tests/**/*.spec.ts: E2E test files must be namedbrowser_tests/**/*.spec.tsand use Playwright
Follow Playwright best practices described in the official documentation for E2E tests
Do not usewaitForTimeoutin Playwright tests; use Locator actions and retrying assertions instead
Use tags like@mobileand@2xin Playwright tests for relevant test scenarios
Files:
browser_tests/tests/queue/queueList.spec.tsbrowser_tests/tests/actionbar.spec.ts
tests-ui/**/*.test.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (tests-ui/CLAUDE.md)
tests-ui/**/*.test.{js,ts,jsx,tsx}: Write tests for new features
Follow existing test patterns in the codebase
Use existing test utilities rather than writing custom utilities
Mock external dependencies in tests
Always prefer vitest mock functions over writing verbose manual mocks
Files:
tests-ui/tests/composables/useJobList.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Write tests for all changes, especially bug fixes to catch future regressions
Unit/component test files must be named**/*.test.tsor intests-ui/directory
Do not write change detector tests that just assert default values
Do not write tests dependent on non-behavioral features like utility classes or styles
Be parsimonious in testing; do not write redundant tests; see composable tests approach
Follow 'Don't Mock What You Don't Own' principle - avoid mocking external dependencies
Do not write tests that just test the mocks; ensure tests fail when code behaves unexpectedly
Leverage Vitest's mocking utilities where possible for test mocking
Keep module mocks contained in test files; do not use global mutable state within test files; usevi.hoisted()if necessary
For Component testing, use Vue Test Utils and follow advice about making components easy to test
Aim for behavioral coverage of critical and new features in unit tests
Files:
tests-ui/tests/composables/useJobList.test.tssrc/components/queue/QueueOverlayHeader.test.ts
🧠 Learnings (27)
📓 Common learnings
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test user workflows in browser tests
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
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/components/queue/job/QueueJobItem.stories.tsbrowser_tests/tests/queue/queueList.spec.tsbrowser_tests/fixtures/ws.tstests-ui/tests/composables/useJobList.test.tssrc/components/queue/QueueOverlayHeader.test.tssrc/components/queue/QueueInlineProgressSummary.stories.tsbrowser_tests/tests/actionbar.spec.tsbrowser_tests/fixtures/ComfyPage.tsbrowser_tests/fixtures/components/QueueList.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/components/queue/job/QueueJobItem.stories.tsbrowser_tests/tests/queue/queueList.spec.tsbrowser_tests/fixtures/ws.tstests-ui/tests/composables/useJobList.test.tssrc/components/queue/QueueOverlayHeader.test.tssrc/components/queue/QueueInlineProgressSummary.stories.tsbrowser_tests/tests/actionbar.spec.tsbrowser_tests/fixtures/ComfyPage.tsbrowser_tests/fixtures/components/QueueList.ts
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to browser_tests/**/*.spec.ts : Follow Playwright best practices described in the official documentation for E2E tests
Applied to files:
browser_tests/tests/queue/queueList.spec.tsbrowser_tests/fixtures/ws.tsbrowser_tests/tests/actionbar.spec.tsbrowser_tests/fixtures/components/QueueList.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Use Playwright fixtures for browser tests
Applied to files:
browser_tests/tests/queue/queueList.spec.tsbrowser_tests/fixtures/ws.tsbrowser_tests/tests/actionbar.spec.tsbrowser_tests/fixtures/components/QueueList.ts
📚 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/queue/queueList.spec.tstests-ui/tests/composables/useJobList.test.tssrc/components/queue/QueueOverlayHeader.test.tsbrowser_tests/tests/actionbar.spec.tsbrowser_tests/fixtures/ComfyPage.tsbrowser_tests/fixtures/components/QueueList.ts
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to browser_tests/**/*.spec.ts : E2E test files must be named `browser_tests/**/*.spec.ts` and use Playwright
Applied to files:
browser_tests/tests/queue/queueList.spec.tsbrowser_tests/tests/actionbar.spec.tsbrowser_tests/fixtures/components/QueueList.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test user workflows in browser tests
Applied to files:
browser_tests/tests/queue/queueList.spec.tsbrowser_tests/tests/actionbar.spec.tsbrowser_tests/fixtures/ComfyPage.tsbrowser_tests/fixtures/components/QueueList.ts
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to browser_tests/**/*.spec.ts : Do not use `waitForTimeout` in Playwright tests; use Locator actions and retrying assertions instead
Applied to files:
browser_tests/tests/queue/queueList.spec.tsbrowser_tests/tests/actionbar.spec.tsbrowser_tests/fixtures/ComfyPage.tsbrowser_tests/fixtures/components/QueueList.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Follow naming conventions for browser tests
Applied to files:
browser_tests/tests/queue/queueList.spec.tstests-ui/tests/composables/useJobList.test.tsbrowser_tests/tests/actionbar.spec.ts
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to browser_tests/**/*.spec.ts : Use tags like `mobile` and `2x` in Playwright tests for relevant test scenarios
Applied to files:
browser_tests/tests/queue/queueList.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Prefer specific selectors in browser tests
Applied to files:
browser_tests/tests/queue/queueList.spec.tsbrowser_tests/tests/actionbar.spec.tsbrowser_tests/fixtures/components/QueueList.ts
📚 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} : Use existing test utilities rather than writing custom utilities
Applied to files:
browser_tests/tests/queue/queueList.spec.tstests-ui/tests/composables/useJobList.test.tsbrowser_tests/tests/actionbar.spec.ts
📚 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: Check tests-ui/README.md for test guidelines
Applied to files:
browser_tests/tests/queue/queueList.spec.ts
📚 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} : Mock external dependencies in tests
Applied to files:
browser_tests/tests/queue/queueList.spec.tsbrowser_tests/tests/actionbar.spec.ts
📚 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} : Follow existing test patterns in the codebase
Applied to files:
browser_tests/tests/queue/queueList.spec.tstests-ui/tests/composables/useJobList.test.tsbrowser_tests/tests/actionbar.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test across multiple viewports
Applied to files:
browser_tests/tests/queue/queueList.spec.ts
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to **/*.test.ts : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
tests-ui/tests/composables/useJobList.test.tsbrowser_tests/tests/actionbar.spec.ts
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to **/*.test.ts : Be parsimonious in testing; do not write redundant tests; see composable tests approach
Applied to files:
tests-ui/tests/composables/useJobList.test.tsbrowser_tests/tests/actionbar.spec.ts
📚 Learning: 2025-12-10T03:09:13.807Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7303
File: src/components/topbar/CurrentUserPopover.test.ts:199-205
Timestamp: 2025-12-10T03:09:13.807Z
Learning: In test files, prefer selecting or asserting on accessible properties (text content, aria-label, role, accessible name) over data-testid attributes. This ensures tests validate actual user-facing behavior and accessibility, reducing reliance on implementation details like test IDs.
Applied to files:
tests-ui/tests/composables/useJobList.test.tssrc/components/queue/QueueOverlayHeader.test.ts
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to **/*.test.ts : Aim for behavioral coverage of critical and new features in unit tests
Applied to files:
browser_tests/tests/actionbar.spec.ts
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to **/*.test.ts : Do not write tests dependent on non-behavioral features like utility classes or styles
Applied to files:
browser_tests/tests/actionbar.spec.ts
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to **/*.test.ts : Leverage Vitest's mocking utilities where possible for test mocking
Applied to files:
browser_tests/tests/actionbar.spec.ts
📚 Learning: 2025-12-09T03:50:03.127Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/platform/updates/components/WhatsNewPopup.vue:5-13
Timestamp: 2025-12-09T03:50:03.127Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, when avoiding PrimeVue components, prefer using the repo's common button components from src/components/button/ (IconButton.vue, TextButton.vue, IconTextButton.vue) rather than plain HTML buttons. These components wrap PrimeVue with custom design system styling.
Applied to files:
browser_tests/fixtures/ComfyPage.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue TabMenu component with Tabs without panels
Applied to files:
browser_tests/fixtures/ComfyPage.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue Sidebar component with Drawer
Applied to files:
browser_tests/fixtures/ComfyPage.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.{vue,ts,js} : Use existing VueUse composables (such as useElementHover) instead of manually managing event listeners
Applied to files:
browser_tests/fixtures/ComfyPage.ts
🧬 Code graph analysis (5)
browser_tests/fixtures/ws.ts (1)
src/schemas/apiSchema.ts (1)
StatusWsMessage(136-136)
src/components/queue/QueueOverlayHeader.test.ts (1)
browser_tests/fixtures/components/QueueList.ts (1)
closeButton(19-21)
src/components/queue/QueueInlineProgressSummary.stories.ts (5)
src/platform/workflow/management/stores/workflowStore.ts (1)
ComfyWorkflow(35-139)src/platform/workflow/validation/schemas/workflowSchema.ts (1)
ComfyWorkflowJSON(460-462)src/scripts/changeTracker.ts (1)
ChangeTracker(28-446)src/stores/executionStore.ts (1)
useExecutionStore(106-667)src/schemas/apiSchema.ts (2)
NodeProgressState(151-151)ProgressWsMessage(137-137)
browser_tests/tests/actionbar.spec.ts (1)
browser_tests/fixtures/ws.ts (1)
WsMessage(5-5)
browser_tests/fixtures/ComfyPage.ts (1)
browser_tests/fixtures/components/QueueList.ts (1)
QueueList(4-57)
🔇 Additional comments (20)
src/components/queue/QueueOverlayHeader.test.ts (2)
39-39: LGTM - i18n message added.The "close" label addition to the i18n messages properly supports the new Close button test.
99-106: LGTM - Close button test follows accessibility best practices.The test correctly uses
aria-label="Close"to locate the button, validating user-facing accessibility rather than implementation details like test IDs.src/components/queue/job/QueueJobItem.stories.ts (1)
67-68: LGTM - Story enhanced with running node name.The addition of
runningNodeNameto the story args provides visual documentation for the running node state, aligning with the broader queue UI testing additions in this PR.browser_tests/fixtures/ws.ts (1)
3-8: Excellent type safety improvement.Replacing
anywith the constrainedWsMessagetype provides compile-time validation of WebSocket payloads used in tests, reducing the likelihood of malformed test data.tests-ui/tests/composables/useJobList.test.ts (2)
392-392: LGTM - Test description updated consistently.The test description properly reflects the queue-order-based grouping behavior after the API rename.
315-334: Composable exportsorderedTaskscorrectly.The test's use of
orderedTasksis verified—the property is exported from the composable implementation alongsidejobItems,groupedJobItems, andcurrentNodeName.src/components/queue/QueueInlineProgressSummary.stories.ts (3)
146-168: LGTM - Story demonstrates running node progress.The
RunningKSamplerstory effectively seeds execution state to visualize inline progress with a specific running node and progress values.
170-192: LGTM - Story covers fallback node name rendering.The
RunningWithFallbackNamestory validates the UI behavior when a node lacks a title, demonstrating the fallback to node type.
194-211: LGTM - Story covers progress without active node.The
ProgressWithoutCurrentNodestory validates overall progress display when no specific node is actively running.browser_tests/fixtures/ComfyPage.ts (2)
16-16: LGTM - QueueList fixture integration.The new
queueListfield enables tests to interact with the queue overlay UI through a well-structured page object, following Playwright best practices.Also applies to: 155-155, 188-188
126-130: No action needed. TheconfirmDialog.click()method doesn't introduce race conditions. Playwright'sLocator.click()automatically waits for the element to be actionable, and the subsequentcloseToasts(1)call uses a retrying assertion (expect().toHaveCount(1)) that ensures the page has settled before proceeding. This follows Playwright best practices by avoiding explicit timeouts.browser_tests/tests/actionbar.spec.ts (2)
4-6: LGTM - Import paths updated for typed fixtures.The imports now use the typed
WsMessagefrom the updated fixture, improving type safety across tests.
64-76: LGTM - WebSocket message construction is now type-safe.Using
satisfies WsMessageensures compile-time validation of the message structure, preventing malformed test data.browser_tests/tests/queue/queueList.spec.ts (5)
31-57: LGTM - Proper test isolation with mocked routes.The
beforeEachhook correctly mocks/api/promptand/api/historyendpoints to prevent real network requests and ensure test isolation.
59-82: LGTM - Test validates overlay toggle and count updates.The test comprehensively verifies:
- Initial empty state display
- Count updates from WebSocket status events
- Overlay visibility toggling
- Job item count matching queue state
Follows Playwright best practices using retrying assertions instead of timeouts.
84-104: LGTM - Test validates job display and hover interactions.The test verifies running and pending jobs are displayed correctly and that hover reveals cancel actions, providing good coverage of queue UI interactions.
119-157: Excellent queue controller pattern.The
createQueueControllerhelper provides a clean abstraction for:
- Managing in-memory queue state
- Mocking API responses based on that state
- Synchronizing state via WebSocket events and API fetches
This pattern makes tests readable and maintainable.
10-29: Verify ComfyUI queue API contract before implementing tests.The
QueueJobtuple type structure[string, string, Record<string, unknown>, Record<string, unknown>, string[]]should be validated against the actual ComfyUI queue API response format. Before finalizing the test implementation, confirm:
- The exact field order and types in ComfyUI's queue job response
- How the
runningandpendingqueue states are returned from the API- Whether the tuple elements should be properly typed (avoiding bare
Record<string, unknown>) based on actual API contractsConsider creating shared types in a dedicated file (e.g.,
src/types/queue.ts) rather than inline test-only types to enable code reuse and ensure consistency across the codebase.browser_tests/fixtures/components/QueueList.ts (2)
4-30: QueueList fixture and locator getters are well-factoredEncapsulating the queue overlay into a dedicated fixture with focused getters for each control keeps the E2E tests readable and aligned with the “prefer specific selectors” guidance (data-testid / role-based). No issues here from a correctness or maintainability standpoint.
Based on learnings, this matches the browser_tests selector guidance.
31-43: Idempotentopen/closehelpers look good and avoid timeoutsThe guards around
overlay.isVisible()make these helpers idempotent, and usingexpect(...).toBeVisible()/not.toBeVisible()rather thanwaitForTimeoutaligns with the Playwright best practices called out in AGENTS.md.If you want to double-check behavior, verify in Playwright docs that
locator.isVisible()has the semantics you expect in your DOM setup (attached-but-hidden vs not-attached elements).
| async getJobCount(state?: string) { | ||
| if (state) { | ||
| return await this.page | ||
| .locator(`[data-testid="queue-job-item"][data-job-state="${state}"]`) | ||
| .count() | ||
| } | ||
| return await this.jobItems.count() | ||
| } | ||
|
|
||
| getJobAction(actionKey: string) { | ||
| return this.page.getByTestId(`job-action-${actionKey}`) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider tightening typing for state and actionKey to match queue/job domain
getJobCount(state?: string) and getJobAction(actionKey: string) currently accept plain strings, while the underlying UI likely has a closed set of job states and actions.
If there is already a shared string-literal union or enum for job states / action keys elsewhere in the codebase (e.g., used by useJobList or job type definitions), it would be preferable to reuse that type here instead of bare string. That keeps tests in sync with the domain model and prevents typos in selectors from compiling.
If no such shared type exists yet, this is fine for now, but consider introducing one when you touch this area again.
🤖 Prompt for AI Agents
In browser_tests/fixtures/components/QueueList.ts around lines 45 to 56, the
parameters typed as plain string (state?: string and actionKey: string) are too
permissive; change them to the existing job-state and job-action string-literal
union or enum used by the app (or create and export a small union type in a
shared test/types file if none exists), update imports accordingly, and update
the method signatures to use those types so callers and selectors are checked at
compile time for valid job states/actions.
| function resetExecutionStore() { | ||
| const exec = useExecutionStore() | ||
| exec.activePromptId = null | ||
| exec.queuedPrompts = {} | ||
| exec.nodeProgressStates = {} | ||
| exec.nodeProgressStatesByPrompt = {} | ||
| exec._executingNodeProgress = null | ||
| exec.lastExecutionError = null | ||
| exec.lastNodeErrors = null | ||
| exec.initializingPromptIds = new Set() | ||
| exec.promptIdToWorkflowId = new Map() | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find Storybook configuration files and global setup
echo "=== Storybook Configuration Files ==="
find .storybook -type f \( -name "*.ts" -o -name "*.js" -o -name "*.json" \) 2>/dev/null | head -20
echo ""
echo "=== Checking Storybook preview/main config ==="
ls -la .storybook/ 2>/dev/null || echo "No .storybook directory found"
echo ""
echo "=== Looking for global Storybook decorators ==="
rg -t ts -t js 'export.*decorators|preview\.' .storybook/ 2>/dev/null || echo "No decorators found"Repository: Comfy-Org/ComfyUI_frontend
Length of output: 686
🏁 Script executed:
#!/bin/bash
# Read the story file to understand how resetExecutionStore is used
echo "=== Story file: QueueInlineProgressSummary.stories.ts ==="
cat -n src/components/queue/QueueInlineProgressSummary.stories.ts 2>/dev/null || echo "File not found"Repository: Comfy-Org/ComfyUI_frontend
Length of output: 6864
🏁 Script executed:
#!/bin/bash
# Search for store reset patterns and decorators in Storybook stories
echo "=== Looking for beforeEach/setup patterns in stories ==="
rg -t ts 'beforeEach|beforeAll|Meta|decorators' src/components/queue/QueueInlineProgressSummary.stories.ts 2>/dev/null
echo ""
echo "=== Checking for any reset patterns in this specific story ==="
rg -t ts 'resetExecutionStore|render.*store|setup' src/components/queue/QueueInlineProgressSummary.stories.ts 2>/dev/nullRepository: Comfy-Org/ComfyUI_frontend
Length of output: 404
🏁 Script executed:
#!/bin/bash
# Look for similar store reset patterns in other story files
echo "=== Looking for store reset patterns in other stories ==="
rg -t ts 'resetExecutionStore|reset.*Store' src/components/ 2>/dev/null | head -20
echo ""
echo "=== Checking for Storybook decorator patterns across stories ==="
rg -t ts 'meta\.decorators|export const meta' src/components/*.stories.ts 2>/dev/null | head -10Repository: Comfy-Org/ComfyUI_frontend
Length of output: 965
Store is reset in each story's setup() via seedExecutionState(), but consider using a global Storybook decorator for consistency.
The resetExecutionStore() function is already being called before each story renders—it's invoked within seedExecutionState() (line 84), which is called from every story's setup() function. This prevents state pollution between stories in practice.
However, this pattern relies on story developers remembering to call seedExecutionState(). A global Storybook decorator would be more robust and prevent accidental state leakage if a future story skips this call.
🤖 Prompt for AI Agents
In src/components/queue/QueueInlineProgressSummary.stories.ts around lines 62 to
73, the store reset function is only invoked indirectly via seedExecutionState()
in each story's setup, which risks future omissions; add a global Storybook
decorator (in your .storybook/preview.ts or equivalent) that calls
resetExecutionStore() (or seedExecutionState()) before rendering every story so
the execution store is consistently cleared for all stories, removing reliance
on per-story setup and preventing state leakage.
|
Are the tests meant to be failing? |
christian-byrne
left a comment
christian-byrne
left a comment
There was a problem hiding this comment.
I noticed that in this state of the job details popover (https://b53a45f9.comfy-storybook.pages.dev/?path=/story/queue-jobdetailspopover--completed), if the completion time is 1s, it shows compute hours used as 0.000. That seems incorrect right?
I don't see it in the storybook, but I'm taking a look |
Yeah, needed new generations |
benceruleanlu
added
the
New Browser Test Expectations
|
Updating Playwright Expectations |
benceruleanlu
removed
the
New Browser Test Expectations
I was using the storybook to just easily point to the component I was referencing. The issue happens e.g. when creating a workflow that's just Load Image => Save Image. The "compute hours used" will show 0 - is that intended? |
christian-byrne
merged commit 53dbca9
into
queue-overlay-additions
I guess it just rounded, I'll change it to <0.001 hours or just 0.001 |
3 participants
Summary
main <-- #7336 <-- #7338 <-- #7342
┆Issue is synchronized with this Notion page by Unito