mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-05 21:54:50 +00:00
a9efd4de6217c6f4dbfc375a01e738b764aa7fe7
991 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
pythongosssss
|
ac728b92ae |
fix: fix webcam node not showing preview in nodes 2.0 (#11549)
## Summary Adds test coverage for webcam node & fixes issue found in testing where the captured image does not show in nodes 2.0 ## Changes - **What**: - call `setNodePreviewsByNodeId` alongside `node.imgs = [img]` - add tests for general coverage ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11549-fix-fix-webcam-node-not-showing-preview-in-nodes-2-0-34a6d73d3650810c89eee9c25cd07700) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
jaeone94
|
cbc479d8b4 |
refactor: extract test helpers and use UI-based subgraph entry in draft position test (#11327)
## Summary Follow-up to #10828. Addresses all deferred review nits from @DrJKL tracked in #10932. - Remove YAGNI `timeout` parameter from `waitForDraftPersisted` — default 5s from `waitForFunction` is sufficient - Extract `reloadAndWaitForApp()` into `WorkflowHelper` — preserves localStorage (drafts) and URL hash (subgraph navigation), unlike `ComfyPage.setup()` which clears storage and navigates to base URL - Replace programmatic `canvas.setGraph()` with `vueNodes.enterSubgraph()` for real UI-based subgraph entry - Add `@vue-nodes` tag required for `enterSubgraph()` button rendering - Extract `getSubgraphNodePositions` to deduplicate three identical inline `page.evaluate` calls - Fix vacuous pass: capture `positionsBefore` inside `expect.poll` to ensure the array is non-empty before the verification loop - Remove inline comments, relying on descriptive helper method names ## Test plan - [x] `pnpm typecheck:browser` passes - [x] `pnpm lint` passes - [x] `pnpm test:browser:local -- browser_tests/tests/subgraph/subgraphDraftPositions.spec.ts` passes locally Fixes #10932 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11327-refactor-extract-test-helpers-and-use-UI-based-subgraph-entry-in-draft-position-test-3456d73d3650813cacc1e69398e3f80a) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: GitHub Action <action@github.com> |
||
|
pythongosssss
|
5a598ef2e1 |
test: add GLSL execution e2e test (#11516)
## Summary Add e2e tests for GLSL shader execution ## Changes - **What**: - add test workflows containing GLSL nodes - tests execution, value propagation, error, subgraph handling - adds console warn on invalid shader to surface error and allow test to detect ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11516-tst-add-GLSL-execution-e2e-test-3496d73d36508199a8e0fa341186ee4d) by [Unito](https://www.unito.io) |
||
|
Dante
|
2c772077e0 |
test: add E2E tests for billing dialogs (CancelSubscription, TopUpCredits) (#10969)
## Summary - Add Playwright E2E tests for `CancelSubscriptionDialogContent` and `TopUpCreditsDialogContentLegacy` - CancelSubscription tests: dialog display with date formatting, keep subscription dismiss, confirm cancel with mocked API, error handling on API failure - TopUpCredits tests: dialog display with preset amounts, insufficient credits variant, preset selection, close button dismiss, pricing link visibility Part of the FixIt Burndown test coverage initiative (Untested Dialogs). ## Test plan - [ ] Verify tests pass in CI against OSS build - [ ] `pnpm test:browser:local -- browser_tests/tests/dialogs/cancelSubscriptionDialog.spec.ts` - [ ] `pnpm test:browser:local -- browser_tests/tests/dialogs/topUpCreditsDialog.spec.ts` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10969-test-add-E2E-tests-for-billing-dialogs-CancelSubscription-TopUpCredits-33c6d73d36508164b268c08c99464ca1) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
71ca582325 |
fix: reset file input value after selection to allow same-file reupload (#11417)
*PR Created by the Glary-Bot Agent* --- ## Summary Fixes the "choose video to upload" button becoming unresponsive after running a workflow with a subgraph a few times. **Root cause**: The detached input element in `useNodeFileInput` never resets its `value`. The browser's `onchange` only fires when the value *changes* — re-selecting the same file silently drops the event. A page refresh recreates the input with an empty value, which is why refreshing fixes it. ## Changes - `useNodeFileInput.ts`: Reset `fileInput.value` before invoking callbacks so value is cleared even if a callback throws - `useNodeDragAndDrop.ts`: Add `onRemoved` cleanup for installed handlers (only clears own handlers; preserves replacements from extensions) - `useNodePaste.ts`: Add `onRemoved` cleanup for installed `pasteFiles` handler (same reference-safe pattern) - 3 new colocated test files with 26 test cases covering all branches ## Codebase Audit Audited all 11 file upload implementations across the codebase. Found 5 using the ghost/virtual input pattern — 3 with the same missing value-reset bug: - `useNodeFileInput.ts` — fixed in this PR - `scripts/utils.ts` (`uploadFile()`) — one-shot pattern, lower risk - `extensions/core/load3d.ts` — partial reset only The 4 Vue component implementations already reset correctly. ## Future Work VueUse `useFileDialog` composable handles same-file reselection via `reset: true` and provides automatic lifecycle cleanup. A follow-up PR could migrate the ghost input patterns for a centralized solution. ## Test Plan - 26 unit tests across 3 new test files (all pass) - 9 existing useNodeImageUpload tests still pass - Pre-commit hooks pass (oxfmt, oxlint, eslint, typecheck) - Oracle code review addressed ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11417-fix-reset-file-input-value-after-selection-to-allow-same-file-reupload-3476d73d3650814d95efdab602a3852d) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: GitHub Action <action@github.com> |
||
|
pythongosssss
|
9ed7a7bd87 |
test: Add tests for help center (#11475)
## Summary Test coverage for help center & associated popups ## Changes - **What**: - Adds HelpCenterHelper for mocking endpoints and locators - Tests for popup, menu items & positioning ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11475-test-Add-tests-for-help-center-3486d73d365081af91a2eb7465e503fe) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
3e62033f09 |
test: extract TestIdValue as mapped type (#11474)
## Summary Prevent needing to update the union with newly added keys ## Changes - **What**: - Change the `TestIdValue` union to a mapped type, excluding function values ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11474-test-extract-TestIdValue-as-mapped-type-3486d73d365081d299efd87a0c46d66f) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
b36242475c |
test: add E2E tests for topbar menu commands (#11208)
## Summary Add 5 Playwright E2E tests covering topbar menu command interactions. ## Changes - **What**: New test file `browser_tests/tests/topbarMenuCommands.spec.ts` with 5 tests: - New command creates a new workflow tab - Edit > Undo undoes the last action - Edit > Redo restores an undone action - File > Save opens save dialog - View > Bottom Panel toggles bottom panel visibility ## Review Focus Tests use `triggerTopbarCommand()` for menu navigation and `expect.poll()` for async assertions. The "New" command is a top-level menu item (path `["New"]`), not nested under File. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11208-test-add-E2E-tests-for-topbar-menu-commands-3416d73d36508143afe5e67a98910f56) by [Unito](https://www.unito.io) |
||
|
Benjamin Lu
|
d83c84aa85 |
test: extract asset api browser fixture (#11279)
## Summary Move asset API mocking off `ComfyPage` and into a standalone Playwright fixture. ## Changes - add `assetApiFixture` for browser tests that need asset API mocking - remove `assetApi` from `ComfyPage` - migrate `browser_tests/tests/assetHelper.spec.ts` to use the standalone fixture ## Why This is the first slice of the browser-fixture split. It reduces global fixture surface area without changing test behavior. ## Validation - `pnpm typecheck:browser` - `pnpm exec oxlint browser_tests/fixtures/ComfyPage.ts browser_tests/fixtures/assetApiFixture.ts browser_tests/tests/assetHelper.spec.ts --type-aware` - repo hooks during commit/push: `pnpm typecheck`, `pnpm typecheck:browser`, `pnpm knip` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11279-test-extract-asset-api-browser-fixture-3436d73d3650818393bcd43dc909c8a2) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
35bfe509b3 |
test: add/update terminal tests (#11239)
## Summary Adds test coverage for the integrated terminal ## Changes - **What**: - refactor and simplify existing tests - add new tests for xterm integration ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11239-test-add-update-terminal-tests-3426d73d365081c99445c35d8808afb4) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
a1ba567dbc |
test: remove --listen 0.0.0.0 from E2E test mock argv (#11021)
## Summary Remove `--listen 0.0.0.0` from mock `argv` in E2E test fixtures to avoid normalizing a flag that exposes the server to all network interfaces. ## Changes - **What**: Removed `--listen` and `0.0.0.0` from `mockSystemStats.system.argv` in `browser_tests/fixtures/data/systemStats.ts` (shared fixture) and the ManagerDialog-specific override in `browser_tests/tests/dialogs/managerDialog.spec.ts`. Neither value is required for any test assertion. Fixes #11008 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11021-test-remove-listen-0-0-0-0-from-E2E-test-mock-argv-33e6d73d365081c59d3fe9610afbeb6f) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
b157182a20 |
refactor: inline node footer layout to fix selection bounding box (#10741)
## Summary Refactor node footer from absolute overlay to inline flow layout, fixing the selection bounding box not encompassing footer buttons and collapsed node dimensions. ## Background The node footer (Enter Subgraph, Advanced, Error buttons) was rendered as an absolute overlay (`absolute top-full`) outside the node body. This caused: 1. **Selection bounding box** did not include footer height — the dashed multi-select border cut through footer buttons 2. **Footer offset compensation** required 3 hardcoded computed classes (`footerStateOutlineBottomClass`, `footerRootBorderBottomClass`, `footerResizeHandleBottomClass`) with magic pixel values (31px, 35px, etc.) that had to stay in sync with CSS ## Solution: Inline Footer with `isolate -z-1` The footer is moved into normal document flow (no longer `absolute top-full`). The key challenge was keeping the footer visually behind the body's rounded bottom edge (the "tuck under" effect) without adding `z-index` to the body — because adding `z-index` to the body creates a stacking context that traps slot connection dots, making them appear behind overlay borders. The solution uses CSS `isolation: isolate` combined with `-z-1` on the footer wrapper: - **`isolate`** creates an independent stacking context for the footer, so internal z-index (Error button `z-10` above Enter button) does not leak to the parent - **`-z-1`** places the entire footer behind the body (`z-index: auto`), achieving the visual overlap without touching the body's stacking behavior - **Slot dots remain free** — the body has no explicit z-index, so slots participate in the root stacking context and are never trapped behind overlay borders This eliminates all 3 footer offset computed classes and their hardcoded pixel values. ## Selection Box: `min-height` on root + unified size path Moving `min-h-(--node-height)` from the body (`node-inner-wrapper`) to the root element makes the footer height naturally included in `node.size` via ResizeObserver → layoutStore → litegraph sync. This means `boundingRect` is automatically correct for expanded nodes — no callbacks or overrides needed. For collapsed nodes, a pre-existing issue (since v1.40) caused `_collapsed_width` to fall back to `NODE_COLLAPSED_WIDTH = 80px` because Vue nodes lack a canvas context for text measurement. The fix lets collapsed dimensions flow through the **same** `batchUpdateNodeBounds` path as expanded nodes — no parallel data structure, no separate accessor, no cache: 1. ResizeObserver writes the collapsed DOM dimensions to `layoutStore.size` via `batchUpdateNodeBounds` 2. `useLayoutSync` syncs `layoutStore.size` → `liteNode.size` as it does for any other size change 3. The expanded size survives the collapse→expand round trip via CSS custom properties — the `isCollapsed` watcher in `LGraphNode.vue` swaps `--node-width` to `--node-width-x` on collapse and restores it on expand 4. `measure()` reads `this.size` directly for Vue collapsed nodes via a one-line gate: `if (!this.flags?.collapsed || LiteGraph.vueNodesMode)`. Legacy behavior is unchanged. ## Changes - **NodeFooter.vue**: `absolute top-full` overlay → inline flow with `isolate -z-1` wrappers, Error/Enter button layering via `-mr-5` + DOM order, reactive props destructuring, static `RADIUS_CLASS` lookup for Tailwind scanning, Vue 3.3+ `defineEmits` property syntax - **LGraphNode.vue**: Move `min-h-(--node-height)` from body to root; remove `footerStateOutlineBottomClass`, `footerRootBorderBottomClass`, `footerResizeHandleBottomClass`, `hasFooter` computed; replace dynamic `beforeShapeClass` interpolation with static `bypassOverlayClass`/`mutedOverlayClass` computeds for Tailwind scanning - **LGraphNode.ts**: `measure()` collapsed branch gated by `|| LiteGraph.vueNodesMode` — Vue mode defers to `this.size`; legacy path unchanged - **useVueNodeResizeTracking.ts**: Collapsed and expanded nodes both flow through `batchUpdateNodeBounds`; narrowed `useVueElementTracking` parameter from `MaybeRefOrGetter<string>` to `string`; `deferredElements.delete(element)` on unmount to prevent memory retention - **selectionBorder.ts**: Unchanged — `createBounds` just works because `boundingRect` is now correct - **12 parameterized E2E tests**: Vue mode (subgraph/regular × expanded/collapsed × bottom-left/bottom-right) + legacy mode (expanded/collapsed × bottom-left/bottom-right), driven by `keyboard.collapse()` (Alt+C) - **Unit tests**: `measure()` branching (legacy fallback, Vue `this.size` usage, expanded parity) - **Shared test helpers**: `repositionNodes`, `KeyboardHelper.collapse`, `measureSelectionBounds`, `assertSelectionEncompassesNodes` ## Review Focus - `isolate -z-1` CSS layering pattern — is this acceptable long-term? - `measure()` collapsed branch gated on `LiteGraph.vueNodesMode` — one-line gate to avoid the canvas-ctx-less fallback in Vue mode - Footer button overlap design (`-mr-5` with DOM order for painting) ## Screenshots <img width="1392" height="800" alt="image" src="https://github.com/user-attachments/assets/abaebff5-bb8c-4b5b-8734-8d44fdee4cb9" /> <img width="1493" height="872" alt="image" src="https://github.com/user-attachments/assets/6b9c77f9-e3ae-4d4e-81dc-acfa9a24c768" /> <img width="813" height="515" alt="image" src="https://github.com/user-attachments/assets/ce15bafb-e157-408c-971b-a650088f316a" /> <img width="1031" height="669" alt="image" src="https://github.com/user-attachments/assets/20fdc336-4bc2-4d47-ab7e-c0cbcee0d150" /> <img width="753" height="525" alt="image" src="https://github.com/user-attachments/assets/2dccbe31-7d18-49bc-9ed4-158b1659fddf" /> <img width="730" height="370" alt="image" src="https://github.com/user-attachments/assets/ab87edfa-a4b4-46f7-86ae-4965a4509b42" /> <img width="1132" height="465" alt="image" src="https://github.com/user-attachments/assets/54643f5b-4a31-4c3d-9475-c433f87aedb0" /> <img width="1102" height="449" alt="image" src="https://github.com/user-attachments/assets/9c045df3-e1f5-481e-b1cb-ead1db1626f5" /> --------- Co-authored-by: github-actions <github-actions@github.com> |
||
|
Terry Jia
|
deba72e7a0 |
gizmo controls (#11274)
## Summary Add Gizmo transform controls to load3d - Remove automatic model normalization (scale + center) on load; models now appear at their original transform. The previous auto-normalization conflicted with gizmo controls — applying scale/position on load made it impossible to track and reset the user's intentional transform edits vs. the system's normalization - Add a manual Fit to Viewer button that performs the same normalization on demand, giving users explicit control - Add Gizmo Controls (translate/rotate) for interactive model manipulation with full state persistence across node properties, viewer dialog, and model reloads - Gizmo transform state is excluded from scene capture and recording to keep outputs clean ## Motivation The gizmo system is a prerequisite for these potential features: - Custom cameras — user-placed cameras in the scene need transform gizmos for precise positioning and orientation - Custom lights — scene lighting setup requires the ability to interactively position and aim light sources - Multi-object scene composition — positioning multiple models relative to each other requires per-object transform controls - Pose editor — skeletal pose editing depends on the same transform infrastructure to manipulate individual bones/joints Auto-normalization was removed because it silently mutated model transforms on load, making it impossible to distinguish between the original model pose and user edits. This broke gizmo reset (which needs to know the "clean" state) and would corrupt round-trip transform persistence. ## Screenshots (if applicable) https://github.com/user-attachments/assets/621ea559-d7c8-4c5a-a727-98e6a4130b66 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11274-gizmo-controls-3436d73d365081c38357c2d58e49c558) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
da91bdc957 |
fix: persist middle-click reroute node setting across reloads (#11362)
*PR Created by the Glary-Bot Agent* --- ## Summary - Remove hardcoded `LiteGraph.middle_click_slot_add_default_node = true` from `slotDefaults` extension `init()` that unconditionally overrode the user's persisted preference on every page load - Add E2E regression test verifying both the setting store value and the LiteGraph runtime flag persist through page reload ## Root Cause The `Comfy.SlotDefaults` extension's `init()` method (in `slotDefaults.ts`) contained a hardcoded `LiteGraph.middle_click_slot_add_default_node = true` from the original JS→TS conversion (July 2024). When `Comfy.Node.MiddleClickRerouteNode` was later made configurable in v1.3.42, this line was never removed. Since extension `init()` runs **after** `useLitegraphSettings()` syncs the stored value, the hardcoded assignment overwrote the user's preference on every reload. ## Changes | File | Change | |------|--------| | `src/extensions/core/slotDefaults.ts` | Remove line 21 (`LiteGraph.middle_click_slot_add_default_node = true`) | | `browser_tests/tests/dialogs/settingsDialog.spec.ts` | Add reload persistence test asserting both store value and LiteGraph global | The setting default (`true`) is already properly managed by `coreSettings.ts` and reactively synced via `useLitegraphSettings.ts`, so removing the hardcoded line preserves existing default behavior while allowing user overrides to persist. ## Screenshots    ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11362-fix-persist-middle-click-reroute-node-setting-across-reloads-3466d73d365081ef8692dbd0619c8594) by [Unito](https://www.unito.io) Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
Terry Jia
|
54f3127658 |
test: regenerate screenshot expectations (#11360)
## Summary regenerate screenshot expectations ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11360-test-regenerate-screenshot-expectations-3466d73d365081878addd53a266a31b7) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <github-actions@github.com> |
||
|
Johnpaul Chiwetelu
|
beaa269a63 |
feat: polish settings dialog layout and keybinding display (#11241)
Polish keybinding display. Based on #11212 with adjustments: left-aligned content (no centering), key uppercase moved to UI layer. - Reduce settings content font size to 14px - Increase spacing between setting sections with cleaner dividers - Consistent min-height for form items (toggle, slider, dropdown) - Capitalize keybinding badges via CSS `uppercase` instead of mutating data model - Remove '+' separator between keybinding badges - Unbold keybinding badges with fixed min-width Supersedes #11212 ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11241-feat-polish-settings-dialog-layout-and-keybinding-display-3426d73d3650812a97e4d941a76a4fe9) by [Unito](https://www.unito.io) --------- Co-authored-by: Alex <alex@Mac.localdomain> Co-authored-by: github-actions <github-actions@github.com> |
||
|
Kelly Yang
|
cf98013c18 |
test: expand Image Crop E2E and fix loading overlay deadlock (#11193)
## Summary
Expands Playwright coverage for the **ImageCropV2** widget (Levels 1–3
from the image crop E2E plan), fixes **loading / image mount** behavior
when `imageUrl` changes, adds **stable resize-handle selectors**, and
adds a **small Vitest** for URL→loading transitions.
## Changes
- [x] **Level 1 (E2E)** — Empty state: assert resize handle hidden;
screenshot baseline `image-crop-empty-state.png`; pointer drag on empty
state does not change widget bounds.
- [x] **Level 1 (E2E)** — After run: assert **8 visible** resize handles
with `data-testid` + `filter({ visible: true })`; broken `img.src`
returns to empty state (`crop-empty-state`, no overlay).
- [x] **Level 1 (E2E)** — **Slow `/api/view`** route (delay only
`example.png`) to assert **“Loading…”** then hidden after image loads;
comment clarifies delay is in the route handler, not
`page.waitForTimeout`.
- [x] **Level 2 (E2E)** — Drag clamps to **right/bottom** and
**top-left** image bounds via `setCropBounds` + `expect.poll` on natural
bounds.
- [x] **Level 3 (E2E)** — Free resize: right / left / bottom / top
edges; SE and NW corners; `MIN_CROP_SIZE` (16px); right-edge boundary
clamp; **8 handles** screenshot `image-crop-eight-handles.png`; SE/NW
screenshots (`image-crop-resize-se.png`, `image-crop-resize-nw.png`).
- [x] **E2E helpers** — Shared `getCropValue`, `setCropBounds`,
`dragOnLocator`, `POINTER_OPTS`; drag regression uses **`expect.poll`**
instead of `toPass` where appropriate.
- [x] **`WidgetImageCrop.vue`** — When `imageUrl` is set, **always
render `<img>`**; show loading as an **absolute overlay** (fixes
deadlock where `isLoading` blocked `<img>` so `@load` never ran); add
**`data-testid="crop-resize-{direction}"`** on resize handles.
- [x] **`useImageCrop.ts`** — Watch `imageUrl` and drive `isLoading`;
extract **`imageCropLoadingAfterUrlChange`** (`boolean | null`) for
clear semantics and tests.
- [x] **`useImageCrop.test.ts`** — Vitest coverage for
`imageCropLoadingAfterUrlChange` (null URL, URL change, first URL,
unchanged URL).
## Screenshot / CI notes
- [ ] **Linux screenshot expectations** for new/updated
`toHaveScreenshot(...)` names must be produced on **CI (Linux)** — add
the **`New Browser Test Expectation`** label (or equivalent workflow);
**do not** commit local **Darwin** golden files.
- [x] Existing Linux baselines under `imageCrop.spec.ts-snapshots/` for
prior tests are unchanged where applicable; new baselines are expected
from CI after merge workflow.
## Files
- [x] `browser_tests/tests/vueNodes/widgets/imageCrop.spec.ts`
- [x] `src/components/imagecrop/WidgetImageCrop.vue`
- [x] `src/composables/useImageCrop.ts`
- [x] `src/composables/useImageCrop.test.ts` (new)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Touches interactive crop UI rendering and `isLoading` state
transitions, which can affect user-visible behavior and input handling;
changes are mitigated by extensive new E2E and unit tests.
>
> **Overview**
> Improves the `WidgetImageCrop` loading behavior by always rendering
the preview `<img>` when `imageUrl` is set and showing “Loading…” as an
absolute overlay, preventing a deadlock where `isLoading` could block
the `@load` event. Adds stable `data-testid="crop-resize-{direction}"`
selectors for resize handles and hardens pointer-capture handling in
`useImageCrop`.
>
> Greatly expands automated coverage: the Playwright spec now tests
empty-state rendering/screenshot, drag/resize interactions (edge/corner,
min size, and clamping to image bounds), aspect-ratio lock handle
visibility, slow `/api/view` loading overlay behavior, and broken image
fetch recovery. Adds a new Vitest suite for `useImageCrop` (including
`imageCropLoadingAfterUrlChange`) to unit-test URL→loading transitions
and core drag/resize/aspect-ratio logic.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
AustinMroz
|
e28c1e7e43 |
Show multitype slices of shared color (#11250)
A tiny update requested by the backend team. Previously, multitype slot indicators would have inputs that resolve to the same connection color display be combined into a single slice. For example, both `INT` and `FLOAT` have the same color, so an `INT,FLOAT` slot displays as a solid color instead of 2 semi-circles. This was a conscientious decision to improve readability on slots that allow many types, but meant that the more common cases (like `INT,FLOAT`) would have no indicator at all. Since priority is given to types based on order of listing, node authors can still control which types are elided on a slot accepting many types. <img width="430" height="320" alt="image" src="https://github.com/user-attachments/assets/1fc7fb1c-a634-487c-bc03-711637aeef13" /> - I do not believe there are any core nodes affected by this change. - The vue implementation of merging slot colors never functioned properly, but is still removed. - Vue was bugged to incorrectly pass slot types for widgets. This is also fixed. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11250-Show-multitype-slices-of-shared-color-3436d73d365081b6b484ea74423435a1) by [Unito](https://www.unito.io) |
||
|
AustinMroz
|
3fd3c565ae |
Fix dropdown chevron color (#11335)
Updates the the color of the chevron on dropdown widgets to only have the disabled color when the widget is disabled. | Before | After | | ------ | ----- | | <img width="360" alt="before" src="https://github.com/user-attachments/assets/25d35e78-9147-4397-be19-df9d6f87ac72" /> | <img width="360" alt="after" src="https://github.com/user-attachments/assets/3bf3640d-fa14-42cb-afb4-7109eb878d1a" />| ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11335-Fix-dropdown-chevron-color-3456d73d3650819e99c7d15173f11319) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <github-actions@github.com> |
||
|
Kelly Yang
|
29d6263fb9 |
test: add Preview3D execution flow E2E tests (#11014)
## Summary
Adds Playwright coverage for `Preview3D execution` and persistence :
real queue execution against a `Load3D → Preview3D` workflow, plus `save
/ full reload / reopen` from the sidebar.
## What these tests do
**Fixture** (every test)
Turns on Vue Nodes, uses the sidebar for workflows, loads a Load3D →
Preview3D workflow, waits for nodes, then clears saved workflows after
the test so runs stay isolated.
**Test 1 — execution updates Preview3D**
Uploads `cube.obj`(the existing test file in the merged version) to
Load3D, runs `Queue Prompt`, then checks that Preview3D’s model_file and
Last Time Model File match and the canvas has non-zero size. No 3D
screenshots (GPU flakiness).
**Test 2 — persistence after reload**
Same upload + queue, then saves the workflow, reloads the page,
re-applies the same UI settings, opens the saved workflow, and checks
the same model path and camera state (with a small numeric tolerance).
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Adds new slow, WebGL-dependent E2E tests and fixtures, which can
increase CI runtime and introduce flakiness due to timing/graphics
variability, but does not change production logic.
>
> **Overview**
> Adds a new `Load3D → Preview3D` workflow asset and a dedicated
Playwright fixture (`Preview3DPipelineFixture`) to drive real queue
execution, upload a 3D model, and interact with the 3D canvases (orbit
drags) while asserting `model_file`/`Last Time Model File` and camera
state via node properties.
>
> Introduces camera-state comparison helpers with explicit numeric
tolerances, and adds a new `preview3dExecution.spec.ts` suite that
validates (1) Preview3D updates from execution output and (2) model +
camera persistence across save, full page reload, and reopening the
workflow from the sidebar.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
jaeone94
|
a1e6fb36d2 |
refactor: harden ChangeTracker lifecycle with self-defending API (#10816)
## Summary Harden the `ChangeTracker` lifecycle to eliminate the class of bugs where an inactive workflow's tracker silently captures the wrong graph state. Renames `checkState()` to `captureCanvasState()` with a self-defending assertion, introduces `deactivate()` and `prepareForSave()` lifecycle methods, and closes a latent undo-history corruption bug discovered during code review. ## Background ComfyUI supports multiple workflows open as tabs, but only one canvas (`app.rootGraph`) exists at a time. When the user switches tabs, the old workflow's graph is unloaded and the new one is loaded into this shared canvas. The old `checkState()` method serialized `app.rootGraph` into `activeState` to track changes for undo/redo. It had no awareness of *which* workflow it belonged to -- if called on an inactive tab's tracker, it would capture the active tab's graph data and silently overwrite the inactive workflow's state. This caused permanent data loss (fixed in PR #10745 with caller-side `isActive` guards). The caller-side guards were fragile: every new call site had to remember to add the guard, and forgetting would reintroduce the same silent data corruption. Additionally, `beforeLoadNewGraph` only called `store()` (viewport/outputs) without `checkState()`, meaning canvas state could be stale if a tab switch happened without a preceding mouseup event. ### Before (fragile) ``` saveWorkflow(workflow): if (isActive(workflow)) <-- caller must remember this guard workflow.changeTracker.checkState() <-- name implies "read", actually writes ... beforeLoadNewGraph(): activeWorkflow.changeTracker.store() <-- only saves viewport, NOT graph state ``` ### After (self-defending) ``` saveWorkflow(workflow): workflow.changeTracker.prepareForSave() <-- handles active/inactive internally ... beforeLoadNewGraph(): activeWorkflow.changeTracker.deactivate() <-- captures graph + viewport together ``` ## Changes - Rename `checkState` to `captureCanvasState` with active-tracker assertion - Add `deactivate()` and `prepareForSave()` lifecycle methods - Fix undo-history corruption: `captureCanvasState()` guarded by `_restoringState` - Fix viewport regression during undo: `deactivate()` skips `captureCanvasState()` during undo/redo but always calls `store()` to preserve viewport (regression from PR #10247) - Log inactive tracker warnings unconditionally at warn level (not DEV-only) - Deprecated `checkState()` wrapper for extension compatibility - Rename `checkState` to `captureCanvasState` in `useWidgetSelectActions` composable - Add `appModeStore.ts` to manual call sites documentation - Add `checkState()` deprecation note to architecture docs - Add 16 unit tests covering all guard conditions, lifecycle methods, and undo behavior - Add E2E test: "Undo preserves viewport offset" ## New ChangeTracker Public API | Method | Caller | Purpose | |--------|--------|---------| | `captureCanvasState()` | Event handlers, UI interactions | Snapshots canvas into activeState, pushes undo. Asserts active tracker. | | `deactivate()` | `beforeLoadNewGraph` only | `captureCanvasState()` (skipped during undo/redo) + `store()`. Freezes state for tab switch. | | `prepareForSave()` | Save paths only | Active: `captureCanvasState()`. Inactive: no-op. | | `checkState()` | **Deprecated** -- extensions only | Wrapper that delegates to `captureCanvasState()` with deprecation warning. | | `store()` | Internal to `deactivate()` | Saves viewport, outputs, subgraph navigation. | | `restore()` | `afterLoadNewGraph` | Restores viewport, outputs, subgraph navigation. | | `reset()` | `afterLoadNewGraph`, save | Resets initial state (marks as "clean"). | ## Test plan - [x] Unit tests: 16 tests covering all guard conditions, state capture, undo queue behavior - [x] E2E test: "Undo preserves viewport offset" verifies no viewport drift on undo - [x] E2E test: "Prevents captureCanvasState from corrupting workflow state during tab switch" - [x] Existing E2E: "Closing an inactive tab with save preserves its own content" - [ ] Manual: rapidly switch tabs during undo/redo, verify no viewport drift - [ ] Manual: verify extensions calling `checkState()` see deprecation warning in console |
||
|
jaeone94
|
394e36984f |
fix: re-sync collapsed node slot positions after subgraph fitView (#11240)
## Summary Fix collapsed node connection links rendering at wrong positions when entering a subgraph for the first time. `fitView()` (added in #10995) changes canvas scale/offset, invalidating cached slot positions for collapsed nodes. ## Changes - **What**: Schedule `requestSlotLayoutSyncForAllNodes()` on the next frame after `fitView()` in `restoreViewport()` so collapsed node slot positions are re-measured against the updated transform. Inner RAF guarded against mid-frame graph changes. - **Test coverage**: - Unit tests in `subgraphNavigationStore.viewport.test.ts` verify the RAF chain calls `requestSlotLayoutSyncForAllNodes` after `fitView`, and skip the re-sync when the active graph changes between frames. - E2E screenshot test (`@screenshot` tag) validates correct link rendering on first subgraph entry using a new fixture with a pre-collapsed inner node. ## Review Focus The nested `requestAnimationFrame` is intentional: the outer RAF runs `fitView()`, which updates `ds.scale`/`ds.offset` and triggers a CSS transform update on `TransformPane`. The inner RAF ensures the DOM has reflowed with the new transform before `requestSlotLayoutSyncForAllNodes()` measures `getBoundingClientRect()` on slot elements. --------- Co-authored-by: github-actions <github-actions@github.com> |
||
|
Dante
|
19fff29204 |
test: backfill e2e coverage gaps for toolkit widgets, minimap, mask editor, painter (#11183)
## Summary Backfills missing e2e test coverage identified in the [FixIt Burndown](https://www.notion.so/comfy-org/FixIt-Burndown-32e6d73d365080609a81cdc9bc884460) audit. Adds 39 new behavioral tests across 5 spec files with zero test-code overlap. ## Changes - **What**: New e2e specs for Image Crop (6 tests) and Curve Widget (6 tests). Deepened coverage for Minimap (+6), Mask Editor (+10), Painter (+11). - **New fixtures**: `curve_widget.json`, updated `image_crop_widget.json` ## Test Inventory | Spec | New tests | Coverage area | |---|---|---| | `imageCrop.spec.ts` | 6 | Empty state, bounding box inputs, ratio selector/presets, lock toggle, programmatic value update | | `curveWidget.spec.ts` | 6 | SVG render, click-to-add point, drag-to-reshape, Ctrl+click remove, interpolation mode switch, min-2 guard | | `minimap.spec.ts` | +6 | Click-to-pan, drag-to-pan, zoom viewport shrink, node count changes, workflow reload, pan state reflection | | `maskEditor.spec.ts` | +10 | Brush drawing, undo/redo, clear, cancel, invert, Ctrl+Z, tool panel/switching, brush settings, save with mock, eraser | | `painter.spec.ts` | +11 | Clear, eraser, control visibility toggle, brush size slider, stroke width comparison, canvas dimensions, background color, multi-stroke accumulate, color picker, opacity, partial erase | ## Review Focus - Mask editor tests use `.maskEditor_toolPanelContainer` class selectors — may need test-id hardening later - Painter slider interaction tests could be flaky if slider layout changes - All canvas pixel-count assertions use `expect.poll()` with timeouts for reliability ## Test plan - [ ] CI passes all new/modified specs - [ ] No duplicate coverage with existing tests (verified via grep before writing) - [ ] No `waitForTimeout` usage (confirmed) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11183-test-backfill-e2e-coverage-gaps-for-toolkit-widgets-minimap-mask-editor-painter-3416d73d3650819ca33edd1f27b9651a) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Johnpaul Chiwetelu
|
b3b895a2a9 |
refactor(test): use canvasOps.clickEmptySpace in copyPaste spec (#10991)
## Summary
Replace two hardcoded blank-canvas click positions in
`copyPaste.spec.ts` with the existing
`comfyPage.canvasOps.clickEmptySpace()` helper.
## Changes
- **What**: Both `{ x: 50, y: 500 }` click literals in the `Copy paste
node, image paste onto LoadImage, image paste on empty canvas` test now
use `canvasOps.clickEmptySpace()` (which wraps
`DefaultGraphPositions.emptySpaceClick = { x: 35, y: 31 }`). Redundant
`await nextFrame()` calls dropped — the helper already awaits a frame
internally.
## Review Focus
Draft PR — need CI to confirm `(35, 31)` is a valid blank-canvas click
for the `load_image_with_ksampler` workflow used by this test. The
workflow places `LoadImage` at `[50, 50]` and `KSampler` at `[500, 50]`,
so `(35, 31)` should be clear of both. Locally the test was already
failing on `main` (pre-existing, unrelated), so CI is the source of
truth here. If CI fails, the fallback is to add a dedicated named
constant `emptyCanvasClick: { x: 50, y: 500 }` to
`DefaultGraphPositions` as originally proposed in the issue.
Fixes #10330
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10991-refactor-test-use-canvasOps-clickEmptySpace-in-copyPaste-spec-33d6d73d3650817aa3ccea44cb48c0ae)
by [Unito](https://www.unito.io)
|
||
|
Dante
|
e5c81488e4 |
fix: include focusMode in splitter refresh key to prevent panel resize (#11295)
## Summary When the properties panel is open, toggling focus mode on then off causes the panel to resize unexpectedly. The root cause is that `splitterRefreshKey` in `LiteGraphCanvasSplitterOverlay.vue` does not include `focusMode`, so the PrimeVue Splitter component instance is reused across focus mode transitions and restores stale panel sizes from localStorage. Fix: add `focusMode` to `splitterRefreshKey` so the Splitter is recreated when focus mode toggles. ## Red-Green Verification | Commit | CI Status | Purpose | |--------|-----------|---------| | `test: add failing test for focus mode toggle resizing properties panel` | 🔴 Red | Proves the test catches the bug | | `fix: include focusMode in splitter refresh key to prevent panel resize` | 🟢 Green | Proves the fix resolves the bug | ## demo ### AS IS https://github.com/user-attachments/assets/95f6a9e3-e4c7-4aba-8e17-0eee11f70491 ### TO BE https://github.com/user-attachments/assets/595eafcd-6a80-443d-a6f3-bb7605ed0758 ## Test Plan - [ ] CI red on test-only commit - [ ] CI green on fix commit - [ ] E2E regression test added in `browser_tests/tests/focusMode.spec.ts` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11295-fix-include-focusMode-in-splitter-refresh-key-to-prevent-panel-resize-3446d73d365081b7bc3ac65338e17a8f) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
ecb6fbe8fb |
test: Add waitForWorkflowIdle & remove redundant nextFrame (#11264)
## Summary More cleanup and reliability ## Changes - **What**: - Add wait for idle - Remove redundant nextFrames ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11264-test-Add-waitForWorkflowIdle-remove-redundant-nextFrame-3436d73d3650812c837ac7503ce0947b) by [Unito](https://www.unito.io) Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Alexander Brown
|
52ccd9ed1a |
refactor: internalize nextFrame() into fixture/helper methods (#11166)
## Summary
Internalize `nextFrame()` calls into fixture/helper methods so spec
authors don't need to remember to call it after common operations.
`nextFrame()` waits for one `requestAnimationFrame` (~16ms) — an extra
call is always safe, making this a low-risk refactor.
## Changes
### Phase 1: `SettingsHelper.setSetting()`
`setSetting()` now calls `nextFrame()` internally. Removed 15 redundant
calls across 7 files.
### Phase 2: `CommandHelper.executeCommand()`
`executeCommand()` now calls `nextFrame()` internally. Removed 15
redundant calls across 7 files, including the now-redundant call in
`AppModeHelper.toggleAppMode()`.
### Phase 3: `WorkflowHelper.loadGraphData()`
New helper wraps `page.evaluate(loadGraphData)` + `nextFrame()`.
Migrated `SubgraphHelper.serializeAndReload()` and `groupNode.spec.ts`.
### Phase 4: `NodeReference` cleanup
Removed redundant `nextFrame()` from `copy()`, `convertToGroupNode()`,
`resizeNode()`, `dragTextEncodeNode2()`, and
`convertDefaultKSamplerToSubgraph()`. Removed 6 spec-level calls after
`node.click('title')`.
### Phase 5: `KeyboardHelper.press()` and `delete()`
New convenience methods that press a key and wait one frame. Converted
40 `canvas.press(key)` + `nextFrame()` pairs across 13 spec files.
### Phase 6: `ComfyPage.expectScreenshot()`
New helper combines `nextFrame()` + `toHaveScreenshot()`. Converted 45
pairs across 12 spec files.
## Total impact
- **~130 redundant `nextFrame()` calls eliminated** across ~35
spec/helper files
- **3 new helper methods** added (`loadGraphData`, `press`/`delete`,
`expectScreenshot`)
- **2 existing methods** enhanced (`setSetting`, `executeCommand`)
## What was NOT changed
- `performance.spec.ts` frame-counting loops (intentional)
- `ComfyMouse.ts` / `CanvasHelper.ts` (already internalized)
- `SubgraphHelper.packAllInteriorNodes()` (deliberate orchestration)
- Builder helpers (already internalized)
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11166-refactor-internalize-nextFrame-into-fixture-helper-methods-33f6d73d3650817bb5f6fb46e396085e)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Amp <amp@ampcode.com>
|
||
|
Kelly Yang
|
92ad6fc798 |
test: address review nits for image compare E2E (#11260)
## Summary
A follow-up PR of #11196.
| # | Nit | Action | Reason |
| :--- | :--- | :--- | :--- |
| 1 | Replace `page.on('pageerror')` with request-wait | **Left as-is**
| The `pageErrors` array is an accumulator checked at the end via
`expect(pageErrors).toHaveLength(0)` – the goal is to assert that broken
image URLs don't surface as uncaught JS exceptions during the test run.
A request-wait can't substitute for that behavioral assertion, so the
listener pattern is intentional here. |
| 2 | Move helpers to a `vueNodes.getImageCompareHelper()` subclass |
**Left as-is** | Helpers such as `setImageCompareValue` and
`moveToPercentage` are only used in this file, making local
encapsulation enough. Extracting them to a page object would increase
the file/interface surface area and violate YAGNI; additionally,
`AGENTS.md` clearly states to "minimize the exported values of each
module. |
| 3 | Use `TestIds` enum for test ID strings | **Fixed** – added
`imageCompare` section to `TestIds` in `selectors.ts`; replaced all 8
inline string IDs in `imageCompare.spec.ts` with
`TestIds.imageCompare.*` references | The project already has a
`TestIds` convention for centralizing test IDs. Inline strings create
drift risk between the Vue component and the test file. |
| 4 | Move `expect.poll` bounding box check to helper/page object |
**Left as-is** | This logic already lives inside `moveToPercentage`,
which is a local helper. Moving it further to a page object is the same
refactor as #2 above. |
| 5 | Remove `// ---` style section header comments | **Fixed** –
removed all 8 divider blocks from `imageCompare.spec.ts` | Consistent
with project guidelines and your explicit preference. Test names already
describe what each block does. |
| 6 | Name magic numbers `400` and `350` | **Fixed** – introduced
`minWidth = 400` and `minHeight = 350` constants in the test |
Descriptive names make the constraint self-documenting and easier to
update if the workflow asset changes. |
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk: changes are limited to Playwright E2E test code and shared
selector constants, with no production logic impacted.
>
> **Overview**
> **E2E Image Compare tests now use centralized selectors.** Adds an
`imageCompare` section to `TestIds` and updates `imageCompare.spec.ts`
to reference `TestIds.imageCompare.*` instead of inline `data-testid`
strings.
>
> Cleans up the spec by removing divider comments and naming the minimum
size magic numbers (`minWidth`, `minHeight`) used in the node sizing
assertion.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
pythongosssss
|
06686a1f50 |
test: App mode - additional app mode coverage (#11194)
## Summary Adds additional test coverage for empty state/welcome screen/connect outputs/vue nodes auto switch ## Changes - **What**: - add tests ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11194-test-App-mode-additional-app-mode-coverage-3416d73d365081ca91d0ed61de19f840) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
693b8383d6 |
fix: missing-asset correctness follow-ups from #10856 (#11233)
Follow-up to #10856. Four correctness issues and their regression tests. ## Bugs fixed ### 1. ErrorOverlay model count reflected node selection `useErrorGroups` exposed `filteredMissingModelGroups` under the public name `missingModelGroups`. `ErrorOverlay.vue` read that alias to compute its model count label, so selecting a node shrank the overlay total. The overlay must always show the whole workflow's errors. Exposed both shapes explicitly: `missingModelGroups` / `missingMediaGroups` (unfiltered totals) and `filteredMissingModelGroups` / `filteredMissingMediaGroups` (selection-scoped). `TabErrors.vue` destructures the filtered variant with an alias. Before https://github.com/user-attachments/assets/eb848c5f-d092-4a4f-b86f-d22bb4408003 After https://github.com/user-attachments/assets/75e67819-c9f2-45ec-9241-74023eca6120 ### 2. Bypass → un-bypass dropped url/hash metadata Realtime `scanNodeModelCandidates` only reads widget values, so un-bypass produced a fresh candidate without the url that `enrichWithEmbeddedMetadata` had previously attached from `graphData.models`. `MissingModelRow`'s download/copy-url buttons disappeared after a bypass/un-bypass cycle. Added `enrichCandidateFromNodeProperties` that copies `url`/`hash`/`directory` from the node's own `properties.models` — which persists across mode toggles — into each scanned candidate. Applied to every call site of the per-node scan. A later fix in the same branch also enforces directory agreement to prevent a same-name / different-directory collision from stamping the wrong metadata. Before https://github.com/user-attachments/assets/39039d83-4d55-41a9-9d01-dec40843741b After https://github.com/user-attachments/assets/047a603b-fb52-4320-886d-dfeed457d833 ### 3. Initial full scan surfaced interior errors of a muted/bypassed subgraph container `scanAllModelCandidates`, `scanAllMediaCandidates`, and the JSON-based missing-node scan only check each node's own mode. Interior nodes whose parent container was bypassed passed the filter. Added `isAncestorPathActive(rootGraph, executionId)` to `graphTraversalUtil` and post-filter the three pipelines in `app.ts` after the live rootGraph is configured. The filter uses the execution-ID path (`"65:63"` → check node 65's mode) so it handles both live-scan-produced and JSON-enrichment-produced candidates. Before https://github.com/user-attachments/assets/3032d46b-81cd-420e-ab8e-f58392267602 After https://github.com/user-attachments/assets/02a01931-951d-4a48-986c-06424044fbf8 ### 4. Bypassed subgraph entry re-surfaced interior errors `useGraphNodeManager` replays `graph.onNodeAdded` for each existing interior node when the Vue node manager initializes on subgraph entry. That chain reached `scanSingleNodeErrors` via `installErrorClearingHooks`' `onNodeAdded` override. Each interior node's own mode was active, so the caller guards passed and the scan re-introduced the error that the initial pipeline had correctly suppressed. Added an ancestor-activity gate at the top of `scanSingleNodeErrors`, the single entry point shared by paste, un-bypass, subgraph entry, and subgraph container activation. A later commit also hardens this guard against detached nodes (null execution ID → skip) and applies the same ancestor check to `isCandidateStillActive` in the realtime verification callback. Before https://github.com/user-attachments/assets/fe44862d-f1d6-41ed-982d-614a7e83d441 After https://github.com/user-attachments/assets/497a76ce-3caa-479f-9024-4cd0f7bd20a4 ## Tests - 6 unit tests for `isAncestorPathActive` (root, active, immediate-bypass, deep-nested mute, unresolvable ancestor, null rootGraph) - 4 unit tests for `enrichCandidateFromNodeProperties` (enrichment, no-overwrite, name mismatch, directory mismatch) - 1 unit test for `scanSingleNodeErrors` ancestor guard (subgraph entry replaying onNodeAdded) - 2 unit tests for `useErrorGroups` dual export + ErrorOverlay contract - 4 E2E tests: - ErrorOverlay model count stays constant when a node is selected (new fixture `missing_models_distinct.json`) - Bypass/un-bypass cycle preserves Copy URL button (uses `missing_models_from_node_properties`) - Loading a workflow with bypassed subgraph suppresses interior missing model error (new fixture `missing_models_in_bypassed_subgraph.json`) - Entering a bypassed subgraph does not resurface interior missing model error (shares the above fixture) `pnpm typecheck`, `pnpm lint`, 206 related unit tests passing. ## Follow-up Several items raised by code review are deferred as pre-existing tech debt or scope-avoided refactors. Tracked via comments on #11215 and #11216. --- Follows up on #10856. |
||
|
Christian Byrne
|
033b3dad3a |
feat: add Slack notification workflow for coverage improvements (#10977)
## Summary
Adds a GitHub Actions workflow + TypeScript script that posts to Slack
when a merged PR improves unit or E2E test coverage.
## Changes
- **What**: New `coverage-slack-notify.yaml` workflow triggered on push
to main. Compares current coverage against previous baselines, generates
Slack Block Kit payload with progress bars and milestone celebrations,
posts to `#p-frontend-automated-testing`.
- **Script**: `scripts/coverage-slack-notify.ts` — parses lcov files,
computes deltas, detects milestone crossings (every 5%), builds Slack
payload. Pure functions exported for testability.
- **Tests**: 26 unit tests in `scripts/coverage-slack-notify.test.ts`
covering all pure functions including edge cases (malformed lcov, exact
boundaries, zero coverage).
### Security hardening
- All `${{ }}` expressions moved from `run:` blocks to `env:` variables
- `SLACK_BOT_TOKEN` passed via env var, not inline
- Unique heredoc delimiter (timestamp-based) prevents payload injection
- `parseInt` fallback (`|| 0`) guards against malformed lcov
- PR regex anchored to first line of commit message
### Robustness
- `continue-on-error: true` on Slack post step (outage does not fail the
job)
- Baseline save guarded by `steps.unit-tests.outcome == success`
(prevents corrupt baselines on test failure)
- Channel ID commented for maintainability
- Top-level `text` field added for Slack mobile push notifications
- Author linked to GitHub profile instead of bare `@username`
## Review Focus
- Workflow step ordering and conditional logic
- Security of expression handling and secret management
- Slack payload structure and Block Kit formatting
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10977-feat-add-Slack-notification-workflow-for-coverage-improvements-33d6d73d3650819c8950f483c83f297c)
by [Unito](https://www.unito.io)
---------
Co-authored-by: GitHub Action <action@github.com>
|
||
|
Kelly Yang
|
66e8d570e7 |
test: expand Image Compare E2E and stabilize widget selectors (#11196)
## Summary
This PR extends **Image Compare** browser tests, adds **stable
`data-testid` hooks** on `WidgetImageCompare.vue`, and aligns slider
interactions with the **same viewport element** used by
`useMouseInElement`, with **layout polling** to reduce flakes.
## What this PR does
- [x] Adds **`data-testid="image-compare-viewport"`** on the compare
area root (the `containerRef` div) so E2E targets the real slider hit
region instead of a long Tailwind class chain or the `<img>` box alone.
- [x] Adds **`data-testid="image-compare-empty"`** on the no-images
branch so the empty state can be asserted **without hard-coded English**
UI text.
- [x] Adds a **smoke** test that the widget **shows both images and the
drag handle** after value injection, with **`waitForImagesLoaded`** (no
extra full-node screenshot to avoid duplicating the default-50 golden).
- [x] Extends slider coverage to **clamp at both edges** (**0%** and
**~100%**) using the viewport locator and **`expect.poll`** until
**`boundingBox()`** is valid before reading coordinates.
- [x] Updates **`moveToPercentage`** to **`expect.poll`** until the
target locator has a **non-empty layout box** before moving the mouse.
- [x] Routes **hover**, **preserve position**, and **25% / 75%
screenshot** mouse moves through **`image-compare-viewport`**
(consistent with `containerRef`).
- [x] Adds an E2E assertion that the **workflow ImageCompare node** size
is **at least 400×350** (matches the widget workflow asset).
- [x] Hardens the **broken image** case: **`page.on('pageerror')`** /
**`page.off`** in **`finally`**, **`http://127.0.0.1:1/...`** URLs for
fast failure, **`expect.soft`** on key UI invariants, and a hard
assertion that **no page errors** were recorded.
- [x] Extends the **large batch (20×20)** test to **page to the last
index** and assert **counters `20 / 20`**, **previous enabled**, and
**next disabled** on both sides.
- [x] Renames the clamp test title to use an **ASCII hyphen** (`0-100%`)
for easier grepping.
## Out of scope (unchanged in this PR)
- [ ] Replacing **`setImageCompareValue`**’s **`page.evaluate`** setup
with a full UI-driven path (would be a larger follow-up).
## Suggested title
`test: expand Image Compare E2E and stabilize widget selectors`
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Primarily test and selector-hook changes; low production risk, with
only minor DOM attribute additions that could affect external test
tooling if relied upon.
>
> **Overview**
> Improves Image Compare Playwright coverage and reduces flakiness by
driving slider interactions through a new stable
`data-testid="image-compare-viewport"` hook and polling for a valid
layout box before mouse moves.
>
> Updates assertions to avoid localized text (new
`data-testid="image-compare-empty"`), adds smoke coverage that
images/handle render after value injection, validates slider clamping at
both 0% and ~100%, and extends screenshot tests to use the same viewport
target.
>
> Hardens edge-case tests by ensuring broken image loads don’t raise
uncaught `pageerror`s, adds a minimum node size assertion, and extends
large-batch navigation checks through the final index and
disabled/enabled nav states.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Dante
|
634d57809b |
test: add E2E tests for bottom panel core behaviors (#10814)
## Summary - Add `browser_tests/tests/bottomPanel.spec.ts` with tests for behaviors not covered by existing `bottomPanelLogs` and `bottomPanelShortcuts` specs - Tests cover: close button (X), tab persistence on re-open, resize gutter visibility and drag, canvas interaction when panel is closed, cross-panel switching (terminal <-> shortcuts), and registered tab enumeration - Extend `BottomPanel` fixture with `closeButton` and `resizeGutter` locators ## Test plan - [ ] `pnpm test:browser:local` passes all new tests in `bottomPanel.spec.ts` - [ ] Existing `bottomPanelLogs.spec.ts` and `bottomPanelShortcuts.spec.ts` are unaffected - [ ] `pnpm typecheck:browser` passes - [ ] `pnpm lint` passes ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10814-test-add-E2E-tests-for-bottom-panel-core-behaviors-3366d73d365081ea9b90c643897845fa) by [Unito](https://www.unito.io) --------- Co-authored-by: dante <dante@danteui-MacStudio.local> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Dante
|
5807e03c74 |
test: expand E2E coverage for toolbox actions (#10968)
## Summary - Adds `selectionToolboxMoreActions.spec.ts` with E2E coverage for previously untested toolbox actions - Covers: pin/unpin, minimize/expand, adjust size, copy, duplicate, refresh button, align (top/left), distribute (horizontal/vertical), alignment options hidden for single selection, multi-node bypass toggle - Part of the FixIt Burndown test coverage initiative (toolbox actions) ## Test plan - [ ] All new tests pass in CI - [ ] No regressions in existing selectionToolbox tests ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10968-test-expand-E2E-coverage-for-toolbox-actions-33c6d73d3650811286cefdd0eb4f5242) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Dante
|
6847c7ba2d |
fix: store promoted widget values per SubgraphNode instance (#10849)
## Summary - Multiple SubgraphNode instances of the same blueprint share inner nodes, causing promoted widget values to collide — the last configured instance overwrites all previous values - Add per-instance value storage (`_instanceWidgetValues`) on SubgraphNode so each instance preserves its own promoted widget values independently - Restore `widgets_values` from serialized data into this per-instance map after promoted views are created during configure - Fixes #10146 ## Root Cause When loading a workflow with multiple SubgraphNode instances of the same blueprint: 1. `LGraph.configure()` creates ONE shared Subgraph per blueprint (line 2625) 2. Each SubgraphNode instance calls `configure(instanceData)` sequentially 3. `PromotedWidgetView.value` setter writes to the **shared inner node's widget** (`promotedWidgetView.ts:199`) 4. The last instance's `configure()` overwrites all previous instances' values **Regression**: Introduced by PR #8594 (WidgetValueStore, v1.41.3) which centralized widget state without per-instance scoping for shared blueprints. ## Fix - **SubgraphNode**: Add `_instanceWidgetValues` Map and `_pendingWidgetsValues` for configure-time restoration - **PromotedWidgetView getter**: Check instance map first before falling back to widget store / inner node - **PromotedWidgetView setter**: Write to instance map to avoid shared inner node mutation - **_internalConfigureAfterSlots**: Apply serialized `widgets_values` to per-instance map after promoted views are created ## Red-Green Verification | Commit | CI Status | Purpose | |--------|-----------|---------| | `test: add failing tests for multi-instance subgraph widget value collision` | 🔴 Red | Proves widget values collide across instances | | `fix: store promoted widget values per SubgraphNode instance` | 🟢 Green | Per-instance storage prevents collision | ## Test Plan - [x] CI red on test-only commit - [x] CI green on fix commit - [x] Unit test: `preserves promoted widget values after configure with different widgets_values` - [x] All 253 existing subgraph tests pass - [ ] Manual: load workflow from issue image → verify 3 subgraph instances produce different results ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10849-fix-store-promoted-widget-values-per-SubgraphNode-instance-3386d73d3650815a8544f54adcc0d504) by [Unito](https://www.unito.io) --------- Co-authored-by: dante <dante@danteui-MacStudio.local> |
||
|
AustinMroz
|
988a546721 |
Add missing dialog tests (#11133)
Leveraging the fancy coverage functionality of #10930, this PR aims to add coverage to missing dialogue models. This has proven quite constructive as many of the dialogues have since been shown to be bugged. - The APINodes sign in dialog that displays when attempting to run a workflow containing Partner nodes while not logged in was intended to display a list of nodes required to execute the workflow. The import for this component was forgotten in the original commit (#3532) and the backing component was later knipped - Error dialogs resulting are intended to display the file responsible for the error, but the prop was accidentally left out during the refactoring of #3265 - ~~The node library migration (#8548) failed to include the 'Edit Blueprint' button, and had incorrect sizing and color on the 'Delete Blueprint' button.~~ - On request, the library button changes were spun out to a separate PR ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11133-Add-missing-dialog-tests-33e6d73d3650812cb142d610461adcd4) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
pythongosssss
|
165984fe4c |
test: Improve speed of app mode input corruption test (#11236)
## Summary Speeds up test that was timing out https://9b579efd.comfyui-playwright-chromium.pages.dev/#?testId=b97e313f05078cede9be-5e6b75a76880fb6a5d96 ## Changes - **What**: - load prebuilt workflows to reduce test time (17s -> 11s) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11236-test-Improve-speed-of-app-mode-input-corruption-test-3426d73d3650815b9475ec96dfbd7ad5) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
34a02a29c9 |
test: Remove unnecessary setup, UseNewMenu and waitForNodes calls (#11237)
## Summary More simplification ## Changes - **What**: - Remove more UseNewMenu settings calls - Remove `await comfyPage.setup()` - Remove `waitForNodes` in vue node tagged tests ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11237-test-Remove-unnecessary-setup-UseNewMenu-and-waitForNodes-calls-3426d73d36508198a100c218420d479c) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
a09bb81b98 |
test: Auto wait for nodes after loadWorkflow in vue-node tests (#11238)
## Summary Updates tests to auto wait for vue-nodes when loading a workflow in a test with the vue-nodes tag ## Changes - **What**: - If tag includes vue-nodes, wait - Remove all load->wait calls ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11238-test-Auto-wait-for-nodes-after-loadWorkflow-in-vue-node-tests-3426d73d3650810e8760c5601186fde8) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
72eed86cea |
test: remove redundant setup/settings now handled by @vue-nodes fixture (#11195)
## Summary Follow-up cleanup for #11184 — removes redundant test setup calls that the `@vue-nodes` fixture now handles. ## Changes - **What**: Remove 40 lines of redundant `setSetting`, `setup()`, and `waitForNodes()` calls across 11 test files - `UseNewMenu: 'Top'` calls (already fixture default) - `setup()` + `waitForNodes()` on default workflow (fixture already does this for `@vue-nodes`) - Page reload in `subgraphZeroUuid` (fixture applies VueNodes.Enabled server-side before navigation) ## Review Focus Each removal was verified against the fixture's `setupSettings()` defaults (ComfyPage.ts:420-442) and the `@vue-nodes` auto-setup (lines 454-456). Tests that call `setup()`/`waitForNodes()` after `loadWorkflow()` or `page.evaluate()` were intentionally kept. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11195-test-remove-redundant-setup-settings-now-handled-by-vue-nodes-fixture-3416d73d36508154827df116a97e9130) by [Unito](https://www.unito.io) Co-authored-by: Amp <amp@ampcode.com> |
||
|
pythongosssss
|
5899a9392e |
test: Simplify vue node/menu test setup (#11184)
## Summary Simplifies test setup for common settings ## Changes - **What**: - add vue-nodes tag to auto enable nodes 2.0 - remove UseNewMenu Top as this is default ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11184-test-Simplify-vue-node-menu-test-setup-3416d73d3650815487e0c357d28761fe) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
a373633ab2 |
refactor: fix lint errors in tests (#11182)
## Summary Fix tests failing lint ## Changes - **What**: - Fix relative imports - Fix test not using comfyPage ## Review Focus <!-- Critical design decisions or edge cases that need attention --> <!-- If this PR fixes an issue, uncomment and update the line below --> <!-- Fixes #ISSUE_NUMBER --> ## Screenshots (if applicable) <!-- Add screenshots or video recording to help explain your changes --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11182-refactor-fix-lint-errors-in-tests-3416d73d3650812fbf1bc88554c57de2) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
521019d173 |
fix: exclude muted/bypassed nodes from missing asset detection (#10856)
## Summary Muted and bypassed nodes are excluded from execution but were still triggering missing model/media/node warnings. This PR makes the error system mode-aware: muted/bypassed nodes no longer produce missing asset errors, and all error lifecycle events (mode toggle, deletion, paste, undo, tab switch) are handled consistently. - Fixes Comfy-Org/ComfyUI#13256 ## Behavioral notes - **Tab switch overlay suppression (intentional)**: Switching back to a workflow with missing assets no longer re-shows the error overlay. This reverses the behavior introduced in #10190. The error state is still restored silently in the errors tab — users can access it via the properties panel without being interrupted by the overlay on every tab switch. ## Changes ### 1. Scan filtering - `scanAllModelCandidates`, `scanAllMediaCandidates`, `scanMissingNodes`: skip nodes with `mode === NEVER || BYPASS` - `collectMissingNodes` (serialized data): skip error reporting for muted/bypassed nodes while still calling `sanitizeNodeName` for safe `configure()` - `collectEmbeddedModelsWithSource`: skip muted/bypassed nodes; workflow-level `graphData.models` only create candidates when active nodes exist - `enrichWithEmbeddedMetadata`: filter unmatched workflow-level models when all referencing nodes are inactive ### 2. Realtime mode change handling - `useErrorClearingHooks.ts` chains `graph.onTrigger` to detect `node:property:changed` (mode) - Deactivation (active → muted/bypassed): remove missing model/media/node errors for the node - Activation (muted/bypassed → active): scan the node and add confirmed errors, show overlay - Subgraph container deactivation: remove all interior node errors (execution ID prefix match) - Subgraph container activation: scan all active interior nodes recursively - Subgraph interior mode change: resolve node via `localGraph.getNodeById()` then compute execution ID from root graph ### 3. Node deletion - `graph.onNodeRemoved`: remove missing model/media/node errors for the deleted node - Handle `node.graph === null` at callback time by using `String(node.id)` for root-level nodes ### 4. Node paste/duplicate - `graph.onNodeAdded`: scan via `queueMicrotask` (deferred until after `node.configure()` restores widget values) - Guard: skip during `ChangeTracker.isLoadingGraph` (undo/redo/tab switch handled by pipeline) - Guard: skip muted/bypassed nodes ### 5. Workflow tab switch optimization - `skipAssetScans` option in `loadGraphData`: skip full pipeline on tab switch - Cache missing model/media/node state per workflow via `PendingWarnings` - `beforeLoadNewGraph`: save current store state to outgoing workflow's `pendingWarnings` - `showPendingWarnings`: restore cached errors silently (no overlay), always sync missing nodes store (even when null) - Preserve UI state (`fileSizes`, `urlInputs`) on tab switch by using `setMissingModels([])` instead of `clearMissingModels()` - `MissingModelRow.vue`: fetch file size on mount via `fetchModelMetadata` memory cache ### 6. Undo/redo overlay suppression - `silentAssetErrors` option propagated through pipeline → `surfaceMissingModels`/`surfaceMissingMedia` `{ silent }` option - `showPendingWarnings` `{ silent }` option for missing nodes overlay - `changeTracker.ts`: pass `silentAssetErrors: true` on undo/redo ### 7. Error tab node filtering - Selected node filters missing model/media card contents (not just group visibility) - `isAssetErrorInSelection`: resolve execution ID → graph node for selection matching - Missing nodes intentionally unfiltered (pack-level scope) - `hasMissingMediaSelected` added to `RightSidePanel.vue` error tab visibility - Download All button: show only when 2+ downloadable models exist ### 8. New store functions - `missingModelStore`: `addMissingModels`, `removeMissingModelsByNodeId` - `missingMediaStore`: `addMissingMedia`, `removeMissingMediaByNodeId` - `missingNodesErrorStore`: `removeMissingNodesByNodeId` - `missingModelScan`: `scanNodeModelCandidates` (extracted single-node scan) - `missingMediaScan`: `scanNodeMediaCandidates` (extracted single-node scan) ### 9. Test infrastructure improvements - `data-testid` on `RightSidePanel.vue` tabs (`panel-tab-{value}`) - Error-related TestIds moved from `dialogs` to `errorsTab` namespace in `selectors.ts` - Removed unused `TestIdValue` type - Extracted `cleanupFakeModel` to shared `ErrorsTabHelper.ts` - Renamed `openErrorsTabViaSeeErrors` → `loadWorkflowAndOpenErrorsTab` - Added `aria-label` to pencil edit button and subgraph toggle button ## Test plan ### Unit tests (41 new) - Store functions: `addMissing*`, `removeMissing*ByNodeId` - `executionErrorStore`: `surfaceMissing*` silent option - Scan functions: muted/bypassed filtering, `scanNodeModelCandidates`, `scanNodeMediaCandidates` - `workflowService`: `showPendingWarnings` silent, `beforeLoadNewGraph` caching ### E2E tests (17 new in `errorsTabModeAware.spec.ts`) **Missing nodes** - [x] Deleting a missing node removes its error from the errors tab - [x] Undo after bypass restores error without showing overlay **Missing models** - [x] Loading a workflow with all nodes bypassed shows no errors - [x] Bypassing a node hides its error, un-bypassing restores it - [x] Deleting a node with missing model removes its error - [x] Undo after bypass restores error without showing overlay - [x] Pasting a node with missing model increases referencing node count - [x] Pasting a bypassed node does not add a new error - [x] Selecting a node filters errors tab to only that node **Missing media** - [x] Loading a workflow with all nodes bypassed shows no errors - [x] Bypassing a node hides its error, un-bypassing restores it - [x] Pasting a bypassed node does not add a new error - [x] Selecting a node filters errors tab to only that node **Subgraph** - [x] Bypassing a subgraph hides interior errors, un-bypassing restores them - [x] Bypassing a node inside a subgraph hides its error, un-bypassing restores it **Workflow switching** - [x] Does not resurface error overlay when switching back to workflow with missing nodes - [x] Restores missing nodes in errors tab when switching back to workflow # Screenshots https://github.com/user-attachments/assets/e0a5bcb8-69ba-4120-ab7f-5c83e4cfc3c5 ## Follow-up work - Extract error-detection computed properties from `RightSidePanel.vue` into a composable (e.g. `useErrorsTabVisibility`) --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: GitHub Action <action@github.com> |
||
|
Kelly Yang
|
bd82c855e0 |
test: add minimap E2E tests for graph content and click-to-navigate (#10738)
## Summary
Adds Playwright E2E tests verifying that
1. the minimap canvas renders node content
2. clears when the graph is empty
3. correctly navigates the main canvas on click
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10738-test-add-minimap-E2E-tests-for-graph-content-and-click-to-navigate-3336d73d365081eb955ce711b3efc57f)
by [Unito](https://www.unito.io)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk: changes are limited to adding `data-testid` attributes to
the minimap UI and expanding Playwright E2E assertions, with no
production behavior changes expected.
>
> **Overview**
> Strengthens minimap E2E coverage by switching existing assertions from
CSS selectors to new `data-testid`-based selectors and adding helper
utilities for canvas/overlay interactions.
>
> Adds new Playwright tests that verify the minimap canvas renders
content when nodes exist, clears when the graph is emptied, and that
clicking the minimap pans the main canvas (including a
post-`fitViewToSelectionAnimated` tolerance check).
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Kelly Yang
|
5b7ef3fe21 |
test: Painter Widget E2E Test Plan (#10846)
### Summary of Improvements
* **Custom Test Coverage Extension**: Enhanced the Painter widget E2E
test suite by refactoring logic for better maintainability and
robustness.
* **Stable Component Targeting**: Introduced
`data-testid="painter-dimension-text"` to `WidgetPainter.vue`, providing
a reliable, non-CSS-dependent locator for canvas size verification.
* **Improved Test Organization**: Reorganized existing test scenarios
into logical categories using `test.describe` blocks (Drawing, Brush
Settings, Canvas Size Controls, etc.).
* **Asynchronous Helper Integration**: Converted `hasCanvasContent` to
an asynchronous helper and unified its usage across multiple test cases
to eliminate redundant pixel-checking logic.
* **Locator Resilience**: Updated Reka UI slider interaction logic to
use more precise targeting (`:not([data-slot])`), preventing ambiguity
and improving test stability.
* **Scenario Refinement**: Updated the `pointerup` test logic to
accurately reflect pointer capture behavior when interactions occur
outside the canvas boundaries.
* **Enhanced Verification Feedback**: Added descriptive error messages
to `expect.poll` assertions to provide clearer context on potential
failure points.
* **Standardized Tagging**: Restored the original tagging strategy
(including `@smoke` and `@screenshot` tags) to ensure tests are
categorized correctly for CI environments.
### Red-Green Verification
| Commit | CI Status | Purpose |
| :--- | :--- | :--- |
| `test: refactor painter widget e2e tests and address review findings`
| 🟢 Green | Addresses all E2E test quality and stability issues from
review findings. |
### Test Plan
- [x] **Quality Checks**: `pnpm format`, `pnpm lint`, and `pnpm
typecheck` verified as passing.
- [x] **Component Integration**: `WidgetPainter.vue` `data-testid`
correctly applied and used in tests.
- [x] **Helper Reliability**: `hasCanvasContent` correctly identifies
colored pixels and returns a promise for `expect.poll`.
- [x] **Locator Robustness**: Verified Reka slider locators correctly
exclude internal thumb spans.
- [x] **Boundary Interaction**: Verified `pointerup` correctly ends
strokes when triggered outside the viewport.
- [x] **Tagging Consistency**: Verified `@smoke` and `@screenshot` tags
are present in the final test suite.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10846-test-Painter-Widget-E2E-Test-Plan-3386d73d365081deb70fe4afbd417efb)
by [Unito](https://www.unito.io)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Primarily adds/refactors Playwright E2E tests and stable `data-testid`
hooks, with no changes to Painter drawing logic. Risk is limited to
potential test brittleness or minor UI attribute changes.
>
> **Overview**
> Expands the Painter widget Playwright suite with new grouped scenarios
covering drawing/erasing behavior, tool switching, brush inputs, canvas
resizing (including preserving drawings), clear behavior, and
serialization/upload flows (including failure toast).
>
> Refactors the tests to use a shared `@e2e/helpers/painter` module
(`drawStroke`, `hasCanvasContent`, `triggerSerialization`), improves
stability via role/testid-based locators and clearer `expect.poll`
messaging, and adds `data-testid` attributes (e.g.,
`painter-clear-button`, `painter-*-row`, `painter-dimension-text`) to
`WidgetPainter.vue` to avoid CSS-dependent selectors.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Kelly Yang
|
85de833776 |
test: add E2E tests for ImageCompare widget (#10767)
## Summary
Add E2E tests for ImageCompare widget
Covers slider interaction, batch navigation, single-image modes, visual
regression screenshots, and edge cases for the ImageCompare Vue node
widget.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10767-test-add-E2E-tests-for-ImageCompare-widget-3346d73d365081c6bfc6fbd97fa04e4d)
by [Unito](https://www.unito.io)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Adds Playwright E2E coverage and screenshot assertions only; main risk
is increased CI runtime/flakiness due to additional image-loading and
hover/position polling.
>
> **Overview**
> Adds a new Playwright E2E suite for the ImageCompare Vue widget
(tagged `@widget`) that programmatically sets widget values and asserts
rendering for empty, single-image, and dual-image states.
>
> Expands coverage to **slider behavior** (default 50%, hover movement,
clamping, persistence) using polling on inline `clip-path`/handle
position, and adds **batch navigation** tests for multi-image
before/after sets.
>
> Introduces **visual regression screenshots** at default and specific
slider positions, plus edge-case tests for broken URLs, rapid updates
resetting batch index, legacy string values, and custom alt text.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Kelly Yang
|
cab46567c0 |
test: add E2E tests for ImageCropV2 widget (#10737)
## Summary
Adds Playwright E2E tests for the ImageCropV2 widget covering
1. the empty state (no source image)
2. default control rendering
3. source image display with crop overlay
4. drag-to-reposition behavior.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10737-test-add-E2E-tests-for-ImageCropV2-widget-3336d73d365081b28ed9db63e5df383e)
by [Unito](https://www.unito.io)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk: primarily adds Playwright E2E coverage and introduces
`data-testid` attributes for more stable selectors, with no changes to
core crop behavior.
>
> **Overview**
> Adds new Playwright E2E coverage for the `ImageCropV2` Vue-node
widget, including workflows/fixtures for a disconnected input and a
`LoadImage -> ImageCropV2 -> PreviewImage` pipeline.
>
> Tests validate the empty state and default controls, verify the crop
overlay renders after execution with screenshot assertions, and exercise
drag-to-reposition by dispatching pointer events and asserting the
widget’s crop value updates.
>
> Updates `WidgetImageCrop.vue` to add `data-testid` hooks (empty
state/icon and crop overlay) to make the E2E selectors stable.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Alexander Brown
|
6f579c5992 |
fix: enable playwright/no-force-option lint rule (#11164)
## Summary
Enable the previously disabled `playwright/no-force-option` lint rule at
error level and resolve all 29 violations across 10 files.
## Changes
### Lint rule
- `.oxlintrc.json`: `playwright/no-force-option` changed from `off` to
`error`
### Shared utility
- `CanvasHelper.ts`: Add `mouseClickAt()` and `mouseDblclickAt()`
methods that convert canvas-element-relative positions to absolute page
coordinates and use `page.mouse` APIs, avoiding Playwright's locator
actionability checks that fail when Vue DOM overlays sit above the
`<canvas>` element
### Force removal (20 violations)
- `selectionToolboxActions.spec.ts`: Remove `force: true` from 8 toolbox
button clicks (the `pointer-events: none` splitter overlay does not
intercept `elementFromPoint()`)
- `selectionToolboxSubmenus.spec.ts`: Remove `force: true` from 2
popover menu item clicks
- `BuilderSelectHelper.ts`: Remove `force: true` from 2 widget/node
clicks (builder mode does not disable pointer events)
- `linkInteraction.spec.ts`: Remove `force: true` from 3 slot `dragTo()`
calls (`::after` pseudo-elements do not intercept `elementFromPoint()`)
- `SidebarTab.ts`: Remove `force: true` from toast dismissal (`.catch()`
already handles failures)
- `nodeHelp.spec.ts`: Remove `force: true` from info button click
(preceding `toBeVisible()` assertion is sufficient)
### Rewrites (3 violations)
- `integerWidget.spec.ts`: Replace force-clicking disabled buttons with
`toBeDisabled()` assertions
- `Topbar.ts`: Replace force-click with `waitFor({ state: 'visible' })`
after hover
### Canvas coordinate clicks (9 violations)
- `litegraphUtils.ts`: Convert `NodeReference.click()` and
`navigateIntoSubgraph()` to use
`canvasOps.mouseClickAt()`/`mouseDblclickAt()`
- `subgraphPromotion.spec.ts`: Convert 3 right-click canvas calls to
`canvasOps.mouseClickAt()`
- `selectionToolboxSubmenus.spec.ts`: Convert 1 canvas dismiss-click to
`canvasOps.mouseClickAt()`
## Rationale
The original `force: true` usages were added defensively based on
incorrect assumptions about the `z-999 pointer-events: none` splitter
overlay intercepting Playwright's actionability checks. In reality,
`elementFromPoint()` skips elements with `pointer-events: none`, so the
overlay is transparent to Playwright's hit-test.
For canvas coordinate clicks, `force: true` on a locator does not tunnel
through DOM overlays — it only skips Playwright's preflight checks.
`page.mouse.click()` is the correct API for coordinate-based canvas
interactions.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11164-fix-enable-playwright-no-force-option-lint-rule-33f6d73d365081e78601c6114121d272)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Amp <amp@ampcode.com>
|
||
|
Alexander Brown
|
8c9328c1b2 |
feat: add eslint-plugin-playwright via oxlint JS plugins (#11136)
## Summary Add eslint-plugin-playwright as an oxlint JS plugin scoped to browser_tests/, enforcing Playwright best practices at lint time. ## Changes - **What**: Configure eslint-plugin-playwright@2.10.1 via oxlint's alpha `jsPlugins` field (`.oxlintrc.json` override scoped to `browser_tests/**/*.ts`). 18 recommended rules + `prefer-native-locators` + `require-to-pass-timeout` at error severity. All 173 initial violations resolved (config, auto-fix, manual fixes). `no-force-option` set to off — 28 violations need triage (canvas overlay workarounds vs unnecessary force) in a dedicated PR. - **Dependencies**: `eslint-plugin-playwright@^2.10.1` (devDependency, required by oxlint jsPlugins at runtime) ## Review Focus - `.oxlintrc.json` override structure — this is the first use of oxlint's JS plugins alpha feature in this repo - Manual fixes in spec files: `waitForSelector` → `locator.waitFor`, deprecated page methods → locator equivalents, `toPass()` timeout additions - Compound CSS selectors replaced with `.and()` (Playwright native locator composition) to avoid `prefer-native-locators` suppressions - Lint script changes in `package.json` to include `browser_tests/` in oxlint targets --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: GitHub Action <action@github.com> |
||
|
Johnpaul Chiwetelu
|
f0ae91de43 |
test: add e2e coverage for alt+drag duplicate and meta multi-select drag (#10994)
## Summary
Add Playwright coverage for two previously-untested canvas gestures:
Alt+drag to duplicate a regular node, and holding Meta across
click-click-click-drag to move multiple selected Vue nodes together.
## Changes
- **What**:
- `browser_tests/tests/interaction.spec.ts` — new `Node Duplication`
sub-describe with `Can duplicate a regular node via Alt+drag`. Asserts
CLIPTextEncode count goes 2 → 3 via poll, original node still exists.
Exercises the legacy canvas path at
`src/lib/litegraph/src/LGraphCanvas.ts:2434-2458`, which was only tested
for subgraph nodes before.
- `browser_tests/tests/vueNodes/interactions/node/move.spec.ts` — new
`should move all selected nodes together when dragging one with Meta
held`. Holds Meta for the entire sequence (3× `click({ modifiers:
['Meta'] })` + drag), asserts selection stays at 3 and all three nodes
move by the same delta. Exercises
`src/renderer/extensions/vueNodes/layout/useNodeDrag.ts:54-191`.
- Small refactor: `getLoadCheckpointHeaderPos` now delegates to a
reusable `getHeaderPos(comfyPage, title)` helper. Added `deltaBetween`
and `expectSameDelta` utilities (stricter than `expectPosChanged`, which
only checks `> 0`).
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10994-test-add-e2e-coverage-for-alt-drag-duplicate-and-meta-multi-select-drag-33d6d73d3650812dbf15c7053f44f0fd)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Alexander Brown <drjkl@comfy.org>
|
