Commit Graph

13 Commits

Author SHA1 Message Date
Alexander Brown
f1bb756929 fix: remove redundant and counterproductive e2e timeout overrides (#11110)
## Summary

Alright, alright, alright. These e2e tests have been runnin' around like
they're late for somethin', settin' tight little timeouts like the
world's gonna end in 250 milliseconds. Man, you gotta *breathe*. Let the
framework do its thing. Go slow to go fast, that's what I always say.

## Changes

- **What**: Removed ~120 redundant timeout overrides from auto-retrying
Playwright assertions (`toBeVisible`, `toBeHidden`, `toHaveCount`,
`toBeEnabled`, `toHaveAttribute`, `toContainText`, `expect.poll`) where
5000ms is already the default. Also removed sub-5s timeouts (1s, 2s, 3s)
that were just *begging* for flaky failures — like wearin' a belt and
suspenders and also holdin' your pants up with both hands. Raised the
absurdly short timeouts in `customMatchers.ts` (250ms `toPass` → 5000ms,
256ms poll → default). Kept `timeout: 5000` on `.toPass()` calls
(defaults to 0), `.waitFor()`, `waitForRequest`, `waitForFunction`,
intentionally-short timeouts inside retry loops, and conditional
`.isVisible()/.catch()` checks — those fellas actually need the help.

## Review Focus

Every remaining timeout in the diff is there for a *reason*. The ones on
`.toPass()` stay because that API defaults to zero — it won't retry at
all without one. The ones on `.waitFor()` and `waitForRequest` stay
because those are locator actions, not auto-retrying assertions. The
intentionally-short ones inside `toPass` retry loops
(`interaction.spec.ts`) and the negative assertions (`actionbar.spec.ts`
confirming no response arrives) — those are *supposed* to be tight.

The short timeouts on regular assertions were actively *encouragin'*
flaky failures. That's like settin' your alarm for 4 AM and then gettin'
mad you're tired. Just... don't do that, man. Let things take the time
they need.

38 files, net -115 lines. Less code, more chill. That's livin'.

---------

Co-authored-by: Amp <amp@ampcode.com>
2026-04-10 19:47:20 +00:00
Alexander Brown
0132c77c7d test: harden 82 Playwright specs for deterministic CI runs (#10967)
## Summary

Harden 98 E2E spec files and 8 fixtures/helpers for deterministic CI
runs by replacing race-prone patterns with retry-safe alternatives.

No source code changes -- only `browser_tests/` is touched.

## Changes

- **E2E spec hardening** (98 spec files, 6 fixtures, 2 helpers):

  | Fix class | Sites | Examples |
  |-----------|-------|---------:|
| `expect(await ...)` -> `expect.poll()` | ~153 | interaction,
defaultKeybindings, workflows, featureFlags |
| `const x = await loc.count(); expect(x)` -> `toHaveCount()` | ~19 |
menu, linkInteraction, assets, bottomPanelShortcuts |
| `nextFrame()` -> `waitForHidden()` after menu clicks | ~22 |
contextMenu, rightClickMenu, subgraphHelper |
| Redundant `nextFrame()` removed | many | defaultKeybindings, minimap,
builderSaveFlow |
| `expect(async () => { ... }).toPass()` retry blocks | 5 | interaction
(graphdialog dismiss guard) |
| `force:true` removed from `BaseDialog.close()` | 1 | BaseDialog
fixture |
| ContextMenu `waitForHidden` simplified (check-then-act race removed) |
1 | ContextMenu fixture |
| Non-deterministic node order -> proximity-based selection | 1 |
interaction (toggle dom widget) |
  | Tight poll timeout (250ms) -> >=2000ms | 2 | templates |

- **Helper improvements**: Exposed locator getters on
`ComfyPage.domWidgets`, `ToastHelper.toastErrors`, and
`WorkflowsSidebarTab.activeWorkflowLabel` so callers can use retrying
assertions (`toHaveCount()`, `toHaveText()`) directly.

- **Flake pattern catalog**: Added section 7 table to
`browser_tests/FLAKE_PREVENTION_RULES.md` documenting 8 pattern classes
for reviewers and future authors.

- **Docs**: Fixed bad examples in `browser_tests/README.md` to use
`expect.poll()`.

- **Breaking**: None
- **Dependencies**: None

## Review Focus

- All fixes follow the rules in
`browser_tests/FLAKE_PREVENTION_RULES.md`
- No behavioral changes to tests -- only timing/retry strategy is
updated
- The `ContextMenu.waitForHidden` simplification removes a
swallowed-error anti-pattern; both locators now use direct `waitFor({
state: 'hidden' })`

---------

Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: github-actions <github-actions@github.com>
2026-04-09 20:50:56 -07:00
Alexander Brown
3b78dfbe1c test: migrate browser_tests/ to @e2e/ path alias and add lint rule (#10958)
## Summary

Complete the @e2e/ path alias migration started in #10735 by converting
all 354 remaining relative imports and adding a lint rule to prevent
backsliding.

## Changes

- **What**: Migrate all relative imports in browser_tests/ to use
`@e2e/` (intra-directory) and `@/` (src/ imports) path aliases. Add
`no-restricted-imports` ESLint rule banning `./` and `../` imports in
`browser_tests/**/*.ts`. Suppress pre-existing oxlint `no-eval` and
`no-console` warnings exposed by touching those files.

## Review Focus

- ESLint flat-config merging: the `@playwright/test` ban and
relative-import ban are in two separate blocks to avoid last-match-wins
collision with the `useI18n`/`useVirtualList` blocks higher in the
config.
- The `['./**', '../**']` glob patterns (not `['./*', '../*']`) are
needed to catch multi-level relative paths like `../../../src/foo`.

Follows up on #10735

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10958-test-migrate-browser_tests-to-e2e-path-alias-and-add-lint-rule-33c6d73d365081649d1be771eac986fd)
by [Unito](https://www.unito.io)

Co-authored-by: Amp <amp@ampcode.com>
2026-04-08 11:28:59 -07:00
jaeone94
8b1cf594d1 fix: improve error overlay design and indicator placement (#10564)
## Summary

- Move error red border from TopMenuSection/ComfyActionbar to
ErrorOverlay
- Add error indicator (outline + StatusBadge dot) on right side panel
toggle button when errors are present, the panel/overlay are closed, and
the errors tab setting is enabled
- Replace technical group titles (e.g. "Missing Node Packs") with
user-friendly i18n messages in ErrorOverlay
- Dynamically change action button label based on single error type
(e.g. "Show missing nodes" instead of "See Errors")
- Remove unused `hasAnyError` prop from ComfyActionbar
- Fix `type="secondary"` → `variant="secondary"` on panel toggle button
- Pre-wire `missing_media` error type support for #10309
- Migrate ErrorOverlay E2E selectors from `getByText`/`getByRole` to
`data-testid`
- Update E2E screenshot snapshots affected by TopMenuSection error state
design changes

## Test plan

- [x] Trigger execution error → verify red border on ErrorOverlay, no
red border on TopMenuSection/ComfyActionbar
- [x] With errors and right side panel closed → verify red outline + dot
on panel toggle button
- [x] Open right side panel or error overlay → verify indicator
disappears
- [x] Disable `Comfy.RightSidePanel.ShowErrorsTab` → verify no indicator
even with errors
- [x] Load workflow with only missing nodes → verify "Show missing
nodes" button label and friendly message
- [x] Load workflow with only missing models → verify "Show missing
models" button label and count message
- [x] Load workflow with mixed errors → verify "See Errors" fallback
label
- [x] E2E: `pnpm test:browser:local -- --grep "Error overlay"`

## Screenshots
<img width="498" height="381" alt="스크린샷 2026-03-26 230252"
src="https://github.com/user-attachments/assets/034f0f3f-e6a1-4617-b8f6-cd4c145e3a47"
/>
<img width="550" height="303" alt="스크린샷 2026-03-26 230525"
src="https://github.com/user-attachments/assets/2958914b-0ff0-461b-a6ea-7f2811bf33c2"
/>
<img width="551" height="87" alt="스크린샷 2026-03-26 230318"
src="https://github.com/user-attachments/assets/396e9cb1-667e-44c4-83fe-ab113b313d16"
/>

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Dante <bunggl@naver.com>
2026-03-27 09:11:26 +09:00
jaeone94
e2ef041170 feat: surface missing models in Error Tab for OSS and remove legacy dialog (#9921)
## Summary
- Surface missing models in the Error Tab for OSS environments,
replacing the legacy modal dialog
- Add Download button per model and Download All button in group header
with file size display
- Move download business logic from `components/dialog/content` to
`platform/missingModel`
- Remove legacy missing models dialog components and composable

## Changes
- **Pipeline**: Remove `isCloud` guard from `scanAllModelCandidates` and
`surfaceMissingModels` so OSS detects missing models
- **Grouping**: Group non-asset-supported models by directory in OSS
instead of lumping into UNSUPPORTED
- **UI**: Add Download button (matching Install Node Pack design) and
Download All header button
- **Store**: Add `folderPaths`/`fileSizes` state with setter methods,
race condition guard
- **Cleanup**: Delete `MissingModelsContent`, `MissingModelsHeader`,
`MissingModelsFooter`, `useMissingModelsDialog`, `missingModelsUtils`
- **Tests**: Add OSS/Cloud grouping tests, migrate Playwright E2E to
Error Tab, improve test isolation
- **Snapshots**: Reset Playwright screenshot expectations since OSS
missing model error detection now causes red highlights on affected
nodes
- **Accessibility**: Add `aria-label` with model name, `aria-expanded`
on toggle, warning icon for unknown category

## Test plan
- [x] Unit tests pass (86 tests)
- [x] TypeScript typecheck passes
- [x] knip passes
- [x] Load workflow with missing models in OSS → Error Tab shows missing
models grouped by directory
- [x] Download button triggers browser download with correct URL
- [x] Download All button downloads all downloadable models
- [x] Cloud environment behavior unchanged
- [x] Playwright E2E: `pnpm test:browser:local -- --grep "Missing models
in Error Tab"`

## Screenshots


https://github.com/user-attachments/assets/12f15e09-215a-4c58-87ed-39bbffd1359c

 


┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9921-feat-surface-missing-models-in-Error-Tab-for-OSS-and-remove-legacy-dialog-3236d73d365081f0a9dfc291978f5ecf)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: github-actions <github-actions@github.com>
2026-03-15 22:46:47 +09:00
jaeone94
46c40c755e feat: node-specific error tab with selection-aware grouping and error overlay (#8956)
## Summary
Enhances the error panel with node-specific views: single-node selection
shows errors grouped by message in compact mode, container nodes
(subgraph/group) expose child errors via a badge and "See Error" button,
and a floating ErrorOverlay appears after execution failure with a
deduplicated summary and quick navigation to the errors tab.

## Changes
- **Consolidate error tab**: Remove `TabError.vue`; merge all error
display into `TabErrors.vue` and drop the separate `error` tab type from
`rightSidePanelStore`
- **Selection-aware grouping**: Single-node selection regroups errors by
message (not `class_type`) and renders `ErrorNodeCard` in compact mode
- **Container node support**: Detect child-node errors in subgraph/group
nodes via execution ID prefix matching; show error badge and "See Error"
button in `SectionWidgets`
- **ErrorOverlay**: New floating card shown after execution failure with
deduplicated error messages, "Dismiss" and "See Errors" actions;
`isErrorOverlayOpen` / `showErrorOverlay` / `dismissErrorOverlay` added
to `executionStore`
- **Refactor**: Centralize error ID collection in `executionStore`
(`allErrorExecutionIds`, `hasInternalErrorForNode`); split `errorGroups`
into `allErrorGroups` (unfiltered) and `tabErrorGroups`
(selection-filtered); move `ErrorOverlay` business logic into
`useErrorGroups`

## Review Focus
- `useErrorGroups.ts`: split into `allErrorGroups` / `tabErrorGroups`
and the new `filterBySelection` parameter flow
- `executionStore.ts`: `hasInternalErrorForNode` helper and
`allErrorExecutionIds` computed
- `ErrorOverlay.vue`: integration with `executionStore` overlay state
and `useErrorGroups`

## Screenshots
<img width="853" height="461" alt="image"
src="https://github.com/user-attachments/assets/a49ab620-4209-4ae7-b547-fba13da0c633"
/>
<img width="854" height="203" alt="image"
src="https://github.com/user-attachments/assets/c119da54-cd78-4e7a-8b7a-456cfd348f1d"
/>
<img width="497" height="361" alt="image"
src="https://github.com/user-attachments/assets/74b16161-cf45-454b-ae60-24922fe36931"
/>

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: github-actions <github-actions@github.com>
2026-02-20 12:14:52 -08:00
Alexander Brown
f2d5bfab73 test(browser): refactor browser tests for reliability and maintainability (#8510)
## Summary

Major refactoring of browser tests to improve reliability,
maintainability, and type safety.

## Changes

### Test Infrastructure Decomposition
- Decomposed `ComfyPage.ts` (~1000 lines) into focused helpers:
- `CanvasHelper`, `DebugHelper`, `SubgraphHelper`,
`NodeOperationsHelper`
- `SettingsHelper`, `WorkflowHelper`, `ClipboardHelper`,
`KeyboardHelper`
- Created `ContextMenu` page object, `BaseDialog` base class, and
`BottomPanel` page object
- Extracted `DefaultGraphPositions` constants

### Locator Stability
- Added `data-testid` attributes to Vue components (sidebar, dialogs,
node library)
- Created centralized `selectors.ts` with test ID constants
- Replaced fragile CSS selectors (`.nth()`, `:nth-child()`) with
`getByTestId`/`getByRole`

### Performance & Reliability
- Removed `setTimeout` anti-patterns (replaced with `waitForFunction`)
- Replaced `waitForTimeout` with retrying assertions
- Replaced hardcoded coordinates with computed `NodeReference` positions
- Enforced LF line endings for all text files

### Type Safety
- Enabled `no-explicit-any` lint rule for browser_tests via oxlint
- Purged `as any` casts from browser_tests
- Added Window type augmentation for standardized window access
- Added proper type annotations throughout

### Bug Fixes
- Restored `ExtensionManager` API contract
- Removed test-only settings from production schema
- Fixed flaky selectors and missing test setup

## Testing
- All browser tests pass
- Typecheck passes


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
* Overhauled browser E2E test infrastructure with many new
helpers/fixtures, updated test APIs, and CI test container image bumped
for consistency.

* **Chores**
* Standardized line endings and applied stricter lint rules for browser
tests; workspace dependency version updated.

* **Documentation**
* Updated Playwright and TypeScript testing guidance and test-run
commands.

* **UI**
* Added stable data-testids to multiple components to improve
testability.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2026-02-03 12:29:40 -08:00
Alexander Brown
82bacb82a7 test: add Playwright test tags for filtering (@smoke, @slow, @screenshot, domains) (#8441)
## Summary

Adds structured test tags to all 54 Playwright test files to enable
flexible test filtering during development and CI.

## Tags Added

| Tag | Count | Purpose |
|-----|-------|---------|
| `@screenshot` | 32 files | Tests with visual assertions
(`toHaveScreenshot`) |
| `@smoke` | 5 files | Quick essential tests for fast validation |
| `@slow` | 5 files | Long-running tests (templates, subgraph,
featureFlags) |
| `@canvas` | 15 files | Canvas/graph rendering tests |
| `@node` | 10 files | Node behavior tests |
| `@ui` | 8 files | UI component tests |
| `@widget` | 5 files | Widget-specific tests |
| `@workflow` | 3 files | Workflow operations |
| `@subgraph` | 1 file | Subgraph functionality |
| `@keyboard` | 2 files | Keyboard shortcuts |
| `@settings` | 2 files | Settings/preferences |

## Usage Examples

```bash
# Quick validation (~16 tests, ~30s)
pnpm test:browser -- --grep @smoke

# Skip slow tests for faster CI feedback
pnpm test:browser -- --grep-invert @slow

# Skip visual tests (useful for local development without snapshots)
pnpm test:browser -- --grep-invert @screenshot

# Run only canvas-related tests
pnpm test:browser -- --grep @canvas

# Combine filters
pnpm test:browser -- --grep @smoke --grep-invert @screenshot
```

## Implementation Details

- Uses Playwright's native tag syntax: `test.describe('Name', { tag:
'@tag' }, ...)`
- Tags inherit from describe blocks to child tests
- Preserves existing project-level tags: `@mobile`, `@2x`, `@0.5x`
- Multiple tags supported: `{ tag: ['@screenshot', '@smoke'] }`

## Test Plan

- [x] All existing tests pass unchanged
- [x] Tag filtering works with `--grep` and `--grep-invert`

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8441-test-add-Playwright-test-tags-for-filtering-smoke-slow-screenshot-domains-2f76d73d36508184990ec859c8fd7629)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: GitHub Action <action@github.com>
2026-01-29 16:34:56 -08:00
Alexander Brown
bf8d9de1c1 Fix: Flaky Playwright Tests: retry some assertions (#7389)
## Summary

Retries the widget value change check for up to 2 whole seconds.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7389-Fix-remoteWidgets-Playwright-test-add-retry-for-assertion-2c66d73d3650814e98b6fdfc83f6d3d6)
by [Unito](https://www.unito.io)
2025-12-11 14:02:23 -08:00
Alexander Brown
5c01861f4e Tests: Playwright test timeouts (#7231)
## Summary

See where we can use proper DOM waiting instead of waitForTimeout.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7231-WIP-Playwright-test-timeouts-2c36d73d3650812b966ac3d9c338dfd4)
by [Unito](https://www.unito.io)
2025-12-08 15:50:35 -08:00
Benjamin Lu
bc4549244e test(e2e): align test default menu to Top; make legacy specs explicit (#5746)
## Summary
UseNewMenu has been defaulted to Top in the app for over a year;
Playwright’s test default lagged behind. This PR aligns the test default
with reality and keeps legacy specs stable.

## Changes
- tests(e2e): default to 'Top' via fixture; specs that previously relied
on the old implicit default now explicitly set 'Comfy.UseNewMenu' to
'Disabled'.
- docs(browser-tests): remove outdated README note suggesting tests set
'Top' manually.

## Review Focus
- Intentional uses of 'Top' and 'Bottom' remain unchanged.
- Confirm ComfyPage default remains 'Top' (see
browser_tests/fixtures/ComfyPage.ts).

## Screenshots (if applicable)
N/A

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5746-test-e2e-align-test-default-menu-to-Top-make-legacy-specs-explicit-2786d73d365081218d06c1346f3ae18e)
by [Unito](https://www.unito.io)

---------

Co-authored-by: github-actions <github-actions@github.com>
2025-09-24 18:43:22 -07:00
Chenlei Hu
2425e32d51 Partial execute to selected output nodes (#3818)
Co-authored-by: github-actions <github-actions@github.com>
2025-05-08 11:44:26 -04:00
Chenlei Hu
cb06d96930 [Refactor] Use NodeSlot.hasErrors API (#3284)
Co-authored-by: github-actions <github-actions@github.com>
2025-03-30 20:10:28 -04:00