test(perf): cut cron retry waits and tighten tmp guard prefilter

This commit is contained in:
Peter Steinberger
2026-03-02 11:54:26 +00:00
parent d9a8d3853d
commit c80a332def
2 changed files with 37 additions and 23 deletions

View File

@@ -1,18 +1,30 @@
import fs from "node:fs/promises"; import fs from "node:fs/promises";
import os from "node:os"; import os from "node:os";
import path from "node:path"; import path from "node:path";
import { afterEach, describe, expect, it, vi } from "vitest"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
import { loadCronStore, resolveCronStorePath, saveCronStore } from "./store.js"; import { loadCronStore, resolveCronStorePath, saveCronStore } from "./store.js";
import type { CronStoreFile } from "./types.js"; import type { CronStoreFile } from "./types.js";
let fixtureRoot = "";
let fixtureCount = 0;
beforeAll(async () => {
fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-cron-store-"));
});
afterAll(async () => {
if (!fixtureRoot) {
return;
}
await fs.rm(fixtureRoot, { recursive: true, force: true });
});
async function makeStorePath() { async function makeStorePath() {
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-cron-store-")); const dir = path.join(fixtureRoot, `case-${fixtureCount++}`);
await fs.mkdir(dir, { recursive: true });
return { return {
dir, dir,
storePath: path.join(dir, "jobs.json"), storePath: path.join(dir, "jobs.json"),
cleanup: async () => {
await fs.rm(dir, { recursive: true, force: true });
},
}; };
} }
@@ -56,14 +68,12 @@ describe("cron store", () => {
const store = await makeStorePath(); const store = await makeStorePath();
const loaded = await loadCronStore(store.storePath); const loaded = await loadCronStore(store.storePath);
expect(loaded).toEqual({ version: 1, jobs: [] }); expect(loaded).toEqual({ version: 1, jobs: [] });
await store.cleanup();
}); });
it("throws when store contains invalid JSON", async () => { it("throws when store contains invalid JSON", async () => {
const store = await makeStorePath(); const store = await makeStorePath();
await fs.writeFile(store.storePath, "{ not json", "utf-8"); await fs.writeFile(store.storePath, "{ not json", "utf-8");
await expect(loadCronStore(store.storePath)).rejects.toThrow(/Failed to parse cron store/i); await expect(loadCronStore(store.storePath)).rejects.toThrow(/Failed to parse cron store/i);
await store.cleanup();
}); });
it("does not create a backup file when saving unchanged content", async () => { it("does not create a backup file when saving unchanged content", async () => {
@@ -74,7 +84,6 @@ describe("cron store", () => {
await saveCronStore(store.storePath, payload); await saveCronStore(store.storePath, payload);
await expect(fs.stat(`${store.storePath}.bak`)).rejects.toThrow(); await expect(fs.stat(`${store.storePath}.bak`)).rejects.toThrow();
await store.cleanup();
}); });
it("backs up previous content before replacing the store", async () => { it("backs up previous content before replacing the store", async () => {
@@ -89,7 +98,6 @@ describe("cron store", () => {
const backupRaw = await fs.readFile(`${store.storePath}.bak`, "utf-8"); const backupRaw = await fs.readFile(`${store.storePath}.bak`, "utf-8");
expect(JSON.parse(currentRaw)).toEqual(second); expect(JSON.parse(currentRaw)).toEqual(second);
expect(JSON.parse(backupRaw)).toEqual(first); expect(JSON.parse(backupRaw)).toEqual(first);
await store.cleanup();
}); });
}); });
@@ -97,16 +105,19 @@ describe("saveCronStore", () => {
const dummyStore: CronStoreFile = { version: 1, jobs: [] }; const dummyStore: CronStoreFile = { version: 1, jobs: [] };
it("persists and round-trips a store file", async () => { it("persists and round-trips a store file", async () => {
const { storePath, cleanup } = await makeStorePath(); const { storePath } = await makeStorePath();
await saveCronStore(storePath, dummyStore); await saveCronStore(storePath, dummyStore);
const loaded = await loadCronStore(storePath); const loaded = await loadCronStore(storePath);
expect(loaded).toEqual(dummyStore); expect(loaded).toEqual(dummyStore);
await cleanup();
}); });
it("retries rename on EBUSY then succeeds", async () => { it("retries rename on EBUSY then succeeds", async () => {
const { storePath, cleanup } = await makeStorePath(); const { storePath } = await makeStorePath();
const realSetTimeout = globalThis.setTimeout;
const setTimeoutSpy = vi
.spyOn(globalThis, "setTimeout")
.mockImplementation(((handler: TimerHandler, _timeout?: number, ...args: unknown[]) =>
realSetTimeout(handler, 0, ...args)) as typeof setTimeout);
const origRename = fs.rename.bind(fs); const origRename = fs.rename.bind(fs);
let ebusyCount = 0; let ebusyCount = 0;
const spy = vi.spyOn(fs, "rename").mockImplementation(async (src, dest) => { const spy = vi.spyOn(fs, "rename").mockImplementation(async (src, dest) => {
@@ -119,17 +130,20 @@ describe("saveCronStore", () => {
return origRename(src, dest); return origRename(src, dest);
}); });
await saveCronStore(storePath, dummyStore); try {
expect(ebusyCount).toBe(2); await saveCronStore(storePath, dummyStore);
const loaded = await loadCronStore(storePath);
expect(loaded).toEqual(dummyStore);
spy.mockRestore(); expect(ebusyCount).toBe(2);
await cleanup(); const loaded = await loadCronStore(storePath);
expect(loaded).toEqual(dummyStore);
} finally {
spy.mockRestore();
setTimeoutSpy.mockRestore();
}
}); });
it("falls back to copyFile on EPERM (Windows)", async () => { it("falls back to copyFile on EPERM (Windows)", async () => {
const { storePath, cleanup } = await makeStorePath(); const { storePath } = await makeStorePath();
const spy = vi.spyOn(fs, "rename").mockImplementation(async () => { const spy = vi.spyOn(fs, "rename").mockImplementation(async () => {
const err = new Error("EPERM") as NodeJS.ErrnoException; const err = new Error("EPERM") as NodeJS.ErrnoException;
@@ -142,6 +156,5 @@ describe("saveCronStore", () => {
expect(loaded).toEqual(dummyStore); expect(loaded).toEqual(dummyStore);
spy.mockRestore(); spy.mockRestore();
await cleanup();
}); });
}); });

View File

@@ -146,8 +146,9 @@ function isOsTmpdirExpression(argument: string): boolean {
function mightContainDynamicTmpdirJoin(source: string): boolean { function mightContainDynamicTmpdirJoin(source: string): boolean {
return ( return (
source.includes("path") && source.includes("path") &&
source.includes("join") && source.includes("path.join") &&
source.includes("tmpdir") && source.includes("os.tmpdir") &&
source.includes("`") &&
source.includes("${") source.includes("${")
); );
} }