fix: make flaky legacy prompt dialog test use locator rather than snapshot#7371
Merged
fix: make flaky legacy prompt dialog test use locator rather than snapshot#7371
Conversation
dosubot
bot
added
the
size:S
Contributor
📝 WalkthroughWalkthroughThe changes replace screenshot-based assertions with DOM visibility checks in browser interaction tests. Two test blocks for prompt dialogs now verify element visibility using the Changes
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (4)browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (browser_tests/CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx,vue,json}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (AGENTS.md)
Files:
browser_tests/**/*.spec.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (15)📓 Common learnings📚 Learning: 2025-12-09T20:22:23.620ZApplied to files:
📚 Learning: 2025-11-24T19:48:03.270ZApplied to files:
📚 Learning: 2025-11-24T19:47:22.909ZApplied to files:
📚 Learning: 2025-12-09T20:22:23.620ZApplied to files:
📚 Learning: 2025-11-24T19:48:03.270ZApplied to files:
📚 Learning: 2025-11-24T19:47:22.909ZApplied to files:
📚 Learning: 2025-12-10T03:09:19.636ZApplied to files:
📚 Learning: 2025-12-09T20:22:23.620ZApplied to files:
📚 Learning: 2025-12-09T20:22:23.620ZApplied to files:
📚 Learning: 2025-12-09T20:22:23.620ZApplied to files:
📚 Learning: 2025-11-24T19:47:22.909ZApplied to files:
📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
📚 Learning: 2025-12-09T03:39:54.501ZApplied to files:
📚 Learning: 2025-12-11T12:25:15.470ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/11/2025, 07:42:00 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 12/11/2025, 07:50:27 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.23 MB (baseline 3.23 MB) • ⚪ 0 BMain entry bundles and manifests
Graph Workspace — 988 kB (baseline 988 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
UI Components — 178 kB (baseline 178 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 3.18 kB (baseline 3.18 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.81 MB (baseline 3.81 MB) • ⚪ 0 BBundles that do not match a named category
|
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
![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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
browser_tests/tests/interaction.spec.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (browser_tests/CLAUDE.md)
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}: Test user workflows in browser tests
Use Playwright fixtures for browser tests
Follow naming conventions for browser tests
Check assets/ directory for test data when writing tests
Prefer specific selectors in browser tests
Test across multiple viewports
Files:
browser_tests/tests/interaction.spec.ts
**/*.{ts,tsx,js,jsx,vue,json}
📄 CodeRabbit inference engine (AGENTS.md)
Code style: Use 2-space indentation, single quotes, no trailing semicolons, and 80-character line width (see
.prettierrc)
Files:
browser_tests/tests/interaction.spec.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Imports must be sorted and grouped by plugin; runpnpm formatbefore committing
Use TypeScript for type safety; never useanytype - use proper TypeScript types
Never useas anytype assertions; fix the underlying type issue instead
Use es-toolkit for utility functions
Write code that is expressive and self-documenting; avoid comments unless absolutely necessary; do not add or retain redundant comments
Keep functions short and functional
Minimize nesting in code (e.g., deeply nestediforforstatements); apply the Arrow Anti-Pattern principle
Avoid mutable state; prefer immutability and assignment at point of declaration
Favor pure functions, especially testable ones
Files:
browser_tests/tests/interaction.spec.ts
browser_tests/**/*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
browser_tests/**/*.spec.ts: E2E test files must be namedbrowser_tests/**/*.spec.tsand use Playwright
Follow Playwright best practices described in the official documentation for E2E tests
Do not usewaitForTimeoutin Playwright tests; use Locator actions and retrying assertions instead
Use tags like@mobileand@2xin Playwright tests for relevant test scenarios
Files:
browser_tests/tests/interaction.spec.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to browser_tests/**/*.spec.ts : Do not use `waitForTimeout` in Playwright tests; use Locator actions and retrying assertions instead
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7303
File: src/components/topbar/CurrentUserPopover.test.ts:199-205
Timestamp: 2025-12-10T03:09:19.636Z
Learning: For test files in the Comfy-Org/ComfyUI_frontend repository: When writing tests, prefer selecting elements by accessible properties (text content, aria-label, role, accessible name) over data-testid attributes. This ensures tests verify actual user-facing behavior and accessibility compliance.
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Check tests-ui/README.md for test guidelines
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to browser_tests/**/*.spec.ts : Do not use `waitForTimeout` in Playwright tests; use Locator actions and retrying assertions instead
Applied to files:
browser_tests/tests/interaction.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Prefer specific selectors in browser tests
Applied to files:
browser_tests/tests/interaction.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
browser_tests/tests/interaction.spec.ts
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to browser_tests/**/*.spec.ts : Follow Playwright best practices described in the official documentation for E2E tests
Applied to files:
browser_tests/tests/interaction.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Check tests-ui/README.md for test guidelines
Applied to files:
browser_tests/tests/interaction.spec.ts
📚 Learning: 2025-12-10T03:09:19.636Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7303
File: src/components/topbar/CurrentUserPopover.test.ts:199-205
Timestamp: 2025-12-10T03:09:19.636Z
Learning: For test files in the Comfy-Org/ComfyUI_frontend repository: When writing tests, prefer selecting elements by accessible properties (text content, aria-label, role, accessible name) over data-testid attributes. This ensures tests verify actual user-facing behavior and accessibility compliance.
Applied to files:
browser_tests/tests/interaction.spec.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test user workflows in browser tests
Applied to files:
browser_tests/tests/interaction.spec.ts
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to **/*.test.ts : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
browser_tests/tests/interaction.spec.ts
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to **/*.test.ts : Aim for behavioral coverage of critical and new features in unit tests
Applied to files:
browser_tests/tests/interaction.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
browser_tests/tests/interaction.spec.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
browser_tests/tests/interaction.spec.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
browser_tests/tests/interaction.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint-and-format
- GitHub Check: test
- GitHub Check: setup
DrJKL
approved these changes
Dec 11, 2025
DrJKL
force-pushed
the
test/fix-flaky-prompt-dialog-test
branch
from
dc07912 to
26ab7c7
Compare
Enferlain
pushed a commit
to Enferlain/ComfyUI_frontend
that referenced
this pull request
Dec 18, 2025
…pshot (Comfy-Org#7371) ## Summary Fix the flakiness of [this test](https://fad8c753.comfyui-playwright-chromium.pages.dev/#?testId=967c1c643b6ca86a362c-8b516e2c224693bf7657) by converting it from using snapshots to just normal locators. The LiteGraph prompt that opens when click canvas widgets (number/string) is still the raw DOM dialog created by `LGraphCanvas.prototype.prompt`. That implementation wires its "click outside to close" handler inside a `setTimeout` and ignores outside clicks for ~256 ms after the dialog appears. It also never updates Vue state or exposes a ready attribute/event we can observe from Playwright. Because the UI offers no deterministic signal, using a short intentional wait that matches the real guard is reasonable. We assert the dialog becomes visible, call `await comfyPage.delay(300)` (just longer than the 256 ms guard), and then click outside. Without this wait the closing click fires before the handler exists, so the dialog remains visible and the test flakes. Until the widget exposes a ready hook, this scoped delay is the most reliable approach and stays within Playwright guidance ("only sleep when there is no observable condition to await"). ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7371-fix-make-flaky-legacy-prompt-dialog-test-use-locator-rather-than-snapshot-2c66d73d36508125b388e68861d7cd28) by [Unito](https://www.unito.io)
Yourz
pushed a commit
that referenced
this pull request
Dec 24, 2025
…pshot (#7371) ## Summary Fix the flakiness of [this test](https://fad8c753.comfyui-playwright-chromium.pages.dev/#?testId=967c1c643b6ca86a362c-8b516e2c224693bf7657) by converting it from using snapshots to just normal locators. The LiteGraph prompt that opens when click canvas widgets (number/string) is still the raw DOM dialog created by `LGraphCanvas.prototype.prompt`. That implementation wires its "click outside to close" handler inside a `setTimeout` and ignores outside clicks for ~256 ms after the dialog appears. It also never updates Vue state or exposes a ready attribute/event we can observe from Playwright. Because the UI offers no deterministic signal, using a short intentional wait that matches the real guard is reasonable. We assert the dialog becomes visible, call `await comfyPage.delay(300)` (just longer than the 256 ms guard), and then click outside. Without this wait the closing click fires before the handler exists, so the dialog remains visible and the test flakes. Until the widget exposes a ready hook, this scoped delay is the most reliable approach and stays within Playwright guidance ("only sleep when there is no observable condition to await"). ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7371-fix-make-flaky-legacy-prompt-dialog-test-use-locator-rather-than-snapshot-2c66d73d36508125b388e68861d7cd28) by [Unito](https://www.unito.io)
3 tasks
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.
Summary
Fix the flakiness of this test by converting it from using snapshots to just normal locators.
The LiteGraph prompt that opens when click canvas widgets (number/string) is still the raw DOM dialog created by
LGraphCanvas.prototype.prompt. That implementation wires its "click outside to close" handler inside asetTimeoutand ignores outside clicks for ~256 ms after the dialog appears. It also never updates Vue state or exposes a ready attribute/event we can observe from Playwright.Because the UI offers no deterministic signal, using a short intentional wait that matches the real guard is reasonable. We assert the dialog becomes visible, call
await comfyPage.delay(300)(just longer than the 256 ms guard), and then click outside. Without this wait the closing click fires before the handler exists, so the dialog remains visible and the test flakes. Until the widget exposes a ready hook, this scoped delay is the most reliable approach and stays within Playwright guidance ("only sleep when there is no observable condition to await").┆Issue is synchronized with this Notion page by Unito