perf(snapshots): skip retries on comparison pass, suppress consent modal (#505)

- Add --retries=0 to test:e2e:snapshots: snapshot pixel-diff failures are
  deterministic — retrying cannot fix them. On a PR that changes 36/60
  snapshots this tripled execution count and added ~2 min to the comparison
  pass.

- Extract seedLocalStorage() helper (tests/e2e/snapshots/support/) that
  seeds openhands-onboarded and openhands-telemetry-consent in a single
  addInitScript call. All 13 snapshot specs now use it instead of
  duplicated inline addInitScript blocks.

- Pre-seeding openhands-telemetry-consent='denied' eliminates the race
  condition where changes-tab timed out at 60 s (x3 retries = 3 min) because
  dismissConsentModal fired before the modal rendered with domcontentloaded.

Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
Rohit Malhotra
2026-05-16 00:04:29 -04:00
committed by GitHub
co-authored by openhands
parent 14d2e9454b
commit acb04fd9ba
15 changed files with 104 additions and 61 deletions
+1 -1
View File
@@ -82,7 +82,7 @@
"test": "npm run make-i18n && vitest run",
"test:e2e": "playwright test --pass-with-no-tests",
"test:e2e:live": "node --env-file-if-exists=.env tests/e2e/live/scripts/run-live-e2e.mjs",
"test:e2e:snapshots": "playwright test tests/e2e/snapshots --project=chromium",
"test:e2e:snapshots": "playwright test tests/e2e/snapshots --project=chromium --retries=0",
"test:e2e:snapshots:update": "playwright test tests/e2e/snapshots --project=chromium --update-snapshots",
"test:coverage": "npm run make-i18n && vitest run --coverage",
"dev_wsl": "VITE_WATCH_USE_POLLING=true vite",
@@ -1,4 +1,5 @@
import { test, expect, Page } from "@playwright/test";
import { seedLocalStorage } from "./support/seed-local-storage";
/**
* Visual snapshot tests for the Automations pages.
@@ -39,9 +40,7 @@ async function dismissConsentModal(page: Page) {
* here with INLINE_AUTOMATIONS so all tests have a consistent baseline.
*/
async function setupMocks(page: Page) {
await page.addInitScript(() => {
window.localStorage.setItem("openhands-onboarded", "true");
});
await seedLocalStorage(page);
await page.route("**/api/conversations/search**", async (route) => {
await route.fulfill({
@@ -1,5 +1,6 @@
import { test, expect, type Page } from "@playwright/test";
import type { Backend } from "../../../src/api/backend-registry/types";
import { seedLocalStorage } from "./support/seed-local-storage";
/**
* Extended visual snapshot tests for the backend management UI.
@@ -77,21 +78,16 @@ async function setupPage(
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,
},
);
const extra: [string, string][] = [
["openhands-backends", JSON.stringify(backends)],
];
if (activeBackendId) {
extra.push([
"openhands-active-backend",
JSON.stringify({ backendId: activeBackendId, orgId: null }),
]);
}
await seedLocalStorage(page, { extra });
// Prevent workspace-scan 404s in the sidebar from cluttering timing.
await page.route("**/api/file/**", (route) =>
@@ -1,4 +1,5 @@
import { test, expect, Page } from "@playwright/test";
import { seedLocalStorage } from "./support/seed-local-storage";
/**
* Visual snapshot tests for the backend management UI.
@@ -27,9 +28,7 @@ async function dismissConsentModal(page: Page) {
}
async function setupMocks(page: Page) {
await page.addInitScript(() => {
window.localStorage.setItem("openhands-onboarded", "true");
});
await seedLocalStorage(page);
// Suppress file-API proxy errors emitted when the home page scans the
// workspace directory (same suppression used in sidebar.snapshot.spec.ts).
@@ -1,4 +1,5 @@
import { test, expect, Page } from "@playwright/test";
import { seedLocalStorage } from "./support/seed-local-storage";
/**
* Visual snapshot tests for the Changes (diff viewer) UI.
@@ -52,13 +53,9 @@ const CONVERSATION_STATE_VALUE = JSON.stringify({
* 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],
);
await seedLocalStorage(page, {
extra: [[CONVERSATION_STATE_KEY, CONVERSATION_STATE_VALUE]],
});
// Stub WebSocket so the conversation page doesn't hang waiting for a real
// socket connection. Copied from collapsible-thinking.snapshot.spec.ts.
@@ -1,4 +1,5 @@
import { test, expect, Page } from "@playwright/test";
import { seedLocalStorage } from "./support/seed-local-storage";
/**
* Visual snapshot tests for the CollapsibleThinking component.
@@ -173,10 +174,7 @@ async function injectEvents(page: Page, events: unknown[]) {
* Uses mock conversation "1" which exists in the MSW handlers.
*/
async function navigateToConversation(page: Page, events: unknown[]) {
// Skip onboarding
await page.addInitScript(() => {
window.localStorage.setItem("openhands-onboarded", "true");
});
await seedLocalStorage(page);
await page.route("**/api/bash/execute_bash_command", async (route) => {
await route.fulfill({
@@ -1,13 +1,14 @@
import { expect, test } from "@playwright/test";
import { seedLocalStorage } from "./support/seed-local-storage";
test("captures Docker /projects workspace browser state", async ({ page }) => {
test.setTimeout(60_000);
await page.addInitScript(() => {
window.localStorage.setItem("analytics-consent", "true");
window.localStorage.setItem("openhands-telemetry-consent", "denied");
window.localStorage.setItem("openhands-telemetry-first-use", "true");
window.localStorage.setItem("openhands-onboarded", "1");
await seedLocalStorage(page, {
extra: [
["analytics-consent", "true"],
["openhands-telemetry-first-use", "true"],
],
});
await page.goto("/conversations", { waitUntil: "domcontentloaded" });
@@ -1,4 +1,5 @@
import { test, expect, Page } from "@playwright/test";
import { seedLocalStorage } from "./support/seed-local-storage";
/**
* Visual snapshot tests for the MCP page (/mcp).
@@ -32,9 +33,7 @@ async function dismissConsentModal(page: Page) {
* instead of trying to suppress it here.
*/
async function setupMocks(page: Page) {
await page.addInitScript(() => {
window.localStorage.setItem("openhands-onboarded", "true");
});
await seedLocalStorage(page);
await page.route("**/api/conversations/search**", async (route) => {
await route.fulfill({
@@ -1,4 +1,5 @@
import { test, expect, Page } from "@playwright/test";
import { seedLocalStorage } from "./support/seed-local-storage";
/**
* Visual snapshot tests for the 4-step onboarding modal.
@@ -26,10 +27,9 @@ import { test, expect, Page } from "@playwright/test";
test.describe.configure({ mode: "serial" });
async function setupMocks(page: Page) {
// Intentionally do NOT set openhands-onboarded so the modal appears
await page.addInitScript(() => {
window.localStorage.removeItem("openhands-onboarded");
});
// removeOnboarded: true ensures the onboarding modal appears.
// Analytics consent modal is suppressed (separate concern).
await seedLocalStorage(page, { removeOnboarded: true });
}
async function dismissConsentModal(page: Page) {
@@ -1,4 +1,5 @@
import { test, expect, Page } from "@playwright/test";
import { seedLocalStorage } from "./support/seed-local-storage";
/**
* Visual snapshot tests for UI pages.
@@ -46,10 +47,7 @@ const SETTINGS_WITHOUT_CONSENT = {
* @param showConsentModal - Whether to show the analytics consent modal
*/
async function setupMocks(page: Page, showConsentModal = false) {
// Pre-set localStorage to skip onboarding
await page.addInitScript(() => {
window.localStorage.setItem("openhands-onboarded", "true");
});
await seedLocalStorage(page, { showConsentModal });
// Mock settings API - consent modal appears when user_consents_to_analytics is null
const settingsResponse = showConsentModal
@@ -1,4 +1,5 @@
import { test, expect, Page } from "@playwright/test";
import { seedLocalStorage } from "./support/seed-local-storage";
/**
* Visual snapshot tests for the Secrets Settings page (/settings/secrets).
@@ -23,9 +24,7 @@ async function dismissConsentModal(page: Page) {
}
async function setupMocks(page: Page) {
await page.addInitScript(() => {
window.localStorage.setItem("openhands-onboarded", "true");
});
await seedLocalStorage(page);
// Keep conversations sidebar quiet (page.route wins for this cross-origin path
// only; for same-origin MSW takes precedence and we rely on MSW data).
await page.route("**/api/conversations/search**", async (route) => {
@@ -1,4 +1,5 @@
import { test, expect, Page } from "@playwright/test";
import { seedLocalStorage } from "./support/seed-local-storage";
/**
* Visual snapshot tests for:
@@ -23,9 +24,7 @@ async function dismissConsentModal(page: Page) {
}
async function setupMocks(page: Page) {
await page.addInitScript(() => {
window.localStorage.setItem("openhands-onboarded", "true");
});
await seedLocalStorage(page);
}
test.describe("Settings – Verification & Condenser Visual Snapshots", () => {
+2 -3
View File
@@ -1,4 +1,5 @@
import { test, expect, Page } from "@playwright/test";
import { seedLocalStorage } from "./support/seed-local-storage";
/**
* Visual snapshot tests for the sidebar / conversation panel.
@@ -22,9 +23,7 @@ async function dismissConsentModal(page: Page) {
}
async function setupMocks(page: Page) {
await page.addInitScript(() => {
window.localStorage.setItem("openhands-onboarded", "true");
});
await seedLocalStorage(page);
// Suppress proxy errors for file API (home page workspace scan)
await page.route("**/api/file/**", async (route) => {
await route.fulfill({
@@ -1,4 +1,5 @@
import { test, expect, Page } from "@playwright/test";
import { seedLocalStorage } from "./support/seed-local-storage";
/**
* Visual snapshot tests for the Skills page (/skills).
@@ -78,9 +79,7 @@ async function dismissConsentModal(page: Page) {
* Wire up the base routes every skills test needs.
*/
async function setupMocks(page: Page) {
await page.addInitScript(() => {
window.localStorage.setItem("openhands-onboarded", "true");
});
await seedLocalStorage(page);
}
/**
@@ -0,0 +1,60 @@
import type { Page } from "@playwright/test";
export interface SeedLocalStorageOptions {
/**
* Remove openhands-onboarded instead of setting it.
* Use in onboarding modal tests that need the modal to appear.
*/
removeOnboarded?: boolean;
/**
* Skip suppressing the analytics consent modal.
* Use in tests that specifically snapshot the consent modal UI.
*/
showConsentModal?: boolean;
/**
* Additional [key, value] pairs to seed alongside the standard keys.
*/
extra?: [string, string][];
}
/**
* Seeds the standard localStorage keys required by snapshot tests via
* `page.addInitScript`, so the values are present before any app code runs.
*
* Defaults (overridable via options):
* - openhands-onboarded = "1" (suppresses onboarding modal)
* - openhands-telemetry-consent = "denied" (suppresses analytics consent modal)
*/
export async function seedLocalStorage(
page: Page,
{
removeOnboarded = false,
showConsentModal = false,
extra = [],
}: SeedLocalStorageOptions = {},
) {
await page.addInitScript(
({
removeOnboarded,
showConsentModal,
extra,
}: {
removeOnboarded: boolean;
showConsentModal: boolean;
extra: [string, string][];
}) => {
if (removeOnboarded) {
window.localStorage.removeItem("openhands-onboarded");
} else {
window.localStorage.setItem("openhands-onboarded", "1");
}
if (!showConsentModal) {
window.localStorage.setItem("openhands-telemetry-consent", "denied");
}
for (const [key, value] of extra) {
window.localStorage.setItem(key, value);
}
},
{ removeOnboarded, showConsentModal, extra },
);
}