test(ui): stabilize browser-mode test harnesses (#12301)

This commit is contained in:
Pablo Fernandez Guerra (PFE)
2026-08-05 11:37:58 +02:00
committed by GitHub
parent 5285d25cfd
commit 34431b5b88
6 changed files with 195 additions and 25 deletions
@@ -0,0 +1,61 @@
/**
* Covers only the polling contract of `BrowserHarness`, which every page
* harness inherits: a silent `null` where a predicate actually threw sends the
* caller hunting a phantom timeout. Lives in the browser project because the
* base class imports `vitest/browser`.
*/
import { describe, expect, it } from "vitest";
import { BrowserHarness } from "./browser-harness";
/** Exposes the protected waiting helpers; no fixture or DOM is involved. */
class WaitingHarness extends BrowserHarness<null> {
constructor() {
super(null);
}
probe<T>(fn: () => T | null | undefined | false): Promise<T> {
return this.waitFor(fn, 200, "probe", 10);
}
probeOrNull<T>(fn: () => T | null | undefined | false): Promise<T | null> {
return this.waitForOrNull(fn, 200, "probe");
}
}
describe("BrowserHarness waiting helpers", () => {
it("resolves to null when the predicate only ever stays falsy", async () => {
const harness = new WaitingHarness();
await expect(harness.probeOrNull<string>(() => null)).resolves.toBeNull();
});
it("rejects with the predicate's error when it throws and then goes falsy", async () => {
const harness = new WaitingHarness();
const boom = new Error("predicate blew up");
let calls = 0;
// `vi.waitFor` polls past a throw and rejects with the *last* error, so the
// falsy polls would otherwise bury `boom` under the timeout sentinel.
await expect(
harness.probeOrNull<string>(() => {
calls += 1;
if (calls === 1) throw boom;
return null;
}),
).rejects.toBe(boom);
});
it("still polls through a transient throw", async () => {
const harness = new WaitingHarness();
let calls = 0;
await expect(
harness.probe(() => {
calls += 1;
if (calls === 1) throw new Error("transient");
return "ready";
}),
).resolves.toBe("ready");
});
});
+90 -12
View File
@@ -25,6 +25,12 @@ import { userEvent } from "vitest/browser";
type RequestStartListener = (event: { request: Request }) => void;
/**
* The predicate stayed falsy for the whole timeout. `waitForOrNull` swallows
* only this, so a throwing predicate still surfaces as the bug it is.
*/
class WaitForPending extends Error {}
export abstract class BrowserHarness<TFixture> {
readonly user = userEvent;
@@ -144,20 +150,65 @@ export abstract class BrowserHarness<TFixture> {
// --- Sync helpers -------------------------------------------------------
/** Wait until the predicate returns truthy and return that value. */
/**
* Wait until the predicate returns truthy and return that value. `label`
* names what was awaited, so the timeout message identifies the caller.
*/
protected async waitFor<T>(
fn: () => T | null | undefined | false,
timeoutMs = 5000,
label?: string,
intervalMs = 30,
): Promise<T> {
return vi.waitFor(
() => {
const v = fn();
if (!v) throw new Error("waitFor predicate not yet truthy");
return v;
},
{ timeout: timeoutMs, interval: intervalMs },
) as Promise<T>;
// `vi.waitFor` rejects with the *last* callback error, so a predicate that
// throws and then goes falsy would time out as a bare `WaitForPending`.
// Keep its error so the sentinel never outranks a real one.
let predicateThrew = false;
let predicateError: unknown;
try {
return (await vi.waitFor(
() => {
let v: T | null | undefined | false;
try {
v = fn();
} catch (error) {
predicateThrew = true;
predicateError = error;
throw error;
}
if (!v) {
throw new WaitForPending(
`waitFor: timed out waiting for ${label ?? "predicate to be truthy"}`,
);
}
return v;
},
{ timeout: timeoutMs, interval: intervalMs },
)) as T;
} catch (error) {
if (error instanceof WaitForPending && predicateThrew) {
throw predicateError;
}
throw error;
}
}
/**
* Like `waitFor`, but resolves to null when the predicate never became
* truthy. A throwing predicate still propagates.
*/
protected async waitForOrNull<T>(
fn: () => T | null | undefined | false,
timeoutMs = 5000,
label?: string,
): Promise<T | null> {
try {
return await this.waitFor(fn, timeoutMs, label);
} catch (error) {
if (error instanceof WaitForPending) return null;
throw error;
}
}
protected async waitForText(
@@ -207,9 +258,36 @@ export abstract class BrowserHarness<TFixture> {
await this.user.click(btn);
}
/** Click a dropdown/menu item (rendered in a Radix portal) by its label. */
/**
* Click a dropdown/menu item (rendered in a Radix portal) by its label.
*
* Native click, not `user.click`: the portal animates in while the row behind
* it re-renders, and Playwright rejects the element as unstable or detached.
* A native click needs no stability check but is lost silently on a detached
* node, so re-resolve each attempt and stop once the menu unmounts.
*
* Only for items that close the menu — a checkbox, radio or submenu item
* needs its own helper.
*/
protected async clickMenuItem(name: RegExp): Promise<void> {
const item = await this.waitFor(() => this.byRoleName("menuitem", name));
await this.user.click(item);
for (let attempt = 0; attempt < 3; attempt += 1) {
// The menu is already mounted after the first attempt, so keep the
// re-resolve short to bound the retry tail inside the test budget.
const item = await this.waitFor(
() => this.byRoleName("menuitem", name),
attempt === 0 ? 5000 : 500,
`menu item ${name}`,
);
item.click();
const closed = await this.waitForOrNull(
() => this.q('[role="menu"]') === null,
2000,
`the menu to close after clicking ${name}`,
);
if (closed) return;
}
throw new Error(
`clickMenuItem: menu stayed open after clicking ${name} 3 times`,
);
}
}
@@ -30,8 +30,10 @@ export const QuerySelector = ({
<SelectValue placeholder="Choose a query" />
</SelectTrigger>
<SelectContent>
{/* Radix drops `value` from the DOM; `data-value` is what tests
select an option by. */}
{queries.map((query) => (
<SelectItem key={query.id} value={query.id}>
<SelectItem key={query.id} value={query.id} data-value={query.id}>
<div className="flex flex-col gap-1">
<span className="font-medium">{query.attributes.name}</span>
<span className="text-xs text-gray-500">
@@ -384,23 +384,49 @@ export class AttackPathPageHarness extends BrowserHarness<PageFixture> {
'button[role="combobox"]',
),
10000,
"the query selector trigger",
);
await this.user.click(trigger);
const targetId = queryId ?? this.fixture.queryId;
const targetName = this.fixture.queries.find((q) => q.id === targetId)
?.attributes.name;
const option = await this.waitFor<HTMLElement>(
() =>
document.querySelector<HTMLElement>(
`[role="option"][data-value="${targetId}"]`,
) ??
Array.from(
document.querySelectorAll<HTMLElement>('[role="option"]'),
).find((el) => targetName && el.textContent?.includes(targetName)),
10000,
// Id match is exact (`QuerySelector` emits `data-value`); the name fallback
// is loose, since `textContent` also covers the option's description.
const findOption = (): HTMLElement | null =>
document.querySelector<HTMLElement>(
`[role="option"][data-value="${targetId}"]`,
) ??
Array.from(
document.querySelectorAll<HTMLElement>('[role="option"]'),
).find((el) => targetName && el.textContent?.includes(targetName)) ??
null;
await this.user.click(trigger);
// A re-render landing mid-gesture makes Radix drop the open state, leaving
// the trigger focused and the listbox unmounted. Re-open from the keyboard,
// which acts on the focused trigger — but only when no option mounted at
// all, so a merely slow runner does not get a second gesture.
let option = await this.waitForOrNull(
findOption,
2000,
`option ${targetId}`,
);
if (!option && document.querySelector('[role="option"]') === null) {
await this.user.keyboard("{Enter}");
option = await this.waitForOrNull(findOption, 8000, `option ${targetId}`);
}
if (!option) {
throw new Error(
`selectQuery: no option matching id "${targetId}"` +
(targetName
? ` or name "${targetName}"`
: " (id not in the fixture)"),
);
}
await this.user.click(option);
await this.waitForTransition();
}
@@ -1,5 +1,5 @@
import { listScanConfigurations } from "@/actions/scan-configurations";
import { ProvidersAccountsView } from "@/components/providers";
import { ProvidersAccountsView } from "@/components/providers/providers-accounts-view";
import { isCloud } from "@/lib/shared/env";
import { SearchParamsProps } from "@/types";
import {
+4 -1
View File
@@ -99,6 +99,8 @@ export default defineConfig(() => {
// Without this, Vite optimizes them on demand at the first request and
// reloads the page, killing the test run. Keep this list aligned with
// imports through the page's render tree.
// Kept identical to the prowler-cloud overlay's list so it stops
// re-conflicting on sync; an entry with no importer here is deliberate.
include: [
// Test stack
"vitest-browser-react",
@@ -109,6 +111,7 @@ export default defineConfig(() => {
"react-dom/client",
// Next runtime
"next/headers",
"next/navigation",
"next/link",
"next/image",
@@ -166,7 +169,6 @@ export default defineConfig(() => {
"@tanstack/react-table",
"@react-aria/ssr",
"@react-aria/visually-hidden",
"modern-screenshot",
"framer-motion",
"cmdk",
"driver.js",
@@ -181,6 +183,7 @@ export default defineConfig(() => {
"@uiw/react-codemirror",
"@sentry/nextjs",
"@extractus/feed-extractor",
"@stripe/stripe-js",
],
},
};