fix: update imagePreview browser tests to use current fixture APIs - #8995
Merged
Conversation
dosubot
Bot
added
the
size:S
Contributor
📝 WalkthroughWalkthroughReplaces flat ComfyPage test APIs with nested fixture properties ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/20/2026, 07:26:57 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
|
Playwright: ✅ 518 passed, 0 failed · 4 flaky 📊 Browser Reports
|
📦 Bundle: 4.27 MB gzip ⚪ 0 BDetailsSummary
Category Glance App Entry Points — 21.4 kB (baseline 21.4 kB) • ⚪ 0 BMain entry bundles and manifests
Graph Workspace — 914 kB (baseline 914 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Views & Navigation — 68.6 kB (baseline 68.6 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 430 kB (baseline 430 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Editors & Dialogs — 706 B (baseline 706 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
UI Components — 42.3 kB (baseline 42.3 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 2.4 MB (baseline 2.4 MB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 57.6 kB (baseline 57.6 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 8.7 MB (baseline 8.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.38 MB (baseline 7.38 MB) • ⚪ 0 BBundles that do not match a named category
|
christian-byrne
force-pushed
the
fix/image-preview-browser-tests
branch
from
February 20, 2026 06:00
3ed0c97 to
dd2263b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@browser_tests/tests/vueNodes/interactions/node/imagePreview.spec.ts`:
- Around line 13-16: The helper loadImageOnNode currently assumes
comfyPage.nodeOps.getNodeRefsByType('LoadImage') returns at least one entry and
then uses loadImageNode.getPosition(), which can throw if the array is empty;
update loadImageOnNode to explicitly guard the result of
comfyPage.nodeOps.getNodeRefsByType('LoadImage') (in the loadImageOnNode
function), check that the returned array has length > 0 and that loadImageNode
is defined, and if not throw or return a clear, descriptive error (or fail the
test) instead of proceeding to call methods like loadImageNode.getPosition();
keep the check near the existing retrieval so the error is localized.
Comment on lines
+13
to
+16
| async function loadImageOnNode(comfyPage: ComfyPage) { | ||
| const loadImageNode = ( | ||
| await comfyPage.nodeOps.getNodeRefsByType('LoadImage') | ||
| )[0] |
Contributor
There was a problem hiding this comment.
Guard against missing LoadImage nodes to avoid undefined access.
getNodeRefsByType('LoadImage') can return an empty array if the workflow fails to load or changes, which will throw on getPosition() with a low-signal error. Add an explicit guard for clearer failures.
✅ Suggested fix
- const loadImageNode = (
- await comfyPage.nodeOps.getNodeRefsByType('LoadImage')
- )[0]
+ const loadImageNodes =
+ await comfyPage.nodeOps.getNodeRefsByType('LoadImage')
+ expect(loadImageNodes.length).toBeGreaterThan(0)
+ const loadImageNode = loadImageNodes[0]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function loadImageOnNode(comfyPage: ComfyPage) { | |
| const loadImageNode = ( | |
| await comfyPage.nodeOps.getNodeRefsByType('LoadImage') | |
| )[0] | |
| async function loadImageOnNode(comfyPage: ComfyPage) { | |
| const loadImageNodes = | |
| await comfyPage.nodeOps.getNodeRefsByType('LoadImage') | |
| expect(loadImageNodes.length).toBeGreaterThan(0) | |
| const loadImageNode = loadImageNodes[0] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@browser_tests/tests/vueNodes/interactions/node/imagePreview.spec.ts` around
lines 13 - 16, The helper loadImageOnNode currently assumes
comfyPage.nodeOps.getNodeRefsByType('LoadImage') returns at least one entry and
then uses loadImageNode.getPosition(), which can throw if the array is empty;
update loadImageOnNode to explicitly guard the result of
comfyPage.nodeOps.getNodeRefsByType('LoadImage') (in the loadImageOnNode
function), check that the returned array has length > 0 and that loadImageNode
is defined, and if not throw or return a clear, descriptive error (or fail the
test) instead of proceeding to call methods like loadImageNode.getPosition();
keep the check near the existing retrieval so the error is localized.
The tests from PR #8143 were written against stale ComfyPage APIs that were refactored in PR #8510: - comfyPage.dragAndDropFile → comfyPage.dragDrop.dragAndDropFile - comfyPage.setSetting → comfyPage.settings.setSetting - comfyPage.loadWorkflow → comfyPage.workflow.loadWorkflow - comfyPage.getNodeRefsByType → comfyPage.nodeOps.getNodeRefsByType - comfyPage type param → import ComfyPage type directly Also removes test.fixme since the root cause was API mismatch. Amp-Thread-ID: https://ampcode.com/threads/T-019c73c1-be32-7687-b758-672fedaf61af
christian-byrne
force-pushed
the
fix/image-preview-browser-tests
branch
from
February 20, 2026 07:25
dd2263b to
9d52cca
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
browser_tests/tests/vueNodes/interactions/node/imagePreview.spec.ts (1)
31-45:⚠️ Potential issue | 🟡 MinorTests are still
fixmedespite the stated re-enable intent.The TODOs indicate re-enabling after sync, but the tests remain disabled. If the API mismatch was the root cause, consider re-enabling now or update the TODO/PR objective to reflect current intent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@browser_tests/tests/vueNodes/interactions/node/imagePreview.spec.ts` around lines 31 - 45, The two tests marked with test.fixme (the "opens mask editor from image preview button" and "shows image context menu options" cases that call loadImageOnNode and interact with comfyPage) were left disabled despite the TODO; decide and apply one of: re-enable them by replacing test.fixme with test (or test.describe/appropriate runner call) and run CI to confirm they pass, or update the TODO/comment to reflect that they should remain disabled and add a clear ticket/issue reference; ensure any API mismatch causing failures is fixed in the helper loadImageOnNode or the comfyPage interactions (hover, getByLabel, locator) before re-enabling so the tests don't flake.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@browser_tests/tests/vueNodes/interactions/node/imagePreview.spec.ts`:
- Around line 31-45: The two tests marked with test.fixme (the "opens mask
editor from image preview button" and "shows image context menu options" cases
that call loadImageOnNode and interact with comfyPage) were left disabled
despite the TODO; decide and apply one of: re-enable them by replacing
test.fixme with test (or test.describe/appropriate runner call) and run CI to
confirm they pass, or update the TODO/comment to reflect that they should remain
disabled and add a clear ticket/issue reference; ensure any API mismatch causing
failures is fixed in the helper loadImageOnNode or the comfyPage interactions
(hover, getByLabel, locator) before re-enabling so the tests don't flake.
---
Duplicate comments:
In `@browser_tests/tests/vueNodes/interactions/node/imagePreview.spec.ts`:
- Around line 13-16: The helper loadImageOnNode should guard against an empty
result from comfyPage.nodeOps.getNodeRefsByType('LoadImage') before
dereferencing [0]; update loadImageOnNode to check the returned array length
(from getNodeRefsByType('LoadImage')) and either throw a clear error or return
early if no LoadImage node exists, so subsequent calls like
loadImageNode.getPosition() are not invoked on undefined.
|
Related Documentation Checked 0 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
DrJKL
pushed a commit
that referenced
this pull request
Mar 5, 2026
…8995) The browser tests added in #8143 were failing on main because they were written against stale `ComfyPage` APIs that were refactored in #8510 (merged Feb 3, before #8143 merged Feb 18). ### Changes - `comfyPage.dragAndDropFile` → `comfyPage.dragDrop.dragAndDropFile` - `comfyPage.setSetting` → `comfyPage.settings.setSetting` - `comfyPage.loadWorkflow` → `comfyPage.workflow.loadWorkflow` - `comfyPage.getNodeRefsByType` → `comfyPage.nodeOps.getNodeRefsByType` - Fix `comfyPage` type parameter to use `ComfyPage` import - Remove `test.fixme` since root cause was API mismatch, not test logic ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8995-fix-update-imagePreview-browser-tests-to-use-current-fixture-APIs-30d6d73d365081219c1eda4ea7251160) by [Unito](https://www.unito.io)
zeekay
pushed a commit
to hanzoui/frontend
that referenced
this pull request
Jul 26, 2026
…omfy-Org#8995) The browser tests added in Comfy-Org#8143 were failing on main because they were written against stale `ComfyPage` APIs that were refactored in Comfy-Org#8510 (merged Feb 3, before Comfy-Org#8143 merged Feb 18). ### Changes - `comfyPage.dragAndDropFile` → `comfyPage.dragDrop.dragAndDropFile` - `comfyPage.setSetting` → `comfyPage.settings.setSetting` - `comfyPage.loadWorkflow` → `comfyPage.workflow.loadWorkflow` - `comfyPage.getNodeRefsByType` → `comfyPage.nodeOps.getNodeRefsByType` - Fix `comfyPage` type parameter to use `ComfyPage` import - Remove `test.fixme` since root cause was API mismatch, not test logic ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8995-fix-update-imagePreview-browser-tests-to-use-current-fixture-APIs-30d6d73d365081219c1eda4ea7251160) by [Unito](https://www.unito.io)
dante01yoon
pushed a commit
that referenced
this pull request
Jul 27, 2026
…8995) The browser tests added in #8143 were failing on main because they were written against stale `ComfyPage` APIs that were refactored in #8510 (merged Feb 3, before #8143 merged Feb 18). ### Changes - `comfyPage.dragAndDropFile` → `comfyPage.dragDrop.dragAndDropFile` - `comfyPage.setSetting` → `comfyPage.settings.setSetting` - `comfyPage.loadWorkflow` → `comfyPage.workflow.loadWorkflow` - `comfyPage.getNodeRefsByType` → `comfyPage.nodeOps.getNodeRefsByType` - Fix `comfyPage` type parameter to use `ComfyPage` import - Remove `test.fixme` since root cause was API mismatch, not test logic ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8995-fix-update-imagePreview-browser-tests-to-use-current-fixture-APIs-30d6d73d365081219c1eda4ea7251160) by [Unito](https://www.unito.io)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The browser tests added in #8143 were failing on main because they were written against stale
ComfyPageAPIs that were refactored in #8510 (merged Feb 3, before #8143 merged Feb 18).Changes
comfyPage.dragAndDropFile→comfyPage.dragDrop.dragAndDropFilecomfyPage.setSetting→comfyPage.settings.setSettingcomfyPage.loadWorkflow→comfyPage.workflow.loadWorkflowcomfyPage.getNodeRefsByType→comfyPage.nodeOps.getNodeRefsByTypecomfyPagetype parameter to useComfyPageimporttest.fixmesince root cause was API mismatch, not test logic┆Issue is synchronized with this Notion page by Unito