test(e2e): split @lifecycle into @ci + @real tiers (#621)#640
Draft
Kosinkadink wants to merge 13 commits into
Draft
test(e2e): split @lifecycle into @ci + @real tiers (#621)#640Kosinkadink wants to merge 13 commits into
Kosinkadink wants to merge 13 commits into
Conversation
The @lifecycle Playwright tag had drifted to mean three different things at once (slow, real-ish, lifecycle-adjacent) and the project was excluded from CI entirely. Result: 81 `@lifecycle` tests across 28 files, never verified on PR, some silently broken. Split into two explicit tiers on independent axes: - @ci — every-PR matrix (Windows + macOS + Linux). Seeded harness, __e2e backdoor, window.api bridge, fixtures on disk all allowed. Fast, deterministic, covers renderer + IPC contracts. - @real — real lifecycle. `pnpm run test:e2e:real`. No seeds, no __e2e mutations, no window.api shortcuts, no fake fixture installs. Behaves as if a human had booted the app and clicked through the UI. OS axis stays independent: @ci defaults to all three OSes; @windows-only / @macos-only / @linux-only opt out when behavior is genuinely platform-specific. quit-flow.spec.ts is the lone opt-out today (@ci @macos-only — tray-close mode is macOS). Tag inventory after this change: - 2 files @real: lifecycle.test.ts, lifecycle-cloud.test.ts - 33 files @ci (1 with @macos-only) - 0 files untagged Four tests are currently broken and marked with TODO(#621): - picker-stop-confirm.test.ts:159 (skip) — forwarded update-comfyui never reaches run-action. - lifecycle-snapshot-restore.test.ts:274 (skip) — picker-driven snapshot row never appears. - lifecycle-update-check.test.ts:218 (fail) — PRODUCT BUG from #595: auto-refresh fires once against fresh cache. `test.fail` keeps the regression visible without breaking CI. - lifecycle-update-check.test.ts:247 (skip) — ageReleaseCache e2e hook no longer mutates the map main reads from. Also: - e2e/AGENTS.md: codifies the @real contract + skip discipline. - .github/workflows/ci-real.yml: nightly + workflow_dispatch runner for the @real suite (windows-latest, 90 min budget). - test:e2e:real npm script. Validation on this branch: - pnpm run typecheck ✓ - pnpm run lint ✓ - pnpm run build ✓ - pnpm run test:e2e:windows: 88 passed, 4 skipped (the TODOs), 0 failed, ~1.8 min. The pre-existing src/main/lib/release-cache.test.ts vitest collection failure (`__vite-browser-external:fs`) reproduces on origin/main and is unrelated to this PR. Amp-Thread-ID: https://ampcode.com/threads/T-019e6c7e-bdf2-717b-be9f-156eb68dd55a Co-authored-by: Amp <amp@ampcode.com>
Adds e2e/support/realPrereqs.ts exposing freshInstallStandaloneCpu, comfyFrontendIsLoaded, launchComfyByClickingTile, returnToDashboardViaFileMenu, and ensureInstalledAndLaunched. All helpers drive prereqs via real DOM clicks against the visible webContents (panel / title bar / popup) — no \__e2e.*\ mutations, no \window.api.*\ writes, no synthetic IPC. Mirrors the @real contract codified in e2e/AGENTS.md. e2e/lifecycle.test.ts changes: - Replace the bespoke beforeAll hydration block with ensureInstalledAndLaunched(ctx) when LIFECYCLE_REUSE_DIR points at an existing install. Module-level _updateInstallId / _updateInstallPath / _comfyUIDir / _installedCommit are populated from the returned HydratedInstall; read-only snapshot rehydration stays as-is. - Replace every returnFirstInstallHostToDashboard(ctx.app) backdoor call (4 sites) with returnToDashboardViaFileMenu(ctx), which opens the title-bar file menu and clicks the 'Return to Dashboard' item. - Add prereq launch safety to running-comfy-dependent tests (stop-before-update, re-launch-after-update, snapshot capture, cross-channel update, snapshot restore, compact-row Restart, pin-bottom Restart, pin-bottom Copy, stop-before-delete) so each can be greped individually against a hydrated profile. - Mark every window.api.runAction(...) / window.api.openInstancePicker(...) SUT-call shortcut with // TODO(#621-phase3) — those remain as Phase 3 follow-ups. - Remove unused returnFirstInstallHostToDashboard import from devHooks and unused clickInstallTile import from chooserHelpers. Amp-Thread-ID: https://ampcode.com/threads/T-019e6d01-75c8-709e-a16b-1ede7cd787a7 Co-authored-by: Amp <amp@ampcode.com>
…pi.* shortcuts (#621) Amp-Thread-ID: https://ampcode.com/threads/T-019e6d1c-7bef-77f0-8d06-61081f50790f Co-authored-by: Amp <amp@ampcode.com>
…pi.* shortcuts (#621) Amp-Thread-ID: https://ampcode.com/threads/T-019e6d4c-3a88-77fb-9ffc-6fcb83d0bd80 Co-authored-by: Amp <amp@ampcode.com>
…unskip 4 broken @ci tests (#621) Amp-Thread-ID: https://ampcode.com/threads/T-019e6d4c-3a88-77fb-9ffc-6fcb83d0bd80 Co-authored-by: Amp <amp@ampcode.com>
…cker hop) (#621) Amp-Thread-ID: https://ampcode.com/threads/T-019e6d4c-3a88-77fb-9ffc-6fcb83d0bd80 Co-authored-by: Amp <amp@ampcode.com>
returnToDashboard funnels through consultPanelRendererReturnToDashboard, which short-circuits 'cleared' whenever the panelView is missing or its webContents is destroyed. The chooser-pick attach in index.ts:393 calls destroyPanelView(claimed), so until the user opens Settings or the lifecycle body the install-backed panel has no webContents — meaning the short-circuit also swallowed the 'Stop & Return' confirm that lives in PanelApp.vue's onReturnToDashboardRequest handler. Clicking the picker Home icon (or File > Return to Dashboard) on a running local install tore the ComfyUI session down silently. When the panelView is alive the existing renderer consult still owns the prompt (it also layers Tier 2/3 overlay cancel-prompts on top); when it's gone, fall back to a shell system modal with the same 'Stop & Return' copy. The fallback mirrors useReturnToDashboardConfirm's gates: non-local installs and already-stopped sessions clear silently. Test side: returnToDashboardViaPickerHome now polls for the BaseAlert confirm on either ctx.panel or the comfySystemModal popup, since the WebContentsPage.exists call throws while the webContents doesn't exist yet. Issue #621. Amp-Thread-ID: https://ampcode.com/threads/T-019e6d78-cf01-710a-821c-981fca2e7c22 Co-authored-by: Amp <amp@ampcode.com>
…nd updates it Adds the missing @real coverage for the per-channel copy-update action. Drives the same picker ChannelPicker surface as the existing cross-channel update-comfyui @real test, but invokes the copy-update sibling — drafts latest, clicks update-action-copy-update, fills the new-install-name prompt, waits for the ~500MB copy + git update on the copy + the open-install-window IPC for the new install. Asserts: - New install's ComfyUI HEAD moved past the source HEAD (cross-channel copy-update bumps the copy to master tip). - Source HEAD did NOT move (copy-update writes only to the copy). - New install record.updateChannel = latest; source stays on stable. - run-action IPC carried actionData.channel=latest (same Vue-reactive- proxy deep-clone bug class the cross-channel update-comfyui test pins). - Disk shape on both installs (full standalone tree on the copy, source untouched). Sequenced before the cross-channel update-comfyui test so the source is still on stable when both run — same-channel copy-update would resolve to 'no update available' since the prior update-comfyui test already pushed the source to the latest stable tag. Paired cleanup test deletes the ~500MB copy before the cross-channel update test runs so disk doesn't accumulate. Issue #621. Amp-Thread-ID: https://ampcode.com/threads/T-019e6d78-cf01-710a-821c-981fca2e7c22 Co-authored-by: Amp <amp@ampcode.com>
Picker-driven mutating ops (copy, copy-update, delete, release-update, launch, ...) invoked via comfy-titlepopup:start-background-op were calling handleDelegateToSource directly. Session-level action ids aren't registered in any source's handleAction, so they failed with 'Action not yet implemented' before reaching the real handler. Extract the existing run-action switch into dispatchSessionAction and call it from both the run-action IPC handler and the picker's background-op stub. Session ids land on their handlers; source-specific ids (update-comfyui, snapshot-restore, ...) still fall through to the source via handleDelegateToSource. Amp-Thread-ID: https://ampcode.com/threads/T-019e6dc6-0f96-7659-9107-0864be5cbe04 Co-authored-by: Amp <amp@ampcode.com>
BasePrompt is the surface useDialogs().prompt() routes through (the picker's Copy / Copy & Update / Snapshot Save name prompts). Its testids were string literals; promote them to the shared TID family so @real tests can target them without duplicating selectors and so they're discoverable next to the modalPrompt* (legacy ModalDialog) counterparts. Amp-Thread-ID: https://ampcode.com/threads/T-019e6dc6-0f96-7659-9107-0864be5cbe04 Co-authored-by: Amp <amp@ampcode.com>
Three related @real fixes: - cross-channel copy-update test now drives the basePrompt* inputs (picker uses dialogs.prompt → BasePrompt, not the legacy ModalDialog) and reads run-action from comfy-titlepopup:start-background-op since copy-update routes through inline-picker progress, not the open-install-window branch. Adds standalone-rehydration setup to the cleanup test so it works under --grep. - picker pin-bottom Copy + dashboard kebab Copy tests switch from modalPromptInput/modalConfirm to basePromptInput/basePromptAction for the same reason. - captureInstallStateFromDisk: when multiple installs match, prefer ones whose disk state is actually launchable (ComfyUI/main.py + .venv/Scripts/python.exe). Skips silently hydrating a partial-delete leftover from a prior run. - ensureInstalledAndLaunched: detects unlaunchable hydration and falls through to a fresh install with a [lifecycle-harness] log line, preventing the cryptic 'no Python environment found' timeout. Amp-Thread-ID: https://ampcode.com/threads/T-019e6dc6-0f96-7659-9107-0864be5cbe04 Co-authored-by: Amp <amp@ampcode.com>
vitest 4.x's happy-dom environment (the global default) resolves bare
'fs' through __vite-browser-external, so vi.mock('fs') fails before
any test runs with 'argument filename must be a file URL object'.
Two targeted fixes:
- Add // @vitest-environment node so this single file gets the node
runner without changing the global default that happy-dom-using
Vue component tests need.
- Mock ./safe-file so set() doesn't flush a release-cache.json into
the repo cwd (dataDir() is '' under the mocked Electron app).
Amp-Thread-ID: https://ampcode.com/threads/T-019e6dc6-0f96-7659-9107-0864be5cbe04
Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019e6e8e-e90f-720a-b3b2-d2f392a77a04 Co-authored-by: Amp <amp@ampcode.com> # Conflicts: # e2e/update-pills.test.ts # src/main/lib/release-cache.test.ts # src/renderer/src/components/ui/BasePrompt.vue
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
1 participant
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.
Closes #621 (partial - Phase 1 of the plan).
What this PR does
Retires the overloaded
@lifecycletag and replaces it with two explicit, independent tiers:@ci- every-PR matrix (Windows + macOS + Linux). Seeded harness,__e2ebackdoor,window.apibridge, fixtures on disk all allowed. Fast, deterministic, covers renderer + IPC contracts.@real- real lifecycle.pnpm run test:e2e:real. No seeds, no__e2emutations, nowindow.apishortcuts, no fake fixture installs. Behaves as if a human had booted the app and clicked through the UI.OS axis stays independent:
@cidefaults to all three OSes;@windows-only/@macos-only/@linux-onlyopt out when behavior is genuinely platform-specific.quit-flow.spec.tsis the lone opt-out today.See
e2e/AGENTS.mdfor the contract.Tag inventory
@real:lifecycle.test.ts(29-test install/update/snapshot/copy/delete chain),lifecycle-cloud.test.ts.@ci(1 with@macos-only).Broken tests
Four
@citests are currently broken; each is marked with aTODO(#621)comment so they stay visible and don't silently rot:picker-stop-confirm.test.ts:159skipupdate-comfyuinever reachesrun-action.lifecycle-snapshot-restore.test.ts:274skiplifecycle-update-check.test.ts:218failtest.failkeeps the regression visible without breaking CI.lifecycle-update-check.test.ts:247skipageReleaseCachee2e hook no longer mutates the map main reads from.Follow-up PRs address each. This PR is intentionally rename-only.
CI
test:e2e:{windows,macos,linux}) is unchanged in scope but now driven bygrep: /@ci/with OS opt-out exclusions..github/workflows/ci-real.ymlworkflow runs@realonworkflow_dispatch+ nightly cron (09:00 UTC) onwindows-latestwith a 90-min budget.Validation
pnpm run typecheck?pnpm run lint?pnpm run build?pnpm run test:e2e:windows: 88 passed, 4 skipped (the TODOs), 0 failed, ~1.8 min.The pre-existing
src/main/lib/release-cache.test.tsvitest collection failure (__vite-browser-external:fs) reproduces onorigin/mainand is unrelated to this PR.