commit cee2a55f7b3d1c7cee0cf506ab983e69f4c7b3cb
parent 4e7d84fe910e3e4707c274b47f164a1f0d20ec15
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Thu, 17 Sep 2026 15:06:44 -0400
storage: two accessible names where one contains the other is a trap
Playwright's getByLabel is a SUBSTRING match, and the first e2e run walked
straight into it: "location id" is inside "location identity", "location
root" inside "location root path", so filling the add form targeted a
read-only <dd> instead. The read-outs are renamed ("location path", "volume
identity"); the form fields keep the names the plan's ARIA contract fixes.
It is not only a test problem — a screen reader user hears two controls whose
names shadow each other for the same reason.
And the re-point spec was waiting for the wrong thing. The uuid is on the
page before anybody clicks: the server render probes too. So the settled
condition is now the SETTINGS FILE — the one thing only the action can do is
write the identity, which is what a refresh is for. As written, the test
passed without ever pressing the button.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
2 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/editor/app/storage/components/StorageLocationsTable.tsx b/editor/app/storage/components/StorageLocationsTable.tsx
@@ -166,12 +166,19 @@ function LocationCard({
</div>
<dl className="grid grid-cols-[max-content_1fr] gap-x-4 gap-y-1 text-sm">
+ {/* NOT "location root" and NOT "location identity". Playwright's
+ getByLabel is a SUBSTRING match, so those names would each swallow a
+ form field on this very page — "location id" is inside "location
+ identity", "location root" inside "location root path" — and a spec
+ filling the form would silently target a read-only <dd>. Two
+ accessible names where one contains the other are a trap for screen
+ reader users for the same reason. */}
<dt className="text-muted-foreground">Root</dt>
- <dd className="font-mono text-xs break-all" aria-label="location root path">
+ <dd className="font-mono text-xs break-all" aria-label="location path">
{row.root}
</dd>
<dt className="text-muted-foreground">Volume</dt>
- <dd className="text-xs" aria-label="location identity">
+ <dd className="text-xs" aria-label="volume identity">
{row.identity ?? "unknown — nothing here can ask (no findmnt, or a container)"}
</dd>
<dt className="text-muted-foreground">Channels</dt>
diff --git a/editor/e2e/storage-locations.spec.ts b/editor/e2e/storage-locations.spec.ts
@@ -64,6 +64,13 @@ async function relocateOnDisk(root: string): Promise<string> {
return target;
}
+async function storedVolumeUuid(id: string): Promise<string | undefined> {
+ const s = await readJson<{
+ storage: { locations: Array<{ id: string; volume?: { uuid?: string } }> };
+ }>("test-settings.json");
+ return s.storage.locations.find((l) => l.id === id)?.volume?.uuid;
+}
+
function row(page: Page, id: string) {
return page.getByLabel(`storage location: ${id}`);
}
@@ -137,11 +144,11 @@ test("the page lists a location with its status, counts and refusals", async ({
await mkdir(second, { recursive: true });
await clickUntil(
page.getByRole("button", { name: "New location" }),
- () => page.getByLabel("location id").isVisible(),
+ () => page.getByLabel("location id", { exact: true }).isVisible(),
);
- await page.getByLabel("location id").fill("warm");
- await page.getByLabel("location label").fill("Warm");
- await page.getByLabel("location root").fill(second);
+ await page.getByLabel("location id", { exact: true }).fill("warm");
+ await page.getByLabel("location label", { exact: true }).fill("Warm");
+ await page.getByLabel("location root", { exact: true }).fill(second);
await page.getByLabel("auto re-point").check();
await page.getByLabel("add storage location").click();
@@ -191,19 +198,17 @@ test("a volume that came up somewhere else is re-pointed in one click", async ({
await page.goto("/storage");
const cold = row(page, "cold");
await expect(cold.getByLabel("location status")).toHaveText("Available");
- await clickUntil(cold.getByLabel("refresh cold"), async () =>
- (await cold.getByLabel("location identity").textContent())?.includes(UUID) ??
- false,
+ // THE SETTLED CONDITION IS THE FILE, NOT THE SCREEN. The server render probes
+ // too, so the uuid is already on the page before anybody clicks anything —
+ // waiting for it would pass without the button ever being pressed. What only
+ // the action can do is WRITE the identity, and that is what a refresh is for:
+ // it writes identity and nothing else (availability is never stored, or every
+ // page load would bump the pulse revision).
+ await clickUntil(
+ cold.getByLabel("refresh cold"),
+ async () => (await storedVolumeUuid("cold")) === UUID,
);
- // A refresh writes IDENTITY and only identity — availability is never stored.
- await expect
- .poll(async () => {
- const s = await readJson<{
- storage: { locations: Array<{ id: string; volume?: { uuid?: string } }> };
- }>("test-settings.json");
- return s.storage.locations.find((l) => l.id === "cold")?.volume?.uuid;
- }, { timeout: 15_000 })
- .toBe(UUID);
+ await expect(cold.getByLabel("volume identity")).toContainText(UUID);
await expect(cold.getByLabel("location warning")).toContainText("automount");
// --- the disk comes back somewhere else ---------------------------------
