From b2af00b0378a81dd0d34465b31bb9e1340bfc64e Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 4 May 2026 23:53:59 +0800 Subject: [PATCH] =?UTF-8?q?test(app):=20triage=20desktop=20test=20files=20?= =?UTF-8?q?=E2=80=94=20delete=2016-mock=20component=20slop,=20tighten=20at?= =?UTF-8?q?tachment=20store=20(#721)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(app): triage desktop test files — delete 16-mock component slop, tighten attachment store assertion Removes desktop-updates-section.test.tsx (16 internal mocks, JSDOM, toHaveBeenCalledWith chains on component renders). Tightens desktop-attachment-store.test.ts: toHaveBeenCalled() → toHaveBeenCalledWith(attachment). * refactor(app): extract daemon management toggle coordinator with unit tests Fills the coverage gap left by deleting desktop-updates-section.test.tsx. Extracts executeDaemonManagementToggle from useDaemonManagementToggle — pure async coordinator with injected deps (no vi.mock). Unit tests verify the three key invariants: settings persist before stop, stop is skipped when not desktop-managed, and start/stop are invoked on enable/disable. --- .../desktop-attachment-store.test.ts | 2 +- .../desktop-updates-section.test.tsx | 362 ------------------ .../components/desktop-updates-section.tsx | 91 ++--- .../daemon/daemon-management-toggle.test.ts | 159 ++++++++ .../daemon/daemon-management-toggle.ts | 41 ++ 5 files changed, 232 insertions(+), 423 deletions(-) delete mode 100644 packages/app/src/desktop/components/desktop-updates-section.test.tsx create mode 100644 packages/app/src/desktop/daemon/daemon-management-toggle.test.ts create mode 100644 packages/app/src/desktop/daemon/daemon-management-toggle.ts diff --git a/packages/app/src/desktop/attachments/desktop-attachment-store.test.ts b/packages/app/src/desktop/attachments/desktop-attachment-store.test.ts index 175d9d597..4903380e1 100644 --- a/packages/app/src/desktop/attachments/desktop-attachment-store.test.ts +++ b/packages/app/src/desktop/attachments/desktop-attachment-store.test.ts @@ -139,7 +139,7 @@ describe("desktop attachment store", () => { await store.garbageCollect({ referencedIds: new Set(["att_3"]) }); expect(readDesktopFileBase64Mock).toHaveBeenCalledWith("/managed/att_3.jpg"); - expect(resolveDesktopPreviewUrlMock).toHaveBeenCalled(); + expect(resolveDesktopPreviewUrlMock).toHaveBeenCalledWith(attachment); expect(releaseDesktopPreviewUrlMock).toHaveBeenCalledWith({ url: "blob:test" }); expect(deleteDesktopAttachmentFileMock).toHaveBeenCalledWith({ path: "/managed/att_3.jpg", diff --git a/packages/app/src/desktop/components/desktop-updates-section.test.tsx b/packages/app/src/desktop/components/desktop-updates-section.test.tsx deleted file mode 100644 index fc0067950..000000000 --- a/packages/app/src/desktop/components/desktop-updates-section.test.tsx +++ /dev/null @@ -1,362 +0,0 @@ -/** - * @vitest-environment jsdom - */ -import { cleanup, fireEvent, render, waitFor } from "@testing-library/react"; -import React from "react"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { LocalDaemonSection } from "./desktop-updates-section"; - -const { alertMock, confirmDialogMock, settingsState, daemonStatusState } = vi.hoisted(() => ({ - alertMock: vi.fn(), - confirmDialogMock: vi.fn(), - settingsState: { - settings: { - releaseChannel: "stable" as const, - daemon: { - manageBuiltInDaemon: true, - keepRunningAfterQuit: true, - }, - }, - updateSettings: vi.fn< - (updates: { - daemon?: { - manageBuiltInDaemon?: boolean; - keepRunningAfterQuit?: boolean; - }; - }) => Promise - >(), - }, - daemonStatusState: { - data: { - status: { - serverId: "desktop", - status: "running" as const, - listen: null, - hostname: null, - pid: 123, - home: "/tmp/paseo", - version: "1.2.3", - desktopManaged: true, - error: null, - }, - logs: { - logPath: "/tmp/paseo/daemon.log", - contents: "daemon log", - }, - }, - isLoading: false, - error: null as string | null, - setStatus: vi.fn(), - refetch: vi.fn(), - }, -})); - -vi.mock("react-native", () => ({ - ActivityIndicator: () => React.createElement("div", { "data-testid": "loading-spinner" }), - Alert: { alert: alertMock }, - Text: ({ children }: { children?: React.ReactNode }) => - React.createElement("span", null, children), - View: ({ children }: { children?: React.ReactNode }) => - React.createElement("div", null, children), -})); - -vi.mock("react-native-unistyles", () => ({ - StyleSheet: { - create: (factory: unknown) => - typeof factory === "function" - ? (factory as (theme: Record) => unknown)({ - spacing: { 1: 4, 2: 8, 3: 12, 6: 24 }, - borderRadius: { lg: 12 }, - fontSize: { xs: 12, sm: 14 }, - colors: { - foreground: "#111", - foregroundMuted: "#666", - palette: { amber: { 500: "#f59e0b" } }, - }, - }) - : factory, - }, - useUnistyles: () => ({ - theme: { - iconSize: { sm: 14 }, - colors: { - foreground: "#111", - foregroundMuted: "#666", - }, - }, - }), -})); - -vi.mock("expo-clipboard", () => ({ - setStringAsync: vi.fn(() => Promise.resolve()), -})); - -vi.mock("lucide-react-native", () => { - const icon = (name: string) => () => React.createElement("span", { "data-icon": name }); - return { - Activity: icon("Activity"), - ArrowUpRight: icon("ArrowUpRight"), - Copy: icon("Copy"), - FileText: icon("FileText"), - }; -}); - -vi.mock("@/styles/settings", () => ({ - settingsStyles: new Proxy( - {}, - { - get: (_target, prop) => String(prop), - }, - ), -})); - -vi.mock("@/screens/settings/settings-section", () => ({ - SettingsSection: ({ - children, - title, - trailing, - }: { - children?: React.ReactNode; - title: string; - trailing?: React.ReactNode; - }) => - React.createElement( - "section", - null, - React.createElement("h2", null, title), - trailing, - children, - ), -})); - -vi.mock("@/components/adaptive-modal-sheet", () => ({ - AdaptiveModalSheet: ({ - children, - visible, - title, - }: { - children?: React.ReactNode; - visible?: boolean; - title?: string; - }) => (visible ? React.createElement("div", { "data-title": title }, children) : null), -})); - -vi.mock("@/components/ui/button", () => ({ - Button: ({ - children, - onPress, - disabled, - accessibilityLabel, - }: { - children?: React.ReactNode; - onPress?: () => void; - disabled?: boolean; - accessibilityLabel?: string; - }) => - React.createElement( - "button", - { type: "button", onClick: onPress, disabled, "aria-label": accessibilityLabel }, - children, - ), -})); - -vi.mock("@/components/ui/switch", () => ({ - Switch: ({ - value, - onValueChange, - disabled, - accessibilityLabel, - }: { - value: boolean; - onValueChange?: (next: boolean) => void; - disabled?: boolean; - accessibilityLabel?: string; - }) => - React.createElement("button", { - type: "button", - role: "switch", - "aria-label": accessibilityLabel, - "aria-checked": value, - disabled, - onClick: () => onValueChange?.(!value), - }), -})); - -vi.mock("@/desktop/settings/desktop-settings", () => ({ - useDesktopSettings: () => ({ - ...settingsState, - isLoading: false, - error: null, - }), -})); - -vi.mock("@/desktop/hooks/use-daemon-status", () => ({ - useDaemonStatus: () => daemonStatusState, -})); - -vi.mock("@/utils/confirm-dialog", () => ({ - confirmDialog: confirmDialogMock, -})); - -vi.mock("@/utils/open-external-url", () => ({ - openExternalUrl: vi.fn(), -})); - -vi.mock("@/desktop/updates/desktop-updates", () => ({ - isVersionMismatch: vi.fn(() => false), -})); - -const daemonCommandMocks = vi.hoisted(() => ({ - getCliDaemonStatusMock: vi.fn(), - startDesktopDaemonMock: vi.fn(), - stopDesktopDaemonMock: vi.fn(), -})); - -vi.mock("@/desktop/daemon/desktop-daemon", () => ({ - getCliDaemonStatus: daemonCommandMocks.getCliDaemonStatusMock, - shouldUseDesktopDaemon: vi.fn(() => true), - startDesktopDaemon: daemonCommandMocks.startDesktopDaemonMock, - stopDesktopDaemon: daemonCommandMocks.stopDesktopDaemonMock, -})); - -vi.mock("@/utils/app-version", () => ({ - resolveAppVersion: vi.fn(() => "1.2.3"), -})); - -describe("LocalDaemonSection", () => { - afterEach(() => { - cleanup(); - }); - - beforeEach(() => { - alertMock.mockReset(); - confirmDialogMock.mockReset(); - settingsState.settings.daemon.manageBuiltInDaemon = true; - settingsState.settings.daemon.keepRunningAfterQuit = true; - settingsState.updateSettings.mockReset(); - settingsState.updateSettings.mockResolvedValue(); - daemonStatusState.data.status.status = "running"; - daemonStatusState.data.status.desktopManaged = true; - daemonStatusState.setStatus.mockReset(); - daemonStatusState.refetch.mockReset(); - daemonCommandMocks.startDesktopDaemonMock.mockReset(); - daemonCommandMocks.startDesktopDaemonMock.mockResolvedValue({ - ...daemonStatusState.data.status, - status: "running", - desktopManaged: true, - }); - daemonCommandMocks.stopDesktopDaemonMock.mockReset(); - daemonCommandMocks.stopDesktopDaemonMock.mockResolvedValue({ - ...daemonStatusState.data.status, - status: "stopped", - }); - daemonCommandMocks.getCliDaemonStatusMock.mockReset(); - }); - - it("renders the daemon toggles as switches reflecting current settings", () => { - const screen = render(); - - expect(screen.getByText("Keep daemon running after quit")).toBeTruthy(); - expect(screen.getByText("Manage built-in daemon")).toBeTruthy(); - - const keepRunningSwitch = screen.getByRole("switch", { - name: "Keep daemon running after quit", - }); - expect(keepRunningSwitch.getAttribute("aria-checked")).toBe("true"); - - const manageSwitch = screen.getByRole("switch", { name: "Manage built-in daemon" }); - expect(manageSwitch.getAttribute("aria-checked")).toBe("true"); - }); - - it("does not render a duplicate restart-daemon control", () => { - const screen = render(); - - expect(screen.queryByText("Restart daemon")).toBeNull(); - expect(screen.queryByText("Start daemon")).toBeNull(); - }); - - it("updates keep-running-after-quit without changing daemon lifecycle", async () => { - const screen = render(); - - fireEvent.click(screen.getByRole("switch", { name: "Keep daemon running after quit" })); - - await waitFor(() => { - expect(settingsState.updateSettings).toHaveBeenCalledWith({ - daemon: { - keepRunningAfterQuit: false, - }, - }); - }); - expect(confirmDialogMock).not.toHaveBeenCalled(); - expect(daemonCommandMocks.stopDesktopDaemonMock).not.toHaveBeenCalled(); - }); - - it("pauses built-in daemon management and persists the setting through desktop settings", async () => { - confirmDialogMock.mockResolvedValue(true); - const screen = render(); - - fireEvent.click(screen.getByRole("switch", { name: "Manage built-in daemon" })); - - await waitFor(() => { - expect(daemonCommandMocks.stopDesktopDaemonMock).toHaveBeenCalledTimes(1); - }); - expect(settingsState.updateSettings).toHaveBeenCalledWith({ - daemon: { - manageBuiltInDaemon: false, - }, - }); - }); - - it("persists paused management before stopping the desktop-managed daemon", async () => { - confirmDialogMock.mockResolvedValue(true); - const screen = render(); - - fireEvent.click(screen.getByRole("switch", { name: "Manage built-in daemon" })); - - await waitFor(() => { - expect(daemonCommandMocks.stopDesktopDaemonMock).toHaveBeenCalledTimes(1); - }); - - expect(settingsState.updateSettings.mock.invocationCallOrder[0]).toBeLessThan( - daemonCommandMocks.stopDesktopDaemonMock.mock.invocationCallOrder[0], - ); - }); - - it("does not stop a manually managed daemon when pausing built-in daemon management", async () => { - confirmDialogMock.mockResolvedValue(true); - daemonStatusState.data.status.desktopManaged = false; - const screen = render(); - - fireEvent.click(screen.getByRole("switch", { name: "Manage built-in daemon" })); - - await waitFor(() => { - expect(settingsState.updateSettings).toHaveBeenCalledWith({ - daemon: { - manageBuiltInDaemon: false, - }, - }); - }); - expect(daemonCommandMocks.stopDesktopDaemonMock).not.toHaveBeenCalled(); - }); - - it("starts the built-in daemon when management is re-enabled", async () => { - settingsState.settings.daemon.manageBuiltInDaemon = false; - const screen = render(); - - fireEvent.click(screen.getByRole("switch", { name: "Manage built-in daemon" })); - - await waitFor(() => { - expect(daemonCommandMocks.startDesktopDaemonMock).toHaveBeenCalledTimes(1); - }); - expect(settingsState.updateSettings).toHaveBeenCalledWith({ - daemon: { - manageBuiltInDaemon: true, - }, - }); - expect(daemonStatusState.setStatus).toHaveBeenCalledWith({ - ...daemonStatusState.data.status, - status: "running", - desktopManaged: true, - }); - }); -}); diff --git a/packages/app/src/desktop/components/desktop-updates-section.tsx b/packages/app/src/desktop/components/desktop-updates-section.tsx index 04e38e560..de3dd8e55 100644 --- a/packages/app/src/desktop/components/desktop-updates-section.tsx +++ b/packages/app/src/desktop/components/desktop-updates-section.tsx @@ -17,6 +17,7 @@ import { startDesktopDaemon, stopDesktopDaemon, } from "@/desktop/daemon/desktop-daemon"; +import { executeDaemonManagementToggle } from "@/desktop/daemon/daemon-management-toggle"; import { useDaemonStatus } from "@/desktop/hooks/use-daemon-status"; import { useDesktopSettings, type DesktopSettings } from "@/desktop/settings/desktop-settings"; import type { DesktopDaemonStatus } from "@/desktop/daemon/desktop-daemon"; @@ -39,72 +40,42 @@ function useDaemonManagementToggle(args: { return; } - if (!settings.manageBuiltInDaemon) { - setIsUpdatingDaemonManagement(true); - void updateSettings({ manageBuiltInDaemon: true }) - .then(() => startDesktopDaemon()) - .then((newStatus) => { - setStatus(newStatus); - refetch(); - return; - }) - .catch((error) => { - console.error("[Settings] Failed to update built-in daemon management", error); - Alert.alert("Error", "Unable to update built-in daemon management."); - }) - .finally(() => { - setIsUpdatingDaemonManagement(false); - }); - return; - } - - void confirmDialog({ - title: "Pause built-in daemon", - message: - "This will stop the built-in daemon immediately. Running agents and terminals connected to the built-in daemon will be stopped.", - confirmLabel: "Pause and stop", - cancelLabel: "Cancel", - destructive: true, + setIsUpdatingDaemonManagement(true); + void executeDaemonManagementToggle(settings.manageBuiltInDaemon, daemonStatus, { + confirm: () => + confirmDialog({ + title: "Pause built-in daemon", + message: + "This will stop the built-in daemon immediately. Running agents and terminals connected to the built-in daemon will be stopped.", + confirmLabel: "Pause and stop", + cancelLabel: "Cancel", + destructive: true, + }), + persistSettings: (next) => updateSettings(next) as Promise, + startDaemon: startDesktopDaemon, + stopDaemon: stopDesktopDaemon, }) - .then((confirmed) => { - if (!confirmed) { + .then((result) => { + if (result.kind === "cancelled") { return; } - - setIsUpdatingDaemonManagement(true); - - void updateSettings({ manageBuiltInDaemon: false }) - .then(() => { - if (daemonStatus?.status === "running" && daemonStatus.desktopManaged) { - return stopDesktopDaemon(); - } - return daemonStatus ?? null; - }) - .then((newStatus) => { - if (newStatus) { - setStatus(newStatus); - } - return; - }) - .then(() => { - refetch(); - return; - }) - .catch((error) => { - console.error("[Settings] Failed to stop built-in daemon", error); - Alert.alert( - "Error", - "Built-in daemon management was paused, but Paseo could not stop the daemon.", - ); - }) - .finally(() => { - setIsUpdatingDaemonManagement(false); - }); + if (result.newStatus) { + setStatus(result.newStatus); + } + refetch(); return; }) .catch((error) => { - console.error("[Settings] Failed to open built-in daemon pause confirmation", error); - Alert.alert("Error", "Unable to open the daemon confirmation dialog."); + console.error("[Settings] Failed to update built-in daemon management", error); + Alert.alert( + "Error", + settings.manageBuiltInDaemon + ? "Built-in daemon management was paused, but Paseo could not stop the daemon." + : "Unable to update built-in daemon management.", + ); + }) + .finally(() => { + setIsUpdatingDaemonManagement(false); }); }, [ daemonStatus, diff --git a/packages/app/src/desktop/daemon/daemon-management-toggle.test.ts b/packages/app/src/desktop/daemon/daemon-management-toggle.test.ts new file mode 100644 index 000000000..c63f4e13d --- /dev/null +++ b/packages/app/src/desktop/daemon/daemon-management-toggle.test.ts @@ -0,0 +1,159 @@ +import { describe, expect, it } from "vitest"; +import { executeDaemonManagementToggle } from "./daemon-management-toggle"; +import type { DesktopDaemonStatus } from "./desktop-daemon"; + +const runningManagedStatus: DesktopDaemonStatus = { + serverId: "desktop", + status: "running", + listen: null, + hostname: null, + pid: 123, + home: "/tmp/paseo", + version: "1.0.0", + desktopManaged: true, + error: null, +}; + +const stoppedStatus: DesktopDaemonStatus = { ...runningManagedStatus, status: "stopped" }; + +function makeDeps(overrides?: { + confirm?: () => Promise; + persistSettings?: (next: { manageBuiltInDaemon: boolean }) => Promise; + startDaemon?: () => Promise; + stopDaemon?: () => Promise; +}) { + const calls: string[] = []; + return { + deps: { + confirm: overrides?.confirm ?? (() => Promise.resolve(true)), + persistSettings: + overrides?.persistSettings ?? + (async () => { + calls.push("persist"); + }), + startDaemon: + overrides?.startDaemon ?? + (async () => { + calls.push("start"); + return { ...runningManagedStatus, status: "running" as const }; + }), + stopDaemon: + overrides?.stopDaemon ?? + (async () => { + calls.push("stop"); + return stoppedStatus; + }), + }, + calls, + }; +} + +describe("executeDaemonManagementToggle", () => { + describe("enable path (currentlyManaging: false)", () => { + it("persists the new setting then starts the daemon", async () => { + const { deps, calls } = makeDeps(); + + const result = await executeDaemonManagementToggle(false, null, deps); + + expect(result).toEqual({ kind: "enabled", newStatus: runningManagedStatus }); + expect(calls).toEqual(["persist", "start"]); + }); + + it("persists manageBuiltInDaemon: true", async () => { + let persisted: { manageBuiltInDaemon: boolean } | null = null; + const { deps } = makeDeps({ + persistSettings: async (next) => { + persisted = next; + }, + }); + + await executeDaemonManagementToggle(false, null, deps); + + expect(persisted).toEqual({ manageBuiltInDaemon: true }); + }); + }); + + describe("disable path (currentlyManaging: true)", () => { + it("returns cancelled without changing settings when confirmation is rejected", async () => { + const persistedSettings: unknown[] = []; + const { deps } = makeDeps({ + confirm: () => Promise.resolve(false), + persistSettings: async (next) => { + persistedSettings.push(next); + }, + }); + + const result = await executeDaemonManagementToggle(true, runningManagedStatus, deps); + + expect(result).toEqual({ kind: "cancelled" }); + expect(persistedSettings).toHaveLength(0); + }); + + it("persists settings BEFORE stopping the daemon", async () => { + const callOrder: string[] = []; + const { deps } = makeDeps({ + persistSettings: async () => { + callOrder.push("persist"); + }, + stopDaemon: async () => { + callOrder.push("stop"); + return stoppedStatus; + }, + }); + + await executeDaemonManagementToggle(true, runningManagedStatus, deps); + + expect(callOrder).toEqual(["persist", "stop"]); + }); + + it("persists manageBuiltInDaemon: false when disabling", async () => { + let persisted: { manageBuiltInDaemon: boolean } | null = null; + const { deps } = makeDeps({ + persistSettings: async (next) => { + persisted = next; + }, + }); + + await executeDaemonManagementToggle(true, runningManagedStatus, deps); + + expect(persisted).toEqual({ manageBuiltInDaemon: false }); + }); + + it("stops the daemon when it is running and desktop-managed", async () => { + const { deps, calls } = makeDeps(); + + const result = await executeDaemonManagementToggle(true, runningManagedStatus, deps); + + expect(calls).toContain("stop"); + expect(result).toEqual({ kind: "disabled", newStatus: stoppedStatus }); + }); + + it("skips stop when daemon is running but not desktop-managed", async () => { + const { deps, calls } = makeDeps(); + const manuallyManagedStatus = { ...runningManagedStatus, desktopManaged: false }; + + const result = await executeDaemonManagementToggle(true, manuallyManagedStatus, deps); + + expect(calls).not.toContain("stop"); + expect(result).toEqual({ kind: "disabled", newStatus: null }); + }); + + it("skips stop when daemon is stopped (regardless of desktopManaged)", async () => { + const { deps, calls } = makeDeps(); + + const result = await executeDaemonManagementToggle(true, stoppedStatus, deps); + + expect(calls).not.toContain("stop"); + expect(result).toEqual({ kind: "disabled", newStatus: null }); + }); + + it("skips stop when daemonStatus is null", async () => { + const { deps, calls } = makeDeps(); + + const result = await executeDaemonManagementToggle(true, null, deps); + + expect(calls).not.toContain("stop"); + expect(result).toEqual({ kind: "disabled", newStatus: null }); + }); + }); +}); diff --git a/packages/app/src/desktop/daemon/daemon-management-toggle.ts b/packages/app/src/desktop/daemon/daemon-management-toggle.ts new file mode 100644 index 000000000..efec0ad20 --- /dev/null +++ b/packages/app/src/desktop/daemon/daemon-management-toggle.ts @@ -0,0 +1,41 @@ +import type { DesktopDaemonStatus } from "./desktop-daemon"; + +export interface DaemonManagementToggleDeps { + confirm: () => Promise; + persistSettings: (next: { manageBuiltInDaemon: boolean }) => Promise; + startDaemon: () => Promise; + stopDaemon: () => Promise; +} + +export type DaemonManagementToggleResult = + | { kind: "cancelled" } + | { kind: "enabled"; newStatus: DesktopDaemonStatus } + | { kind: "disabled"; newStatus: DesktopDaemonStatus | null }; + +export async function executeDaemonManagementToggle( + currentlyManaging: boolean, + daemonStatus: Pick | null, + deps: DaemonManagementToggleDeps, +): Promise { + if (!currentlyManaging) { + await deps.persistSettings({ manageBuiltInDaemon: true }); + const newStatus = await deps.startDaemon(); + return { kind: "enabled", newStatus }; + } + + const confirmed = await deps.confirm(); + if (!confirmed) { + return { kind: "cancelled" }; + } + + // Settings must persist before the daemon is stopped so the persisted + // state reflects what was actually applied if the stop fails. + await deps.persistSettings({ manageBuiltInDaemon: false }); + + if (daemonStatus?.status === "running" && daemonStatus.desktopManaged) { + const newStatus = await deps.stopDaemon(); + return { kind: "disabled", newStatus }; + } + + return { kind: "disabled", newStatus: null }; +}