commit 947f28fb8f960533864fa9abf4f225cfe89426c4
parent 21ccd05e1c72bdc66f8e44cb54c96e3b7c038fd0
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Fri, 11 Sep 2026 20:36:32 -0400
e2e: a spec's settings are the fixture's plus what it names
writeSettings REPLACED test-settings.json, so every key in
fixtures/test-settings.default.json — which resetData() had copied in moments
earlier — was gone the instant a spec called it. The keys did not fall back to
something neutral. They fell back to the PRODUCT defaults, which is a different
fixture, chosen for operators and not for a test host, and four of them diverge:
minFreeDiskGB 0 vs 5 GB
sleepBetweenDownloadsSeconds 0 vs 10 s
verifyAvailabilityBeforeClean false vs true
syncScheduler.fullSweepIntervalMinutes 0 vs 1440
Eighteen spec files write settings without naming the floor; fourteen without
naming the sleep. None of them is about disk or about pacing. The floor arms the
low-disk gate, so those specs' media actions depend on how much room the host has
left — silently, because the refusal is a returned `{ ok: false }` from
pipelineActions.lowDiskError() that no assertion reads. The sleep puts ten
seconds between every download in a fixture batch, which is the difference
between a spec that finishes and one that times out on a slow machine.
The previous commit defaulted the one key. That was the symptom. This merges onto
the fixture, one level deep for nested blocks (syncScheduler is the only one the
fixture sets, and the two specs that name it name everything except the one key),
arrays and every other object replaced rather than merged — `workers: []` has to
mean no workers, and a half-merged policy tree would be a worse surprise than a
replaced one. Explicit wins at every level, which is what disk-space.spec.ts,
widget.spec.ts and backfill.spec.ts's floor case rely on.
Checked before running: of the six keys the fixture sets, adminTitle and
maxTranscriptPageBytes are now inherited too — no spec asserts the admin title,
and 8388608 IS TRANSCRIPT_PAGE_DEFAULT_BYTES, so neither changes a behaviour.
pre-clean-availability.spec.ts names verifyAvailabilityBeforeClean: true itself.
The harness case now asserts both diverging numbers where the operator would read
them, and that a named value still wins with the fixture's others still
underneath it.
editor + common tsc clean; common 971/971; channel-storage + disk-space +
scheduler --repeat-each 3: 54/54.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
4 files changed, 92 insertions(+), 37 deletions(-)
diff --git a/editor/e2e/disk-space.spec.ts b/editor/e2e/disk-space.spec.ts
@@ -275,25 +275,39 @@ test("active-jobs API and monitor widget report low disk", async ({ page }) => {
// THE HARNESS'S OWN INVARIANT, pinned here because breaking it is invisible.
//
-// writeSettings() replaces test-settings.json wholesale rather than merging it
-// with fixtures/test-settings.default.json, so a spec that does not name
-// minFreeDiskGB used to inherit the PRODUCT default (5 GB) instead of the
-// fixture's 0 — arming the low-disk gate for every action in that spec and
-// making its result depend on how much room the host has left. Eighteen spec
-// files write settings without naming the key; none of them is about disk.
+// writeSettings() used to REPLACE test-settings.json rather than merge onto
+// fixtures/test-settings.default.json, so a spec that did not name a key
+// inherited the PRODUCT default instead of the fixture's — a different set of
+// numbers, chosen for operators and not for a test host. Two of them cost:
+// minFreeDiskGB 5 GB instead of 0 arms the low-disk gate, so every media action
+// in that spec depends on how much room the host has left; and
+// sleepBetweenDownloadsSeconds 10 instead of 0 puts ten seconds between every
+// download in a fixture batch. Eighteen spec files write settings without
+// naming the floor and fourteen without naming the sleep; none of them is about
+// either.
//
-// The failure that costs is not a red assertion about disk, it is a red
-// assertion about something else: the preflight in pipelineActions returns
-// `{ ok: false }`, the action never runs, and the spec reports an empty list.
-// This asserts the fixture's intent survives a spec's own write.
-test("a spec that names no disk floor gets the gate switched off", async ({
+// The failure that costs is not a red assertion about disk or about time, it is
+// a red assertion about something else: the preflight in pipelineActions returns
+// `{ ok: false }`, the action never runs, and the spec reports an empty list —
+// or the batch simply does not finish inside the timeout. This asserts the
+// fixture's intent survives a spec's own write, and that a spec's own value
+// still wins over it.
+test("a spec's settings are the fixture's plus what it names", async ({
page,
}) => {
await resetData("test-pipeline");
await writeSettings({ adminTitle: "Test Admin" });
await page.goto("/settings");
+ // Unnamed: both come from the fixture, not from the product defaults (5, 10).
await expect(page.locator('input[name="minFreeDiskGB"]')).toHaveValue("0");
+ await expect(
+ page.locator('input[name="sleepBetweenDownloadsSeconds"]'),
+ ).toHaveValue("0");
+ // Named: the spec's own value survives the merge.
+ await expect(page.locator('input[name="adminTitle"]')).toHaveValue(
+ "Test Admin",
+ );
// And the gate itself agrees — the same payload the indicator reads.
const res = await page.request.get("/api/jobs/active");
@@ -302,4 +316,15 @@ test("a spec that names no disk floor gets the gate switched off", async ({
};
expect(payload.disk.enabled).toBe(false);
expect(payload.disk.low).toBe(false);
+
+ // A named floor still wins, which is what every case above this one needs.
+ await writeSettings({ minFreeDiskGB: HUGE_FLOOR_GB });
+ await page.goto("/settings");
+ await expect(page.locator('input[name="minFreeDiskGB"]')).toHaveValue(
+ String(HUGE_FLOOR_GB),
+ );
+ // ...and the fixture's other keys are still underneath it.
+ await expect(
+ page.locator('input[name="sleepBetweenDownloadsSeconds"]'),
+ ).toHaveValue("0");
});
diff --git a/editor/e2e/helpers.ts b/editor/e2e/helpers.ts
@@ -73,27 +73,55 @@ export async function resetData(fixtureName: string | null = null) {
await fetch(`${baseUrl}/api/test/invalidate-cache`).catch(() => {});
}
-// THE DISK FLOOR IS OFF IN TESTS UNLESS A SPEC ASKS FOR IT.
+// A SPEC'S SETTINGS ARE THE FIXTURE'S PLUS WHAT THE SPEC NAMES.
//
-// This writes the settings file WHOLESALE — it does not merge with
-// fixtures/test-settings.default.json, which resetData() copied in moments
-// earlier — so every key that fixture sets is gone the instant a spec calls
-// this. For `minFreeDiskGB` that does not fall back to something neutral: it
-// falls back to the PRODUCT default, MIN_FREE_DISK_GB_DEFAULT = 5 GB, which
-// arms the low-disk gate and makes the spec's outcome depend on how much room
-// the HOST has left. Eighteen spec files call this without naming the key, and
-// on a nearly-full disk their every media action is refused by the preflight in
-// pipelineActions.lowDiskError() — silently, because the refusal is a returned
-// `{ ok: false }` that no assertion reads. (channel-storage.spec.ts writes the
-// key out by hand for exactly this reason; this is that fix, for everyone.)
+// This used to write the file WHOLESALE, which meant every key in
+// fixtures/test-settings.default.json — copied in by resetData() moments earlier
+// — was gone the instant a spec called this. The keys did not fall back to
+// something neutral: they fell back to the PRODUCT defaults, which is a
+// different fixture, chosen for operators and not for a test host. Four of them
+// diverge, and each one makes a spec depend on something it never mentions:
//
-// An explicit value still wins, which is what disk-space.spec.ts and
-// backfill.spec.ts's "the disk floor refuses to re-acquire anything" rely on.
-export async function writeSettings(settings: Record<string, unknown>) {
- await writeFile(
- testSettingsFile,
- JSON.stringify({ minFreeDiskGB: 0, ...settings }, null, 2),
- );
+// minFreeDiskGB 0 vs 5 GB — arms the low-disk gate,
+// so every media action in the spec depends on how much room the HOST has
+// left, and the refusal is a returned `{ ok: false }` from
+// pipelineActions.lowDiskError() that no assertion reads.
+// sleepBetweenDownloadsSeconds 0 vs 10 s — 10 s between every
+// download in a fixture batch, which is the difference between a spec that
+// finishes and one that times out on a slow host.
+// verifyAvailabilityBeforeClean false vs true — an extra source probe on
+// the cleanup path.
+// syncScheduler.fullSweepIntervalMinutes 0 vs 1440 — whether a never-swept
+// channel is due for a full sweep or the cheap paged walk.
+//
+// Fourteen spec files call this without naming the sleep; eighteen without
+// naming the floor. Merging is what makes "a spec writes the settings it cares
+// about" true. EXPLICIT WINS at every level, which is what disk-space.spec.ts,
+// widget.spec.ts and backfill.spec.ts's "the disk floor refuses to re-acquire
+// anything" rely on.
+//
+// ONE LEVEL DEEP, and no deeper. `syncScheduler` is the only nested block the
+// fixture sets, and a spec that names it (scheduler.spec.ts, cadence-ui.spec.ts)
+// names the whole scheduler except that one key. Arrays and every other object
+// are REPLACED, not merged — `workers: []` has to mean no workers, and a
+// half-merged policy tree would be a worse surprise than a replaced one.
+type SettingsPatch = Record<string, unknown>;
+
+function isPlainObject(v: unknown): v is SettingsPatch {
+ return typeof v === "object" && v !== null && !Array.isArray(v);
+}
+
+export async function writeSettings(settings: SettingsPatch) {
+ const base = JSON.parse(
+ await readFile(defaultTestSettingsFile, "utf8"),
+ ) as SettingsPatch;
+ const merged: SettingsPatch = { ...base, ...settings };
+ for (const [key, value] of Object.entries(settings)) {
+ if (isPlainObject(value) && isPlainObject(base[key])) {
+ merged[key] = { ...base[key], ...value };
+ }
+ }
+ await writeFile(testSettingsFile, JSON.stringify(merged, null, 2));
await fetch(`${baseUrl}/api/test/invalidate-cache`).catch(() => {});
}
diff --git a/editor/e2e/widget.spec.ts b/editor/e2e/widget.spec.ts
@@ -82,12 +82,14 @@ const BACKFILL_SETTINGS = {
const TWO_WORKERS = {
// NAMED, not inherited. Several cases below reach for the widget's "Disk
- // space" cell, which the layout only draws while the gate is CONFIGURED —
- // and writeSettings replaces test-settings.default.json wholesale, so this
- // spec used to get the gate armed by accident, from the product default that
- // filled the gap the fixture's 0 left. It is a positive floor because the
- // cell has to exist, not because the number matters: every assertion here is
- // about position, and the cell renders whether or not the floor is met.
+ // space" cell, which the layout only draws while the gate is CONFIGURED, and
+ // the e2e fixture switches the gate OFF (minFreeDiskGB 0) as every other spec
+ // wants. This spec used to get it armed by accident — writeSettings replaced
+ // the fixture wholesale and the product default filled the gap. It merges
+ // now, so the one spec that wants the gate on says so. A positive floor
+ // because the cell has to exist, not because the number matters: every
+ // assertion here is about position, and the cell renders whether or not the
+ // floor is met.
minFreeDiskGB: 5,
workers: [
{ id: "gpu", name: "GPU", kind: "local", enabled: true, priority: 0, appId: "whisper-cpp", config: {} },
diff --git a/plans/FACTS.md b/plans/FACTS.md
@@ -3599,7 +3599,7 @@ mistaken for the media and `deleteChannel`/`renameChannel` can recognise a targe
cwd-relative writes, the LMDB index (mtimes, which `rsync -a` preserves) and the export build
already use, and the only `lstat`/`readlink`/`realpath`/`symlink` calls anywhere in `common/`,
`editor/` or `export/` are in `channelMedia.ts`, `relocateChannelMedia.ts` and
-`renameChannel.ts` — every other hit is a comment. What that buys
+`renameChannel.ts` — every other hit is a comment or a test. What that buys
in call-site churn it owes in one failure mode — a dangling link reads as ENOENT and every
enumerator swallows ENOENT as "this channel has no videos", which to a runner means
*everything is undownloaded* — so `inspectChannelMedia` (`channelMedia.ts:191`, two stats and
