diff --git a/.github/workflows/snapshot-tests.yml b/.github/workflows/snapshot-tests.yml index faf9c01e97..f561467857 100644 --- a/.github/workflows/snapshot-tests.yml +++ b/.github/workflows/snapshot-tests.yml @@ -14,7 +14,10 @@ on: default: false concurrency: - group: snapshot-tests-${{ github.workflow }}-${{ github.ref }} + # One run at a time per PR — a new commit cancels the in-flight run. + # Falls back to github.ref for main pushes and manual dispatch so those + # don't interfere with each other or with PR runs. + group: snapshot-tests-${{ github.event.pull_request.number || github.ref }} cancel-in-progress: true permissions: @@ -134,6 +137,10 @@ jobs: echo "Found snapshot-baselines artifact from run $RUN_ID" fi + - name: Clear snapshot directory before downloading baselines + if: github.event_name == 'pull_request' + run: rm -rf tests/e2e/__snapshots__/ + - name: Download main-branch baselines if: github.event_name == 'pull_request' && steps.find-run.outputs.has_baselines == 'true' uses: actions/download-artifact@v4 @@ -176,10 +183,13 @@ jobs: - name: Generate current PR snapshots if: github.event_name == 'pull_request' + id: generate run: npm run test:e2e:snapshots:update + continue-on-error: true - name: Post snapshot report to PR - if: github.event_name == 'pull_request' + if: always() && github.event_name == 'pull_request' + id: post-comment env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_NUMBER: ${{ github.event.pull_request.number }} @@ -189,6 +199,8 @@ jobs: MAIN_BASELINES_DIR: /tmp/main-baselines COMPARISON_RESULTS_DIR: /tmp/comparison-results SNAPSHOTS_APPROVED: ${{ contains(github.event.pull_request.labels.*.name, 'update-snapshots') }} + GENERATE_OUTCOME: ${{ steps.generate.outcome }} + COMPARE_OUTCOME: ${{ steps.compare.outcome }} run: node tests/e2e/snapshots/scripts/post-snapshot-comment.mjs - name: Upload test results artifact @@ -202,11 +214,22 @@ jobs: retention-days: 7 - name: Fail if snapshot comparison found differences + # Only fail on real pixel-diff failures (changed snapshots), not on + # missing-baseline failures which are expected for new tests added in + # this PR. `has_changes` is written by post-snapshot-comment.mjs to + # $GITHUB_OUTPUT; it is false when all failures are "new" snapshots. if: >- github.event_name == 'pull_request' && - steps.compare.outcome == 'failure' && - steps.find-run.outputs.has_baselines == 'true' && + steps.post-comment.outputs.has_changes == 'true' && !contains(github.event.pull_request.labels.*.name, 'update-snapshots') run: | echo "::error::Snapshot differences detected. Add the 'update-snapshots' label to acknowledge intentional changes." exit 1 + + - name: Fail if snapshot generation had test crashes + if: >- + github.event_name == 'pull_request' && + steps.generate.outcome == 'failure' + run: | + echo "::error::One or more snapshot tests crashed during generation. See the PR comment and CI logs for details." + exit 1 diff --git a/__tests__/api/mock-conversation-handlers.test.ts b/__tests__/api/mock-conversation-handlers.test.ts index 29c526d270..8c9d530df9 100644 --- a/__tests__/api/mock-conversation-handlers.test.ts +++ b/__tests__/api/mock-conversation-handlers.test.ts @@ -22,13 +22,22 @@ describe("mock conversation handlers", () => { expect(page.items[0]?.title).toBeTruthy(); }); - it("returns empty git changes for mock conversations", async () => { + it("returns pre-seeded git changes for mock conversations", async () => { + // MOCK_GIT_CHANGES is pre-seeded in git-repository-handlers.ts with three + // representative entries (UPDATED→M, ADDED→A, DELETED→D) so E2E snapshot + // tests can exercise the full diff-viewer UI without per-test manipulation. const changes = await AgentServerGitService.getGitChanges( "http://localhost:3000/api/conversations/1", null, "workspace/project", ); - expect(changes).toEqual([]); + expect(changes).toHaveLength(3); + expect(changes.map((c) => c.status)).toEqual(["M", "A", "D"]); + expect(changes.map((c) => c.path)).toEqual([ + "src/components/hello.tsx", + "src/utils/new-helper.ts", + "src/old-module.py", + ]); }); }); diff --git a/src/components/features/backends/backend-form-modal.tsx b/src/components/features/backends/backend-form-modal.tsx index 94a7b57bb2..bdc9f5f055 100644 --- a/src/components/features/backends/backend-form-modal.tsx +++ b/src/components/features/backends/backend-form-modal.tsx @@ -30,11 +30,77 @@ function inferKindFromHost(host: string): BackendKind { return "local"; } +/** + * Returns true for hostnames that represent a local / private-network address. + * Used by normalizeHost to choose http:// instead of https://. + */ +function isLocalAddress(hostname: string): boolean { + // Strip IPv6 bracket notation: [::1] → ::1 + const h = hostname.toLowerCase().replace(/^\[|\]$/g, ""); + // IPv6 loopback, any-address, and named loopback + if (h === "localhost" || h === "::1" || h === "::" || h === "0.0.0.0") + return true; + // 127.x.x.x loopback range + IPv4-mapped loopback (::ffff:127.x.x.x) + if (/^127\./.test(h) || /^::ffff:127\./i.test(h)) return true; + // RFC 1918 private ranges + if (/^10\./.test(h)) return true; + if (/^192\.168\./.test(h)) return true; + if (/^172\.(1[6-9]|2\d|3[01])\./.test(h)) return true; + // IPv6 link-local (fe80::/10) and unique local (fc00::/7) + if (/^fe[89ab][0-9a-f]:/i.test(h)) return true; + if (/^f[cd][0-9a-f]{2}:/i.test(h)) return true; + // mDNS / Bonjour (.local) + if (h.endsWith(".local")) return true; + // Single-label hostnames (no dots, no colons) are local network names. + // Colons are excluded so bare IPv6 addresses don't accidentally match. + if (!h.includes(".") && !h.includes(":")) return true; + return false; +} + function normalizeHost(host: string): string { const trimmed = host.trim().replace(/\/+$/, ""); if (!trimmed) return ""; + // Already has an explicit scheme — respect it. if (/^https?:\/\//i.test(trimmed)) return trimmed; - return `https://${trimmed}`; + // Extract the pure hostname for scheme selection, handling three cases: + // [::1]:8080 → bracket IPv6 notation → extract ::1 + // ::1 → bare IPv6 (multiple colons, no bracket) → whole string + // host:port → regular host:port → part before the colon + const bracketMatch = trimmed.match(/^\[([^\]]+)\]/); + const hostname = bracketMatch + ? bracketMatch[1] + : (trimmed.match(/:/g) ?? []).length > 1 + ? trimmed + : trimmed.split(":")[0]; + const scheme = isLocalAddress(hostname) ? "http" : "https"; + return `${scheme}://${trimmed}`; +} + +/** + * Returns true when `host` represents a reachable backend URL. + * + * Rules (applied in order): + * 1. Must be non-empty after trimming. + * 2. Must contain no whitespace — spaces can never appear in a host/port. + * 3. After normalisation (bare hosts get `https://` prepended), must parse + * as a valid http or https URL with a non-empty hostname. + */ +function isValidHostUrl(host: string): boolean { + const trimmed = host.trim(); + if (!trimmed) return false; + // Spaces anywhere in the input are an immediate rejection. + if (/\s/.test(trimmed)) return false; + const normalized = normalizeHost(trimmed); + if (!normalized) return false; + try { + const url = new URL(normalized); + return ( + (url.protocol === "http:" || url.protocol === "https:") && + url.hostname.length > 0 + ); + } catch { + return false; + } } /** @@ -194,6 +260,10 @@ export function BackendForm({ // already chose one, so don't re-infer over their choice. const [touchedKind, setTouchedKind] = React.useState(mode === "edit"); + // Inline validation: only show errors after the user has left a field. + const [nameTouched, setNameTouched] = React.useState(false); + const [hostTouched, setHostTouched] = React.useState(false); + // Auto-infer kind from host when user hasn't explicitly selected a kind via radio React.useEffect(() => { if (!touchedKind && host) { @@ -206,12 +276,29 @@ export function BackendForm({ const canSubmit = name.trim().length > 0 && - host.trim().length > 0 && + isValidHostUrl(host) && (kind === "local" || apiKey.trim().length > 0); + // Error messages — only surfaced after the user has blurred the field. + const nameError = + nameTouched && !name.trim() ? t(I18nKey.BACKEND$NAME_REQUIRED) : undefined; + const hostError = hostTouched + ? !host.trim() + ? t(I18nKey.BACKEND$HOST_REQUIRED) + : !isValidHostUrl(host) + ? t(I18nKey.BACKEND$HOST_INVALID) + : undefined + : undefined; + const handleSubmit = (event: React.FormEvent) => { event.preventDefault(); - if (!canSubmit) return; + if (!canSubmit) { + // Mark all validated fields as touched so inline errors become visible + // (e.g. user pressed Enter before filling required fields). + setNameTouched(true); + setHostTouched(true); + return; + } const payload = { name: name.trim(), @@ -248,8 +335,11 @@ export function BackendForm({ label={t(I18nKey.BACKEND$NAME_LABEL)} value={name} onChange={setName} + onBlur={() => setNameTouched(true)} placeholder="Production" className="w-full" + showRequiredTag + error={nameError} /> setHostTouched(true)} placeholder="https://app.all-hands.dev" className="w-full" + showRequiredTag + error={hostError} /> {/* Device Flow auth for cloud backends in add mode - always visible */} @@ -270,7 +363,7 @@ export function BackendForm({ host={host} onSuccess={setApiKey} testIdRoot={testIdRoot} - isDisabled={host.trim().length === 0} + isDisabled={!name.trim() || !isValidHostUrl(host)} /> {/* Divider with "or" */} diff --git a/src/components/features/settings/brand-button.tsx b/src/components/features/settings/brand-button.tsx index 48ddaff670..53794d3f66 100644 --- a/src/components/features/settings/brand-button.tsx +++ b/src/components/features/settings/brand-button.tsx @@ -48,7 +48,13 @@ export const BrandButton = forwardRef< aria-label={ariaLabel} aria-busy={ariaBusy} className={cn( - "w-fit p-2 text-sm rounded-sm disabled:opacity-30 disabled:cursor-not-allowed cursor-pointer", + "w-fit p-2 text-sm rounded-sm cursor-pointer", + // Apply disabled appearance via conditional class so it works + // regardless of whether the :disabled pseudo-class is available + // (e.g. Tailwind v4 + postcss-prefix-selector in dev mode). + isDisabled + ? "opacity-30 cursor-not-allowed pointer-events-none" + : "disabled:opacity-30 disabled:cursor-not-allowed", variant === "primary" && "bg-primary text-[var(--oh-color-base)] hover:opacity-80", variant === "secondary" && diff --git a/src/components/features/settings/settings-input.tsx b/src/components/features/settings/settings-input.tsx index d99e324b93..71f0f8f444 100644 --- a/src/components/features/settings/settings-input.tsx +++ b/src/components/features/settings/settings-input.tsx @@ -28,6 +28,14 @@ interface SettingsInputProps { ariaDescribedBy?: string; /** ARIA invalid attribute for accessibility */ ariaInvalid?: boolean; + /** + * Validation error message. When set, the input gets a red border and + * the message is rendered below it. Also sets aria-invalid automatically. + */ + error?: string; + /** Renders a red asterisk next to the label to mark the field as required. */ + showRequiredTag?: boolean; + onBlur?: () => void; } export const SettingsInput = forwardRef( @@ -55,14 +63,23 @@ export const SettingsInput = forwardRef( labelClassName, ariaDescribedBy, ariaInvalid, + error, + showRequiredTag, + onBlur, }, ref, ) { + const errorId = error && testId ? `${testId}-error` : undefined; return ( ); }, diff --git a/src/i18n/translation.json b/src/i18n/translation.json index c322cbd9cc..4385437d3b 100644 --- a/src/i18n/translation.json +++ b/src/i18n/translation.json @@ -25787,5 +25787,14 @@ "tr": "Çalışma alanınıza eklenecek yetenekleri keşfedin. İstemler, curl ve yükleme akışları için bir kart açın. Listeyi filtrelemek için kenar çubuğundan arama yapın. Varsayılan yetenekleri etkinleştirin veya devre dışı bırakın. Devre dışı bırakılan yetenekler ajan bağlamına yüklenmez.", "uk": "Відкривайте навички для додавання до вашого робочого простору. Відкрийте картку для запитів, curl та процесів встановлення. Шукайте на бічній панелі, щоб фільтрувати список. Увімкніть або вимкніть навички за замовчуванням. Вимкнені навички не завантажуватимуться в контекст агента.", "ca": "Descobreix habilitats per afegir al teu espai de treball. Obre una targeta per veure indicacions, curl i fluxos d'instal·lació. Cerca des de la barra lateral per filtrar la llista. Activeu o desactiveu les habilitats predeterminades. Les habilitats desactivades no es carregaran al context de l'agent." + }, + "BACKEND$NAME_REQUIRED": { + "en": "Name is required" + }, + "BACKEND$HOST_REQUIRED": { + "en": "Host is required" + }, + "BACKEND$HOST_INVALID": { + "en": "Enter a valid URL (e.g. http://localhost:8080)" } } diff --git a/src/mocks/git-repository-handlers.ts b/src/mocks/git-repository-handlers.ts index b1e58cd36a..9cbb73426c 100644 --- a/src/mocks/git-repository-handlers.ts +++ b/src/mocks/git-repository-handlers.ts @@ -43,6 +43,39 @@ const MOCK_REPOSITORIES = { // Mock branches (same for all repos for simplicity) const MOCK_BRANCHES = generateMockBranches(25); +// ── Git workspace test helpers ──────────────────────────────────────────────── +// MSW 2.x browser-mode handlers run in the page's main thread (PAGE-level JS), +// not in the service worker. Module-level state is therefore shared across all +// requests in the same page session — the same pattern as the `automations` Map. +// +// Pre-seeded with one modified, one added, and one deleted file so snapshot +// tests can exercise the full file-list / diff-editor / deleted-file-placeholder +// UI without any per-test MSW manipulation. +// +// Playwright specs that need a different set of changes (e.g. empty state) can: +// 1. Call window.__setMockGitChanges__([]) via page.evaluate() after boot. +// 2. Trigger a refetch via window.__TEST_INVALIDATE_QUERIES__?.() + +// Three representative files: modified, added, deleted. +// Uses AgentServerGitChangeStatus values ("UPDATED", "ADDED", "DELETED") as +// returned by the real /api/git/changes endpoint; AgentServerGitService maps +// them via mapAnyGitStatusToClientStatus before handing them to the UI. +export let MOCK_GIT_CHANGES: Array<{ path: string; status: string }> = [ + { path: "src/components/hello.tsx", status: "UPDATED" }, + { path: "src/utils/new-helper.ts", status: "ADDED" }, + { path: "src/old-module.py", status: "DELETED" }, +]; + +export const setMockGitChanges = (changes: typeof MOCK_GIT_CHANGES): void => { + MOCK_GIT_CHANGES = changes; +}; + +// Expose on window so Playwright specs can call it via page.evaluate(). +if (typeof window !== "undefined") { + (window as unknown as Record).__setMockGitChanges__ = + setMockGitChanges; +} + export const GIT_REPOSITORY_HANDLERS = [ http.get("*/api/user/repositories", async ({ request }) => { await delay(500); // Simulate network delay @@ -230,9 +263,23 @@ export const GIT_REPOSITORY_HANDLERS = [ return HttpResponse.json(limitedBranches); }), - http.get("*/api/git/changes", async () => HttpResponse.json([])), + // Pre-seeded git changes so snapshot tests can exercise the file list, + // the Monaco diff editor, and the deleted-file placeholder without any + // per-test MSW manipulation. + // + // Tests that need to override this (e.g. empty-state test) can call + // window.__setMockGitChanges__([]) + // via page.evaluate() after the app has loaded, then trigger a refetch + // via window.__TEST_INVALIDATE_QUERIES__?.() + http.get("*/api/git/changes", async () => + HttpResponse.json(MOCK_GIT_CHANGES), + ), http.get("*/api/git/diff", async () => - HttpResponse.json({ original: "", modified: "" }), + HttpResponse.json({ + original: 'def greet(name):\n return f"Hello, {name}!"\n', + modified: + 'def greet(name: str) -> str:\n return f"Hello, {name}! Welcome."\n', + }), ), ]; diff --git a/src/mocks/handlers.ts b/src/mocks/handlers.ts index 865343fa51..bebb5edf92 100644 --- a/src/mocks/handlers.ts +++ b/src/mocks/handlers.ts @@ -1,7 +1,10 @@ import { FILE_SERVICE_HANDLERS } from "./file-service-handlers"; import { TASK_SUGGESTIONS_HANDLERS } from "./task-suggestions-handlers"; import { SECRETS_HANDLERS } from "./secrets-handlers"; -import { GIT_REPOSITORY_HANDLERS } from "./git-repository-handlers"; +import { + GIT_REPOSITORY_HANDLERS, + setMockGitChanges, +} from "./git-repository-handlers"; import { SETTINGS_HANDLERS, MOCK_DEFAULT_USER_SETTINGS, @@ -33,4 +36,5 @@ export { MOCK_DEFAULT_USER_SETTINGS, resetTestHandlersMockSettings, resetAutomationMockData, + setMockGitChanges, }; diff --git a/tests/e2e/snapshots/backends-extended.snapshot.spec.ts b/tests/e2e/snapshots/backends-extended.snapshot.spec.ts new file mode 100644 index 0000000000..46e3fc1496 --- /dev/null +++ b/tests/e2e/snapshots/backends-extended.snapshot.spec.ts @@ -0,0 +1,570 @@ +import { test, expect, type Page } from "@playwright/test"; +import type { Backend } from "../../../src/api/backend-registry/types"; + +/** + * Extended visual snapshot tests for the backend management UI. + * + * These tests exercise the full lifecycle of backend CRUD operations with + * iterative screenshot captures at each meaningful state transition: + * + * Flow 1 — Add form validation gates + * Form is disabled until both "Host Name" and "Host URL" are filled; + * cloud backends additionally require an API key. + * + * Flow 2 — Kind auto-inference (host → type) + * Typing a cloud-domain URL (all-hands.dev) auto-selects Cloud and + * shows the device-flow OAuth section. A local URL flips back to Local + * and hides OAuth. Manually selecting a kind stops auto-inference. + * + * Flow 3 — Cloud OAuth button gated by host + * The "Login with OpenHands" button is disabled when the host field is + * empty so the user can't start a device-flow with nowhere to point it. + * + * Flow 4 — Remove backend with confirmation step + * Clicking "Remove" opens a ConfirmationModal; confirming removes the + * row; cancelling keeps it. + * + * Flow 5 — Edit backend pre-fills form fields + * Opening the edit modal for an existing backend populates name, host, + * and API-key inputs from the stored backend data. + * + * Flow 6 — Switch active backend via dropdown + * Selecting a different backend fires the environment-switch overlay, + * then updates the selector trigger label once the overlay fades. + * + * Flow 7 — Malformed / empty host blocks submission + * A host with only whitespace keeps the Submit button disabled. + * A syntactically invalid URL (e.g. containing spaces or a garbled + * scheme) also keeps Save disabled — isValidHostUrl() rejects it at + * the form level before normalisation can make it look superficially + * valid to the URL constructor. + * + * Flow 8 — Cancel add form dismisses without saving + * Clicking Cancel closes the modal without altering the backend list. + */ + +// ── Constants ────────────────────────────────────────────────────────────── + +/** Two pre-seeded backends used by multi-backend tests. */ +const LOCAL_BACKEND: Backend = { + id: "default-local", + name: "Local", + host: "http://localhost:3000", + apiKey: "", + kind: "local", +}; + +const CLOUD_BACKEND: Backend = { + id: "test-production", + name: "Production", + host: "https://app.all-hands.dev", + apiKey: "sk-test-key", + kind: "cloud", +}; + +// ── Helpers ──────────────────────────────────────────────────────────────── + +/** + * Seed localStorage with one or two backends and navigate to the + * conversations list so the BackendSelector is visible in the sidebar. + * Routes file API and cloud-proxy requests so they don't produce + * console errors that could affect timing. + */ +async function setupPage( + page: Page, + { + backends = [LOCAL_BACKEND], + activeBackendId, + }: { backends?: Backend[]; activeBackendId?: string } = {}, +) { + await page.addInitScript( + ({ backendsJson, activeJson }: { backendsJson: string; activeJson: string | null }) => { + window.localStorage.setItem("openhands-onboarded", "true"); + window.localStorage.setItem("openhands-backends", backendsJson); + if (activeJson) { + window.localStorage.setItem("openhands-active-backend", activeJson); + } + }, + { + backendsJson: JSON.stringify(backends), + activeJson: activeBackendId + ? JSON.stringify({ backendId: activeBackendId, orgId: null }) + : null, + }, + ); + + // Prevent workspace-scan 404s in the sidebar from cluttering timing. + await page.route("**/api/file/**", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ path: "/home", subdirs: [] }), + }), + ); +} + +async function dismissConsentModal(page: Page) { + await page + .getByRole("button", { name: "Confirm preferences" }) + .click({ timeout: 3_000 }) + .catch(() => undefined); +} + +/** + * Navigate to the conversations list and hover the backend selector to + * open the dropdown. Returns the root-layout locator for snapshots. + */ +async function openDropdown(page: Page) { + await page.goto("/conversations"); + await dismissConsentModal(page); + await page.waitForLoadState("networkidle"); + + const rootLayout = page.getByTestId("root-layout"); + await expect(rootLayout).toBeVisible({ timeout: 15_000 }); + + const selector = page.getByTestId("backend-selector"); + await expect(selector).toBeVisible({ timeout: 10_000 }); + await selector.hover(); + + await expect(page.getByTestId("add-backend-menu-item")).toBeVisible({ + timeout: 5_000, + }); + + return rootLayout; +} + +/** Open the Add Backend modal via the dropdown footer. */ +async function openAddModal(page: Page) { + const rootLayout = await openDropdown(page); + await page.getByTestId("add-backend-menu-item").click(); + await expect(page.getByTestId("add-backend-modal")).toBeVisible({ + timeout: 5_000, + }); + await expect(page.getByTestId("add-backend-name")).toBeVisible({ + timeout: 5_000, + }); + return rootLayout; +} + +/** Open the Manage Backends modal via the dropdown footer. */ +async function openManageModal(page: Page) { + const rootLayout = await openDropdown(page); + await page.getByTestId("manage-backends-menu-item").click(); + await expect(page.getByTestId("manage-backends-modal")).toBeVisible({ + timeout: 5_000, + }); + return rootLayout; +} + +const SNAP_OPTS = { animations: "disabled" as const, maxDiffPixelRatio: 0.01 }; + +// ── Test Suite ───────────────────────────────────────────────────────────── + +test.describe("Backend Management — Extended Flow Snapshots", () => { + test.setTimeout(90_000); + + // ── Flow 1: Add-form validation gates ───────────────────────────────── + + test("Flow 1a — add form blank: Save button disabled until required fields filled", async ({ + page, + }) => { + await setupPage(page); + const rootLayout = await openAddModal(page); + + // 1. Completely blank form — Save must be disabled. + await expect(page.getByTestId("add-backend-submit")).toBeDisabled(); + await expect(rootLayout).toHaveScreenshot( + "backend-add-blank-disabled.png", + SNAP_OPTS, + ); + + // 2. Fill only the name; host still empty → Save still disabled. + // Focus + blur the host field to reveal the "Host is required" error. + await page.getByTestId("add-backend-name").fill("My Backend"); + await page.getByTestId("add-backend-host").focus(); + await page.getByTestId("add-backend-host").blur(); + await expect(page.getByTestId("add-backend-submit")).toBeDisabled(); + await expect(rootLayout).toHaveScreenshot( + "backend-add-name-only-disabled.png", + SNAP_OPTS, + ); + }); + + test("Flow 1b — local backend becomes Save-ready with name + host, no API key required", async ({ + page, + }) => { + await setupPage(page); + const rootLayout = await openAddModal(page); + + // Switch to Local type first so API key is not required. + await page.getByTestId("add-backend-kind-local").click(); + await page.getByTestId("add-backend-name").fill("Dev Server"); + await page.getByTestId("add-backend-host").fill("http://localhost:8080"); + + // API key left empty — Save must be enabled for local kind. + await expect(page.getByTestId("add-backend-submit")).not.toBeDisabled(); + await expect(rootLayout).toHaveScreenshot( + "backend-add-local-ready.png", + SNAP_OPTS, + ); + }); + + test("Flow 1c — cloud backend requires API key; Save stays disabled without it", async ({ + page, + }) => { + await setupPage(page); + const rootLayout = await openAddModal(page); + + // Add form starts in Cloud mode; type a cloud URL to confirm. + await page.getByTestId("add-backend-name").fill("Cloud Prod"); + await page.getByTestId("add-backend-host").fill("https://app.all-hands.dev"); + + // Cloud radio should now be selected (auto-inferred from domain). + await expect(page.getByTestId("add-backend-kind-cloud")).toBeChecked(); + + // No API key → Save disabled. + await expect(page.getByTestId("add-backend-submit")).toBeDisabled(); + await expect(rootLayout).toHaveScreenshot( + "backend-add-cloud-no-key-disabled.png", + SNAP_OPTS, + ); + + // Fill API key → Save enabled. + await page.getByTestId("add-backend-api-key").fill("sk-live-abc123"); + await expect(page.getByTestId("add-backend-submit")).not.toBeDisabled(); + await expect(rootLayout).toHaveScreenshot( + "backend-add-cloud-with-key-enabled.png", + SNAP_OPTS, + ); + }); + + // ── Flow 2: Kind auto-inference + manual override ───────────────────── + + test("Flow 2a — typing a local URL auto-infers Local kind and hides OAuth section", async ({ + page, + }) => { + await setupPage(page); + const rootLayout = await openAddModal(page); + + // Initially cloud (add-mode default). + await expect(page.getByTestId("add-backend-kind-cloud")).toBeChecked(); + + // Type a localhost URL → should flip to Local, hiding device-flow. + await page.getByTestId("add-backend-host").fill("localhost:8888"); + await expect(page.getByTestId("add-backend-kind-local")).toBeChecked({ + timeout: 3_000, + }); + // Device-flow section disappears for local kind. + await expect(page.getByTestId("add-backend-device-flow")).not.toBeVisible(); + await expect(rootLayout).toHaveScreenshot( + "backend-add-kind-local-inferred.png", + SNAP_OPTS, + ); + }); + + test("Flow 2b — typing a cloud URL keeps Cloud kind and shows OAuth section", async ({ + page, + }) => { + await setupPage(page); + const rootLayout = await openAddModal(page); + + // Type an all-hands.dev URL → stays/becomes Cloud, shows device-flow. + await page.getByTestId("add-backend-host").fill("https://app.all-hands.dev"); + await expect(page.getByTestId("add-backend-kind-cloud")).toBeChecked({ + timeout: 3_000, + }); + await expect(page.getByTestId("add-backend-device-flow")).toBeVisible(); + await expect(rootLayout).toHaveScreenshot( + "backend-add-kind-cloud-inferred.png", + SNAP_OPTS, + ); + }); + + test("Flow 2c — manually selecting Local locks the kind even when a cloud URL is typed", async ({ + page, + }) => { + await setupPage(page); + const rootLayout = await openAddModal(page); + + // Explicitly click Local radio → touchedKind = true. + await page.getByTestId("add-backend-kind-local").click(); + await expect(page.getByTestId("add-backend-kind-local")).toBeChecked(); + + // Now type a cloud URL — kind must STAY local (manual override). + await page.getByTestId("add-backend-host").fill("https://app.all-hands.dev"); + // Wait a tick for any potential effect to run. + await page.waitForTimeout(200); + await expect(page.getByTestId("add-backend-kind-local")).toBeChecked(); + await expect(page.getByTestId("add-backend-device-flow")).not.toBeVisible(); + await expect(rootLayout).toHaveScreenshot( + "backend-add-manual-override-local.png", + SNAP_OPTS, + ); + }); + + // ── Flow 3: OAuth button gated by host ──────────────────────────────── + + test("Flow 3 — cloud Login button disabled until both name and host are filled", async ({ + page, + }) => { + await setupPage(page); + const rootLayout = await openAddModal(page); + + // Cloud form, host empty → login button disabled. + await expect(page.getByTestId("add-backend-kind-cloud")).toBeChecked(); + await expect(page.getByTestId("add-backend-login-button")).toBeDisabled(); + await expect(rootLayout).toHaveScreenshot( + "backend-add-oauth-button-disabled.png", + SNAP_OPTS, + ); + + // Fill name + host → login button enabled. + await page.getByTestId("add-backend-name").fill("My Cloud"); + await page.getByTestId("add-backend-host").fill("https://app.all-hands.dev"); + await expect(page.getByTestId("add-backend-login-button")).not.toBeDisabled(); + await expect(rootLayout).toHaveScreenshot( + "backend-add-oauth-button-enabled.png", + SNAP_OPTS, + ); + }); + + // ── Flow 4: Remove backend with confirmation ────────────────────────── + + test("Flow 4 — removing a backend: confirmation modal then row disappears", async ({ + page, + }) => { + await setupPage(page, { backends: [LOCAL_BACKEND, CLOUD_BACKEND] }); + const rootLayout = await openManageModal(page); + + // Both backend rows visible. + await expect( + page.getByTestId("manage-backends-row-Local"), + ).toBeVisible(); + await expect( + page.getByTestId("manage-backends-row-Production"), + ).toBeVisible(); + await expect(rootLayout).toHaveScreenshot( + "backend-manage-two-listed.png", + SNAP_OPTS, + ); + + // Click Remove on "Production". + await page.getByTestId("manage-backends-remove-Production").click(); + + // ConfirmationModal should appear with the backend name in the text. + await expect(page.getByTestId("confirmation-modal")).toBeVisible({ + timeout: 5_000, + }); + await expect(rootLayout).toHaveScreenshot( + "backend-remove-confirmation.png", + SNAP_OPTS, + ); + + // Click Cancel — Production row should still be present. + await page.getByTestId("cancel-button").click(); + await expect(page.getByTestId("confirmation-modal")).not.toBeVisible({ + timeout: 3_000, + }); + await expect( + page.getByTestId("manage-backends-row-Production"), + ).toBeVisible(); + await expect(rootLayout).toHaveScreenshot( + "backend-remove-cancelled.png", + SNAP_OPTS, + ); + + // Remove again and CONFIRM this time. + await page.getByTestId("manage-backends-remove-Production").click(); + await expect(page.getByTestId("confirmation-modal")).toBeVisible({ + timeout: 5_000, + }); + await page.getByTestId("confirm-button").click(); + + // Row disappears from the manage list. + await expect( + page.getByTestId("manage-backends-row-Production"), + ).not.toBeVisible({ timeout: 5_000 }); + await expect(rootLayout).toHaveScreenshot( + "backend-manage-after-removal.png", + SNAP_OPTS, + ); + }); + + // ── Flow 5: Edit backend modal pre-fills form ───────────────────────── + + test("Flow 5 — edit modal pre-populates existing backend's name, host and key", async ({ + page, + }) => { + await setupPage(page, { backends: [LOCAL_BACKEND, CLOUD_BACKEND] }); + const rootLayout = await openManageModal(page); + + // Open Edit for the Production backend. + await page.getByTestId("manage-backends-edit-Production").click(); + await expect(page.getByTestId("edit-backend-modal")).toBeVisible({ + timeout: 5_000, + }); + + // Assert the pre-filled values. + await expect(page.getByTestId("edit-backend-name")).toHaveValue( + CLOUD_BACKEND.name, + ); + await expect(page.getByTestId("edit-backend-host")).toHaveValue( + CLOUD_BACKEND.host, + ); + + await expect(rootLayout).toHaveScreenshot( + "backend-edit-prefilled.png", + SNAP_OPTS, + ); + }); + + // ── Flow 6: Switch active backend ──────────────────────────────────── + + test("Flow 6 — switching backends shows environment-switch overlay then updates selector", async ({ + page, + }) => { + // Start with Local active; Production is a second registered backend. + await setupPage(page, { + backends: [LOCAL_BACKEND, CLOUD_BACKEND], + activeBackendId: LOCAL_BACKEND.id, + }); + const rootLayout = await openDropdown(page); + + // Both options should be visible in the open dropdown. + await expect(page.getByRole("option", { name: "Local" })).toBeVisible(); + await expect( + page.getByRole("option", { name: "Production" }), + ).toBeVisible(); + await expect(rootLayout).toHaveScreenshot( + "backend-dropdown-two-backends.png", + SNAP_OPTS, + ); + + // Click Production option — triggers the environment-switch overlay. + // The overlay is rendered via createPortal into document.body, so it + // lives outside the root-layout subtree. Use a full-page screenshot + // to capture it reliably. + // + // body[data-environment-switching="true"] is set synchronously inside + // triggerEnvironmentSwitch before any React re-render, giving us a + // stable early signal that the overlay is imminent even before React + // paints the portal div. + await page.getByRole("option", { name: "Production" }).click(); + await page.waitForSelector('body[data-environment-switching="true"]', { + timeout: 2_000, + }); + // Now wait for the actual portal div (React needs one render tick). + await page.waitForSelector('[data-testid="environment-switch-overlay"]', { + timeout: 2_000, + }); + + // The overlay card animates from opacity:0 → 1 over 980ms. Playwright's + // `animations: "disabled"` freezes CSS animations at frame 0, making the + // card invisible in the screenshot. Override that so the card renders + // fully opaque for a deterministic snapshot. + await page.addStyleTag({ + content: + ".environment-switch-overlay > div { animation: none !important; opacity: 1 !important; transform: none !important; }", + }); + + await expect(page).toHaveScreenshot("backend-switch-overlay.png", SNAP_OPTS); + + // After overlay fades (980 ms), the selector should show "Production". + await page.waitForSelector('[data-testid="environment-switch-overlay"]', { + state: "hidden", + timeout: 3_000, + }); + // Re-hover to show the updated active backend in the dropdown. + await page.getByTestId("backend-selector").hover(); + await expect( + page.getByRole("option", { name: "Production" }), + ).toBeVisible({ timeout: 5_000 }); + await expect(rootLayout).toHaveScreenshot( + "backend-after-switch.png", + SNAP_OPTS, + ); + }); + + // ── Flow 7: Malformed/empty host ────────────────────────────────────── + + test("Flow 7 — empty or invalid host keeps Save disabled; valid host enables Save", async ({ + page, + }) => { + await setupPage(page); + const rootLayout = await openAddModal(page); + + // Seed just the name, leave host blank. + await page.getByTestId("add-backend-kind-local").click(); + await page.getByTestId("add-backend-name").fill("Bad URL Test"); + + // Whitespace-only host → isValidHostUrl returns false → disabled. + // Blur the field to reveal the inline "Host is required" error. + await page.getByTestId("add-backend-host").fill(" "); + await page.getByTestId("add-backend-host").blur(); + await expect(page.getByTestId("add-backend-submit")).toBeDisabled(); + await expect(rootLayout).toHaveScreenshot( + "backend-add-whitespace-host-disabled.png", + SNAP_OPTS, + ); + + // A syntactically invalid URL (spaces + garbled scheme) is rejected by + // isValidHostUrl() — Save stays disabled and the inline error explains why. + await page.getByTestId("add-backend-host").fill("not://:::a valid url!!!"); + await page.getByTestId("add-backend-host").blur(); + await expect(page.getByTestId("add-backend-submit")).toBeDisabled(); + await expect(rootLayout).toHaveScreenshot( + "backend-add-invalid-url-disabled.png", + SNAP_OPTS, + ); + }); + + // ── Flow 8: Cancel add form ─────────────────────────────────────────── + + test("Flow 8 — canceling the add form closes modal without persisting data", async ({ + page, + }) => { + await setupPage(page); + const rootLayout = await openAddModal(page); + + // Partially fill the form. + await page.getByTestId("add-backend-kind-local").click(); + await page.getByTestId("add-backend-name").fill("Temp Backend"); + await page.getByTestId("add-backend-host").fill("http://localhost:9999"); + + await expect(rootLayout).toHaveScreenshot( + "backend-add-form-partially-filled.png", + SNAP_OPTS, + ); + + // Click Cancel. + await page.getByTestId("add-backend-cancel").click(); + + // Modal is dismissed. + await expect(page.getByTestId("add-backend-modal")).not.toBeVisible({ + timeout: 5_000, + }); + + // Open Manage Backends to confirm "Temp Backend" was NOT saved. + await page.getByTestId("backend-selector").hover(); + await expect(page.getByTestId("manage-backends-menu-item")).toBeVisible({ + timeout: 5_000, + }); + await page.getByTestId("manage-backends-menu-item").click(); + await expect(page.getByTestId("manage-backends-modal")).toBeVisible({ + timeout: 5_000, + }); + + // Only the original "Local" backend should be present. + await expect( + page.getByTestId("manage-backends-row-Local"), + ).toBeVisible(); + await expect( + page.locator('[data-testid*="manage-backends-row-Temp"]'), + ).not.toBeVisible(); + + await expect(rootLayout).toHaveScreenshot( + "backend-cancel-nothing-saved.png", + SNAP_OPTS, + ); + }); +}); diff --git a/tests/e2e/snapshots/backends.snapshot.spec.ts b/tests/e2e/snapshots/backends.snapshot.spec.ts new file mode 100644 index 0000000000..5fb6819ca8 --- /dev/null +++ b/tests/e2e/snapshots/backends.snapshot.spec.ts @@ -0,0 +1,139 @@ +import { test, expect, Page } from "@playwright/test"; + +/** + * Visual snapshot tests for the backend management UI. + * + * The BackendSelector lives in the sidebar footer and opens a dropdown on + * hover. Its footer contains two action buttons: + * - data-testid="add-backend-menu-item" → opens BackendFormModal (add) + * - data-testid="manage-backends-menu-item" → opens ManageBackendsModal + * + * Backend state is seeded from the registry's default local backend + * (DEFAULT_LOCAL_BACKEND_NAME = "Local") which is auto-created in + * localStorage on first load. + * + * Three snapshots are captured: + * 1. Selector dropdown open — shows the "Local" backend with status dot + * and the Add / Manage footer actions. + * 2. Add Backend modal — BackendFormModal in "add" mode (empty form). + * 3. Manage Backends modal — ManageBackendsModal listing the default backend. + */ + +async function dismissConsentModal(page: Page) { + await page + .getByRole("button", { name: "Confirm preferences" }) + .click({ timeout: 3_000 }) + .catch(() => undefined); +} + +async function setupMocks(page: Page) { + await page.addInitScript(() => { + window.localStorage.setItem("openhands-onboarded", "true"); + }); + + // Suppress file-API proxy errors emitted when the home page scans the + // workspace directory (same suppression used in sidebar.snapshot.spec.ts). + await page.route("**/api/file/**", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ path: "/home", subdirs: [] }), + }); + }); +} + +/** + * Navigate to the home page, wait for it to stabilise, then hover over the + * backend selector to open the dropdown. Returns the rootLayout locator. + */ +async function openBackendDropdown(page: Page) { + await page.goto("/conversations"); + await dismissConsentModal(page); + await page.waitForLoadState("networkidle"); + + // Wait for the sidebar to be fully rendered. + const rootLayout = page.getByTestId("root-layout"); + await expect(rootLayout).toBeVisible({ timeout: 15_000 }); + + // The BackendSelector renders its Dropdown with openOnHover=true in the + // expanded sidebar footer. Hovering over data-testid="backend-selector" + // fires onMouseEnter → openMenu(). + const backendSelector = page.getByTestId("backend-selector"); + await expect(backendSelector).toBeVisible({ timeout: 10_000 }); + await backendSelector.hover(); + + // Wait for the dropdown footer actions to confirm the menu is open. + await expect(page.getByTestId("add-backend-menu-item")).toBeVisible({ + timeout: 5_000, + }); + + return rootLayout; +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +test.describe("Backend Management Visual Snapshots", () => { + test.setTimeout(60_000); + + test("backend selector dropdown shows registered backend with status dot", async ({ + page, + }) => { + await setupMocks(page); + const rootLayout = await openBackendDropdown(page); + + await expect(rootLayout).toHaveScreenshot("backend-selector-open.png", { + animations: "disabled", + maxDiffPixelRatio: 0.01, + }); + }); + + test("add backend modal opens with empty form", async ({ page }) => { + await setupMocks(page); + const rootLayout = await openBackendDropdown(page); + + // Click "Add backend" in the dropdown footer. + // onMouseDown has stopPropagation to keep the menu open; onClick opens the modal. + await page.getByTestId("add-backend-menu-item").click(); + + // BackendFormModal (mode="add") has data-testid="add-backend-modal". + await expect(page.getByTestId("add-backend-modal")).toBeVisible({ + timeout: 5_000, + }); + + // Wait for the name input to confirm the form has rendered. + await expect(page.getByTestId("add-backend-name")).toBeVisible({ + timeout: 5_000, + }); + + await expect(rootLayout).toHaveScreenshot("backend-add-modal.png", { + animations: "disabled", + maxDiffPixelRatio: 0.01, + }); + }); + + test("manage backends modal lists the default local backend", async ({ + page, + }) => { + await setupMocks(page); + const rootLayout = await openBackendDropdown(page); + + // Click "Manage backends" in the dropdown footer. + await page.getByTestId("manage-backends-menu-item").click(); + + // ManageBackendsModal has data-testid="manage-backends-modal". + await expect(page.getByTestId("manage-backends-modal")).toBeVisible({ + timeout: 5_000, + }); + + // Confirm at least one backend row is visible (the default "Local" backend). + // Row testids follow the pattern: manage-backends-row-${backend.name}. + await expect(page.getByTestId("manage-backends-row-Local")).toBeVisible({ + timeout: 5_000, + }); + + await expect(rootLayout).toHaveScreenshot("backend-manage-modal.png", { + animations: "disabled", + maxDiffPixelRatio: 0.01, + }); + }); +}); diff --git a/tests/e2e/snapshots/changes-tab.snapshot.spec.ts b/tests/e2e/snapshots/changes-tab.snapshot.spec.ts new file mode 100644 index 0000000000..57bb98e062 --- /dev/null +++ b/tests/e2e/snapshots/changes-tab.snapshot.spec.ts @@ -0,0 +1,287 @@ +import { test, expect, Page } from "@playwright/test"; + +/** + * Visual snapshot tests for the Changes (diff viewer) UI. + * + * The Changes view is rendered by src/routes/changes-tab.tsx inside the Files + * tab (src/routes/files-tab.tsx) when the "Diff view" toggle is ON. We force + * that toggle by pre-seeding the conversation's localStorage state with + * `filesTabDiffView: true` before navigation. + * + * MSW pre-seeds three git changes in src/mocks/git-repository-handlers.ts: + * - src/components/hello.tsx (M — modified) + * - src/utils/new-helper.ts (A — added) + * - src/old-module.py (D — deleted) + * + * Three snapshots are captured: + * 1. Empty state — no files changed (window.__setMockGitChanges__([]) used + * to clear MSW's in-memory list after boot, then a query invalidation + * triggers a re-fetch that returns []). + * 2. Diff viewer — modified file (hello.tsx) expanded to show Monaco. + * 3. Deleted file placeholder — deleted file (old-module.py) shows the + * "file deleted" message instead of a Monaco editor (the diff query is + * disabled for type "D" per useUnifiedGitDiff). + * + * NOTE on MSW vs page.route(): + * MSW 2.x browser-mode handlers run in the page's main thread, not the + * service worker. Playwright's page.route() is blocked by the service worker + * for same-origin requests. We therefore manipulate MSW state via + * page.evaluate() rather than page.route() (same pattern as the automations + * empty-state test). + */ + +// Mock conversation IDs "1", "2", "3" are pre-defined in MSW handlers. +const CONVERSATION_ID = "1"; + +// Pre-enable diff view for conversation 1. +// NOTE: rightPanelShown is intentionally omitted — it is session-only state +// stripped by sanitizeStoredState on read. The right panel is opened +// programmatically via a right-panel-toggle click in navigateAndWaitForFilesTab. +const CONVERSATION_STATE_KEY = `conversation-state-${CONVERSATION_ID}`; +const CONVERSATION_STATE_VALUE = JSON.stringify({ + selectedTab: "files", + filesTabDiffView: true, + filesTabContentViewMode: "rich", + unpinnedTabs: [], + conversationMode: "code", + subConversationTaskId: null, + draftMessage: null, +}); + +/** + * Skip onboarding and pre-enable the diff view for conversation 1. + */ +async function setupMocks(page: Page) { + await page.addInitScript( + ([key, value]) => { + window.localStorage.setItem("openhands-onboarded", "true"); + window.localStorage.setItem(key, value); + }, + [CONVERSATION_STATE_KEY, CONVERSATION_STATE_VALUE] as [string, string], + ); + + // Stub WebSocket so the conversation page doesn't hang waiting for a real + // socket connection. Copied from collapsible-thinking.snapshot.spec.ts. + await page.addInitScript(() => { + const noop = () => {}; + class StubWebSocket extends EventTarget { + static CONNECTING = 0; + static OPEN = 1; + static CLOSING = 2; + static CLOSED = 3; + readyState = StubWebSocket.OPEN; + url: string; + protocol = ""; + extensions = ""; + bufferedAmount = 0; + binaryType: BinaryType = "blob"; + onopen: ((ev: Event) => void) | null = null; + onclose: ((ev: CloseEvent) => void) | null = null; + onmessage: ((ev: MessageEvent) => void) | null = null; + onerror: ((ev: Event) => void) | null = null; + + constructor(url: string | URL) { + super(); + this.url = typeof url === "string" ? url : url.toString(); + setTimeout(() => { + const evt = new Event("open"); + this.onopen?.(evt); + this.dispatchEvent(evt); + }, 10); + } + + send = noop; + close = noop; + CONNECTING = StubWebSocket.CONNECTING; + OPEN = StubWebSocket.OPEN; + CLOSING = StubWebSocket.CLOSING; + CLOSED = StubWebSocket.CLOSED; + } + (window as unknown as { WebSocket: unknown }).WebSocket = + StubWebSocket as unknown as typeof WebSocket; + }); +} + +async function dismissConsentModal(page: Page) { + await page + .getByRole("button", { name: "Confirm preferences" }) + .click({ timeout: 3_000 }) + .catch(() => undefined); +} + +/** + * Navigate to the conversation and wait for the Files tab (diff view) to + * be rendered. Returns the `data-testid="files-tab"` locator. + * + * `isRightPanelShown` is session-only Zustand state (always false on load; + * `sanitizeStoredState` strips any persisted `rightPanelShown` key). + * We open the right panel by clicking the `right-panel-toggle` button, + * which calls `setHasRightPanelToggled(true)` → synced to + * `setIsRightPanelShown(true)` by `use-chat-input-logic`. + */ +async function navigateAndWaitForFilesTab(page: Page) { + await page.goto(`/conversations/${CONVERSATION_ID}`, { + waitUntil: "domcontentloaded", + }); + await dismissConsentModal(page); + + // Open the right panel — it always starts closed on page load. + const toggle = page.getByTestId("right-panel-toggle"); + await expect(toggle).toBeVisible({ timeout: 15_000 }); + await toggle.click(); + + // The FilesTab is lazy-loaded inside the now-open right panel. + const filesTab = page.getByTestId("files-tab"); + await expect(filesTab).toBeVisible({ timeout: 20_000 }); + return filesTab; +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +test.describe("Changes Tab Visual Snapshots", () => { + // Heavier conversation-page setup — run serially to avoid flakiness. + test.describe.configure({ mode: "serial" }); + test.setTimeout(60_000); + + test("changes tab shows empty state when no files changed", async ({ + page, + }) => { + // MSW pre-seeds MOCK_GIT_CHANGES with M/A/D files. We call the exposed + // window setter (installed by git-repository-handlers.ts) AFTER the app + // boots to replace the list with [], then ask React Query to refetch. + // This avoids a page.reload() which would re-seed the module state. + await setupMocks(page); + + await navigateAndWaitForFilesTab(page); + + // Pin the RandomTip section to a fixed height so the flex-1 container + // above it is deterministic across runs. RandomTip renders a randomly + // selected tip whose text can vary in line count, causing different + // layout heights between the baseline-generation run and verification run. + // The class combination ".text-m.bg-tertiary.p-4" is unique to this element + // in changes-tab.tsx (confirmed by grep). Hiding the content removes the + // visual variable; the fixed height keeps the surrounding flex layout stable. + await page.addStyleTag({ + content: `.text-m.bg-tertiary.p-4 { + height: 80px !important; + overflow: hidden !important; + visibility: hidden !important; + }`, + }); + + // Wait for the initial (non-empty) render to settle before mutating state. + await expect( + page.locator('[data-testid="file-diff-viewer-outer"]').first(), + ).toBeVisible({ timeout: 10_000 }); + + // Clear the changes via the exposed window helper and refetch. + await page.evaluate(() => { + ( + window as unknown as { + __setMockGitChanges__?: (changes: unknown[]) => void; + } + ).__setMockGitChanges__?.([]); + }); + await page.evaluate(() => { + ( + window as unknown as { + __TEST_INVALIDATE_QUERIES__?: (queryKey?: unknown[]) => void; + } + ).__TEST_INVALIDATE_QUERIES__?.(["file_changes"]); + }); + + // Wait for the empty-state message from EmptyChangesMessage component. + await expect( + page.getByText("OpenHands hasn't made any changes yet"), + ).toBeVisible({ timeout: 10_000 }); + + // Screenshot the diff-content div (direct child of files-tab when diff view + // is enabled) rather than the full files-tab. This avoids capturing any + // adjacent-panel artefacts that may bleed into the outer element's bounding + // box during CI rendering, while still showing the full empty-state UI. + // + // DOM path: main[data-testid="files-tab"] > div.flex-1.min-h-0 > main + // The inner
rendered by GitChanges is the safest stable target. + const changesContent = page + .getByTestId("files-tab") + .locator("> div") + .last(); + await expect(changesContent).toHaveScreenshot("changes-empty.png", { + animations: "disabled", + maxDiffPixelRatio: 0.01, + }); + }); + + test("changes tab shows file list and diff viewer for modified file", async ({ + page, + }) => { + // MOCK_GIT_CHANGES is pre-seeded; the file list renders without any override. + await setupMocks(page); + + const filesTab = await navigateAndWaitForFilesTab(page); + + // Wait for at least one file row to appear. + await expect( + page.locator('[data-testid="file-diff-viewer-outer"]').first(), + ).toBeVisible({ timeout: 10_000 }); + + // Click the modified file (hello.tsx) header row to expand the diff editor. + // The header row is the first child div of file-diff-viewer-outer and has + // the cursor-pointer class; clicking the strong element (file path) is the + // most reliable targeting. + await page + .locator('[data-testid="file-diff-viewer-outer"]') + .filter({ hasText: "hello.tsx" }) + .locator("strong") + .click(); + + // Wait for the EditorContainer (wraps the Monaco DiffEditor) to appear. + await expect(page.getByTestId("editor-container").first()).toBeVisible({ + timeout: 10_000, + }); + + // Mask the Monaco DiffEditor container. Monaco renders text content + // progressively and uses sub-pixel font hinting that varies between OS/CI + // environments. Masking editor-container captures the panel layout (toolbar, + // file list, editor frame) without the volatile text-rendering pixels. + await expect(filesTab).toHaveScreenshot("changes-diff-viewer.png", { + animations: "disabled", + maxDiffPixelRatio: 0.01, + mask: [page.getByTestId("editor-container")], + }); + }); + + test("changes tab shows deleted-file placeholder instead of diff editor", async ({ + page, + }) => { + // src/old-module.py has type "D" (deleted). useUnifiedGitDiff disables the + // query for deleted files; clicking the row expands the file-deleted-message + // placeholder instead of a Monaco editor. + await setupMocks(page); + + const filesTab = await navigateAndWaitForFilesTab(page); + + // Wait for at least one file row to appear. + await expect( + page.locator('[data-testid="file-diff-viewer-outer"]').first(), + ).toBeVisible({ timeout: 10_000 }); + + // Click the deleted file row to expand it. + await page + .locator('[data-testid="file-diff-viewer-outer"]') + .filter({ hasText: "old-module.py" }) + .locator("strong") + .click(); + + // The deleted-file placeholder (data-testid="file-deleted-message") is + // shown when !isCollapsed && type === "D". + await expect(page.getByTestId("file-deleted-message")).toBeVisible({ + timeout: 10_000, + }); + + await expect(filesTab).toHaveScreenshot("changes-deleted-file.png", { + animations: "disabled", + maxDiffPixelRatio: 0.01, + }); + }); +}); diff --git a/tests/e2e/snapshots/scripts/post-snapshot-comment.mjs b/tests/e2e/snapshots/scripts/post-snapshot-comment.mjs index 2a2a1dc698..87372f3fb5 100644 --- a/tests/e2e/snapshots/scripts/post-snapshot-comment.mjs +++ b/tests/e2e/snapshots/scripts/post-snapshot-comment.mjs @@ -22,6 +22,7 @@ import { execSync } from "node:child_process"; import { + appendFileSync, copyFileSync, existsSync, mkdirSync, @@ -39,6 +40,8 @@ const HEAD_REF = requireEnv("HEAD_REF"); const MAIN_BASELINES_DIR = process.env.MAIN_BASELINES_DIR ?? "/tmp/main-baselines"; const SNAPSHOTS_APPROVED = process.env.SNAPSHOTS_APPROVED === "true"; +const GENERATE_OUTCOME = process.env.GENERATE_OUTCOME ?? "success"; +const COMPARE_OUTCOME = process.env.COMPARE_OUTCOME ?? "success"; const SNAPSHOTS_DIR = "tests/e2e/__snapshots__"; // The workflow saves comparison test-results to this path before the @@ -255,6 +258,27 @@ function buildComment(changed, newSnapshots, unchanged, commitSha) { COMMENT_MARKER, `## 📸 Snapshot Test Report`, "", + ]; + + if (COMPARE_OUTCOME === "failure") { + lines.push( + `> [!WARNING]`, + `> **Snapshot comparison step crashed** (timeout, OOM, or runner error) — diff results below may be incomplete or absent.`, + `> Check the [CI logs](https://github.com/${REPO}/actions/runs/${RUN_ID}) for the full error output (look for the "Run snapshot comparison" step).`, + "", + ); + } + + if (GENERATE_OUTCOME === "failure") { + lines.push( + `> [!WARNING]`, + `> **One or more snapshot tests crashed during generation** — some snapshots below may be incomplete.`, + `> Check the [CI logs](https://github.com/${REPO}/actions/runs/${RUN_ID}) for the full error output (look for the "Generate current PR snapshots" step).`, + "", + ); + } + + lines.push( `${statusIcon} ${statusText}`, "", `| Category | Count |`, @@ -264,7 +288,7 @@ function buildComment(changed, newSnapshots, unchanged, commitSha) { `| ✅ Unchanged | ${unchanged.length} |`, `| **Total** | **${total}** |`, "", - ]; + ); if (hasDifferences && !SNAPSHOTS_APPROVED) { lines.push( @@ -452,6 +476,18 @@ async function main() { const body = buildComment(changed, newSnapshots, unchanged, commitSha); await postFreshComment(body); + + // Tell the workflow whether there are actual pixel-diff failures so the + // "Fail if differences" step can distinguish changed snapshots (should + // fail CI) from missing baselines (new tests from this PR, should pass). + if (process.env.GITHUB_OUTPUT) { + appendFileSync( + process.env.GITHUB_OUTPUT, + `has_changes=${changed.length > 0}\n`, + ); + console.log(` has_changes=${changed.length > 0} written to GITHUB_OUTPUT`); + } + console.log("Done."); }