Commit Graph

4 Commits

Author SHA1 Message Date
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>
2026-04-11 01:25:14 +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
3f375bea9c test: comprehensive E2E tests for error dialog, overlay, and errors tab (#10848)
## Summary

Comprehensive Playwright E2E tests for the error systems: ErrorDialog,
ErrorOverlay, and the errors tab (missing nodes, models, media,
execution errors).

## Changes

- **What**:
- **ErrorDialog** (`errorDialog.spec.ts`, 7 tests): configure/prompt
error triggers, Show Report, Copy to Clipboard, Find Issues on GitHub,
Contact Support
- **ErrorOverlay** (`errorOverlay.spec.ts`, 12 tests): error count
labels, per-type button labels (missing nodes/models/media/multiple),
See Errors flow (open panel, dismiss, close), undo/redo persistence
- **Errors tab — common** (`errorsTab.spec.ts`, 3 tests): tab
visibility, search/filter execution errors
- **Errors tab — Missing nodes** (`errorsTabMissingNodes.spec.ts`, 5
tests): MissingNodeCard, packs group, expand/collapse, locate button
- **Errors tab — Missing models** (`errorsTabMissingModels.spec.ts`, 6
tests): group display, model name, expand/referencing nodes, clipboard
copy, OSS Copy URL/Download buttons
- **Errors tab — Missing media** (`errorsTabMissingMedia.spec.ts`, 7
tests): migrated from `missingMedia.spec.ts` with detection,
upload/library/cancel flows, locate
- **Errors tab — Execution** (`errorsTabExecution.spec.ts`, 2 tests):
Find on GitHub/Copy buttons, runtime error panel
- **Shared helpers**: `ErrorsTabHelper.ts` (openErrorsTabViaSeeErrors),
`clipboardSpy.ts` (interceptClipboardWrite/getClipboardText)
- **Component changes**: added `data-testid` to
`ErrorDialogContent.vue`, `FindIssueButton.vue`, `MissingModelRow.vue`,
`MissingModelCard.vue`
  - **Selectors**: registered all new test IDs in `selectors.ts`
- **Test assets**: `missing_nodes_and_media.json` (compound errors),
`missing_models_with_nodes.json` (expand/locate)
- **Migrations**: error tests from `dialog.spec.ts` → dedicated files,
`errorOverlaySeeErrors.spec.ts` → `errorOverlay.spec.ts`,
`missingMedia.spec.ts` → `errorsTabMissingMedia.spec.ts`

## Review Focus

- OSS tests (`@oss` tag) verify Download/Copy URL buttons appear for
models with embedded URLs.
- The `missing_models.json` fixture must remain without nodes — adding
`CheckpointLoaderSimple` nodes causes directory mismatch in
`enrichWithEmbeddedMetadata` that prevents URL enrichment. A separate
`missing_models_with_nodes.json` fixture is used for expand/locate
tests.

## Cloud tests deferred

Missing model cloud environment tests (`@cloud` tag — hidden buttons,
import-unsupported notice) are deferred to a follow-up PR. The
`comfyPage` fixture cannot bypass the Firebase auth guard in cloud
builds, causing `window.app` initialization timeout. A separate infra PR
is needed to add cloud auth bypass to the fixture.

## Bug Discovery

During testing, a bug was found where the **Locate button for missing
nodes in subgraphs fails on initial workflow load**.
`collectMissingNodes` in `loadGraphData` captures execution IDs using
pre-`configure()` JSON node IDs, but `configure()` triggers subgraph
node ID deduplication (PR #8762, always-on since PR #9510) which remaps
colliding IDs. This will be addressed in a follow-up PR.

- Fixes #10847 (tracked, fix pending in separate PR)

## Testing

- 42 new/migrated E2E tests across 8 spec files
- All OSS tests pass locally and in CI

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10848-test-comprehensive-E2E-tests-for-error-dialog-overlay-and-errors-tab-3386d73d36508137a5e4cec8b12fa2fa)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-08 14:45:17 +09:00