Files
OpenHands/vitest.setup.ts
T
Tim O'Farrellandopenhands 0206d0e9ee fix(skills): save disabled_skills when server omits the field (#837)
* fix(skills): save disabled_skills when settings has no prior disabled_skills field

The hydration effect in SkillsSettingsScreen only set hasHydratedInitialSettings=true
when settings?.disabled_skills was truthy. When no skills had ever been disabled,
the server omits the field entirely (undefined), so the condition was always false:

  if (settings?.disabled_skills) { ... }   // skipped when field is absent

Because settings?.disabled_skills is undefined both before *and* after settings
load, the dependency array [settings?.disabled_skills] never changed value on
load either, so the effect never ran a second time. hasHydratedInitialSettings
stayed false, and the save effect's early-return guard blocked every toggle.

Fix by gating on settingsLoading instead and defaulting the missing field with
?? []. Also add a localStorage stub for Node.js 25+ which ships a built-in
localStorage that is non-functional without --localstorage-file, breaking the
zustand persist middleware in the test environment.

Co-authored-by: openhands <openhands@all-hands.dev>

* fix(tests): use mockResolvedValue(true) for saveSettings spy

saveSettings returns Promise<boolean>, not Promise<void>, so
mockResolvedValue(undefined) caused TS2345 type errors in CI.

Co-authored-by: openhands <openhands@all-hands.dev>

* fix(skills): persist disabled_skills to localStorage for local backend

The local agent-server has no concept of disabled_skills and its
PATCH /api/settings strips the field before sending. As a result,
toggling a skill off appeared to work in the UI but was never
persisted -- the value was silently discarded on every save and
page refresh reverted the toggle.

Fix by mirroring the existing app-preferences pattern:

- saveSettings (local path): call writeStoredDisabledSkills before
  stripping the field from the PATCH payload.
- transformApiResponse: call readStoredDisabledSkills and overlay it
  onto the partial Settings returned from the API, so every getSettings
  call (cached or fresh) surfaces the stored value.

Export DISABLED_SKILLS_STORAGE_KEY so tests can reference the key
without duplicating the string literal.

Co-authored-by: openhands <openhands@all-hands.dev>

---------

Co-authored-by: openhands <openhands@all-hands.dev>
2026-05-27 17:49:25 -06:00

110 lines
3.4 KiB
TypeScript

import { afterAll, afterEach, beforeAll, vi } from "vitest";
import { cleanup } from "@testing-library/react";
import { server } from "#/mocks/node";
import "@testing-library/jest-dom/vitest";
if (typeof HTMLCanvasElement !== "undefined") {
HTMLCanvasElement.prototype.getContext = vi.fn();
}
if (typeof HTMLElement !== "undefined") {
HTMLElement.prototype.scrollTo = vi.fn();
}
const windowStub =
typeof window === "undefined"
? ({ event: undefined } as unknown as Window & typeof globalThis)
: window;
vi.stubGlobal("window", windowStub);
windowStub.scrollTo = vi.fn();
// Node.js 25+ ships a built-in localStorage that requires --localstorage-file
// and is not functional without it. Stub it with a plain in-memory
// implementation so zustand's persist middleware works in tests.
if (typeof localStorage === "undefined" || typeof localStorage.setItem !== "function") {
const store: Record<string, string> = {};
vi.stubGlobal("localStorage", {
getItem: (key: string) => store[key] ?? null,
setItem: (key: string, value: string) => { store[key] = String(value); },
removeItem: (key: string) => { delete store[key]; },
clear: () => { Object.keys(store).forEach((k) => delete store[k]); },
get length() { return Object.keys(store).length; },
key: (index: number) => Object.keys(store)[index] ?? null,
});
}
if (typeof requestAnimationFrame === "undefined") {
vi.stubGlobal("requestAnimationFrame", (callback: FrameRequestCallback) =>
setTimeout(() => callback(0), 0),
);
vi.stubGlobal(
"cancelAnimationFrame",
(timeoutId: ReturnType<typeof setTimeout>) => clearTimeout(timeoutId),
);
}
// Mock ResizeObserver for test environment
class MockResizeObserver {
observe = vi.fn();
unobserve = vi.fn();
disconnect = vi.fn();
}
// Mock the i18n provider
vi.mock("react-i18next", async (importOriginal) => ({
...(await importOriginal<typeof import("react-i18next")>()),
useTranslation: () => ({
t: (key: string) => key,
i18n: {
language: "en",
exists: () => false,
},
}),
}));
vi.mock("#/hooks/use-is-on-tos-page", () => ({
useIsOnTosPage: () => false,
}));
vi.mock("#/hooks/use-is-on-intermediate-page", () => ({
useIsOnIntermediatePage: () => false,
}));
// Mock useRevalidator from react-router to allow direct store manipulation in tests
vi.mock("react-router", async (importOriginal) => ({
...(await importOriginal<typeof import("react-router")>()),
useRevalidator: () => ({
revalidate: vi.fn(),
}),
}));
// Import the Zustand mock to enable automatic store resets
vi.mock("zustand");
// Mock requests during tests
beforeAll(() => {
server.listen({ onUnhandledRequest: "bypass" });
vi.stubGlobal("ResizeObserver", MockResizeObserver);
});
afterEach(async () => {
server.resetHandlers();
// Cleanup the document body after each test
cleanup();
// Drain any queued microtasks before jsdom is torn down between test files.
// Without this, async state updates queued during render (for example by
// HeroUI v2 components wrapped in framer-motion's LazyMotion) can resolve
// after `window` is gone and trigger spurious unhandled rejections in
// react-dom's `resolveUpdatePriority`. We use `Promise.resolve()` (a
// microtask) rather than `setTimeout(0)` so this stays compatible with
// tests that install fake timers.
await Promise.resolve();
await Promise.resolve();
});
afterAll(() => {
server.close();
vi.unstubAllGlobals();
});