From b287c28b2a0085003fdf563ca95ac074449b80e5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 16 Aug 2026 09:18:06 +0000 Subject: [PATCH 1/3] test(cron): do not let leftover crontab entries fail the Linux suites The Linux crontab tests asserted against the whole user crontab, e.g. `expect(crontab).not.toContain("30 2 * * 1")`, and restored whatever the crontab held before each test. An entry left behind by a run that was killed mid-test (or any pre-existing job with the same schedule) therefore failed the removal and replace tests on every later run of the file. Scope the assertions to the lines Bun.cron installs for the title under test, strip this file's own stale entries before the suites run, and make writeCrontab() surface a failed restore instead of ignoring it. The titles registered by this file all use the test- prefix so the stale-entry sweep cannot touch anything else in the crontab. --- test/js/bun/cron/cron.test.ts | 145 +++++++++++++++++++++++++--------- 1 file changed, 108 insertions(+), 37 deletions(-) diff --git a/test/js/bun/cron/cron.test.ts b/test/js/bun/cron/cron.test.ts index 4bca6d3f9bd4..6b3fda6f0c69 100644 --- a/test/js/bun/cron/cron.test.ts +++ b/test/js/bun/cron/cron.test.ts @@ -74,7 +74,10 @@ function writeCrontab(content: string) { const tmpFile = `/tmp/bun-cron-${Date.now()}-${Math.random().toString(36).slice(2)}.tmp`; writeFileSync(tmpFile, content); try { - Bun.spawnSync({ cmd: [crontabPath!, tmpFile] }); + const result = Bun.spawnSync({ cmd: [crontabPath!, tmpFile], stdout: "pipe", stderr: "pipe" }); + if (result.exitCode !== 0) { + throw new Error(`crontab exited with ${result.exitCode}: ${result.stderr.toString()}`); + } } finally { try { unlinkSync(tmpFile); @@ -91,6 +94,38 @@ function saveCrontabState(): Disposable { }; } +// Every title this file registers starts with this prefix, so entries left +// behind by a run that was killed mid-test (nothing restores the crontab then) +// can be removed without touching the user's own jobs. +const TEST_TITLE_PREFIX = "test-"; + +/** + * The lines Bun.cron installs for `title`: its `# bun-cron: ` marker and + * the command line after it. Assertions go through this rather than searching + * the whole crontab, which also holds whatever the user (or an earlier, killed + * run of this file) has installed. + */ +function entryLines(crontab: string, title: string): string[] { + return crontab + .split("\n") + .filter(line => line === `# bun-cron: ${title}` || line.includes(` --cron-title=${title} `)); +} + +function removeStaleTestEntries() { + const lines = readCrontab().split("\n"); + const kept: string[] = []; + for (let i = 0; i < lines.length; i++) { + if (lines[i].startsWith(`# bun-cron: ${TEST_TITLE_PREFIX}`)) { + i++; // skip the command line that belongs to the marker + } else { + kept.push(lines[i]); + } + } + if (kept.length !== lines.length) writeCrontab(kept.join("\n")); +} + +if (hasCrontab) beforeAll(removeStaleTestEntries); + // ========================================================================== // API shape // ========================================================================== @@ -544,10 +579,10 @@ describe.skipIf(!hasCrontab)("cron registration (Linux)", () => { const scriptPath = `${dir}/job.ts`; await Bun.cron(scriptPath, "30 2 * * 1", "test-register"); - const crontab = readCrontab(); - expect(crontab).toContain("# bun-cron: test-register"); - expect(crontab).toContain("30 2 * * 1"); - expect(crontab).toContain(scriptPath); + const [marker, command] = entryLines(readCrontab(), "test-register"); + expect(marker).toBe("# bun-cron: test-register"); + expect(command).toStartWith("30 2 * * 1 "); + expect(command).toContain(scriptPath); }); test("crontab entry contains correct format", async () => { @@ -579,11 +614,11 @@ describe.skipIf(!hasCrontab)("cron registration (Linux)", () => { await Bun.cron(`${dir}/job.ts`, "0 * * * *", "test-replace"); await Bun.cron(`${dir}/job.ts`, "30 2 * * 1", "test-replace"); - const crontab = readCrontab(); - const count = (crontab.match(/# bun-cron: test-replace/g) || []).length; - expect(count).toBe(1); - expect(crontab).toContain("30 2 * * 1"); - expect(crontab).not.toContain("0 * * * *"); + // One marker and one command line: the first entry's pair is gone. + expect(entryLines(readCrontab(), "test-replace")).toEqual([ + "# bun-cron: test-replace", + expect.stringMatching(/^30 2 \* \* 1 /), + ]); }); test("registers multiple different cron jobs", async () => { @@ -593,14 +628,18 @@ describe.skipIf(!hasCrontab)("cron registration (Linux)", () => { "b.ts": `export default { scheduled() {} };`, }); - await Bun.cron(`${dir}/a.ts`, "0 * * * *", "multi-a"); - await Bun.cron(`${dir}/b.ts`, "30 12 * * 5", "multi-b"); + await Bun.cron(`${dir}/a.ts`, "0 * * * *", "test-multi-a"); + await Bun.cron(`${dir}/b.ts`, "30 12 * * 5", "test-multi-b"); const crontab = readCrontab(); - expect(crontab).toContain("# bun-cron: multi-a"); - expect(crontab).toContain("# bun-cron: multi-b"); - expect(crontab).toContain("0 * * * *"); - expect(crontab).toContain("30 12 * * 5"); + expect(entryLines(crontab, "test-multi-a")).toEqual([ + "# bun-cron: test-multi-a", + expect.stringMatching(/^0 \* \* \* \* /), + ]); + expect(entryLines(crontab, "test-multi-b")).toEqual([ + "# bun-cron: test-multi-b", + expect.stringMatching(/^30 12 \* \* 5 /), + ]); }); test("preserves existing non-bun crontab entries", async () => { @@ -646,21 +685,22 @@ describe.skipIf(!hasCrontab)("cron removal (Linux)", () => { "job.ts": `export default { scheduled() {} };`, }); - await Bun.cron(`${dir}/job.ts`, "30 2 * * 1", "rm-target"); + await Bun.cron(`${dir}/job.ts`, "30 2 * * 1", "test-rm-target"); - let crontab = readCrontab(); - expect(crontab).toContain("# bun-cron: rm-target"); + expect(entryLines(readCrontab(), "test-rm-target")).toEqual([ + "# bun-cron: test-rm-target", + expect.stringMatching(/^30 2 \* \* 1 /), + ]); - await Bun.cron.remove("rm-target"); + await Bun.cron.remove("test-rm-target"); - crontab = readCrontab(); - expect(crontab).not.toContain("# bun-cron: rm-target"); - expect(crontab).not.toContain("30 2 * * 1"); + // Both the marker and its command line are removed. + expect(entryLines(readCrontab(), "test-rm-target")).toEqual([]); }); test("removing non-existent entry resolves without error", async () => { using _restore = saveCrontabState(); - const result = await Bun.cron.remove("rm-nonexistent"); + const result = await Bun.cron.remove("test-rm-nonexistent"); expect(result).toBeUndefined(); }); @@ -671,14 +711,20 @@ describe.skipIf(!hasCrontab)("cron removal (Linux)", () => { "b.ts": `export default { scheduled() {} };`, }); - await Bun.cron(`${dir}/a.ts`, "0 * * * *", "rm-keep"); - await Bun.cron(`${dir}/b.ts`, "30 2 * * 1", "rm-delete"); + // A foreign line and another entry share the removed entry's schedule. + writeCrontab("30 2 * * 1 /usr/bin/some-other-job\n"); + await Bun.cron(`${dir}/a.ts`, "30 2 * * 1", "test-rm-keep"); + await Bun.cron(`${dir}/b.ts`, "30 2 * * 1", "test-rm-delete"); - await Bun.cron.remove("rm-delete"); + await Bun.cron.remove("test-rm-delete"); const crontab = readCrontab(); - expect(crontab).toContain("# bun-cron: rm-keep"); - expect(crontab).not.toContain("# bun-cron: rm-delete"); + expect(crontab).toContain("30 2 * * 1 /usr/bin/some-other-job\n"); + expect(entryLines(crontab, "test-rm-keep")).toEqual([ + "# bun-cron: test-rm-keep", + expect.stringMatching(/^30 2 \* \* 1 /), + ]); + expect(entryLines(crontab, "test-rm-delete")).toEqual([]); }); test("register after remove works", async () => { @@ -687,17 +733,42 @@ describe.skipIf(!hasCrontab)("cron removal (Linux)", () => { "job.ts": `export default { scheduled() {} };`, }); - await Bun.cron(`${dir}/job.ts`, "0 * * * *", "rm-reregister"); - await Bun.cron.remove("rm-reregister"); + await Bun.cron(`${dir}/job.ts`, "0 * * * *", "test-rm-reregister"); + await Bun.cron.remove("test-rm-reregister"); - let crontab = readCrontab(); - expect(crontab).not.toContain("# bun-cron: rm-reregister"); + expect(entryLines(readCrontab(), "test-rm-reregister")).toEqual([]); - await Bun.cron(`${dir}/job.ts`, "30 6 * * *", "rm-reregister"); + await Bun.cron(`${dir}/job.ts`, "30 6 * * *", "test-rm-reregister"); - crontab = readCrontab(); - expect(crontab).toContain("# bun-cron: rm-reregister"); - expect(crontab).toContain("30 6 * * *"); + expect(entryLines(readCrontab(), "test-rm-reregister")).toEqual([ + "# bun-cron: test-rm-reregister", + expect.stringMatching(/^30 6 \* \* \* /), + ]); + }); + + test("entries left behind by a killed run are removed, everything else is kept", () => { + using _restore = saveCrontabState(); + writeCrontab( + [ + "# bun-cron: test-stale", + "30 2 * * 1 '/old/bun' run --cron-title=test-stale --cron-period='30 2 * * 1' '/gone/job.ts'", + "30 2 * * 1 /usr/bin/some-other-job", + "# bun-cron: not-a-test-job", + "* * * * * /usr/bin/not-a-test-job", + "", + ].join("\n"), + ); + + removeStaleTestEntries(); + + expect(readCrontab()).toBe( + [ + "30 2 * * 1 /usr/bin/some-other-job", + "# bun-cron: not-a-test-job", + "* * * * * /usr/bin/not-a-test-job", + "", + ].join("\n"), + ); }); }); From c843409c5dda91b39d63bac3477e29fd654bdc2a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 16 Aug 2026 09:33:07 +0000 Subject: [PATCH 2/3] test(cron): only sweep a command line together with its own marker; surface unexpected crontab -l failures --- test/js/bun/cron/cron.test.ts | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/test/js/bun/cron/cron.test.ts b/test/js/bun/cron/cron.test.ts index 6b3fda6f0c69..76bbe32f82ba 100644 --- a/test/js/bun/cron/cron.test.ts +++ b/test/js/bun/cron/cron.test.ts @@ -67,7 +67,10 @@ function readCrontab(): string { stdout: "pipe", stderr: "pipe", }); - return result.exitCode === 0 ? result.stdout.toString() : ""; + if (result.exitCode === 0) return result.stdout.toString(); + // Exit code 1 is "no crontab for <user>"; Bun.cron reads it the same way. + if (result.exitCode === 1) return ""; + throw new Error(`crontab -l exited with ${result.exitCode}: ${result.stderr.toString()}`); } function writeCrontab(content: string) { @@ -112,15 +115,15 @@ function entryLines(crontab: string, title: string): string[] { } function removeStaleTestEntries() { + const marker = `# bun-cron: ${TEST_TITLE_PREFIX}`; const lines = readCrontab().split("\n"); - const kept: string[] = []; - for (let i = 0; i < lines.length; i++) { - if (lines[i].startsWith(`# bun-cron: ${TEST_TITLE_PREFIX}`)) { - i++; // skip the command line that belongs to the marker - } else { - kept.push(lines[i]); - } - } + const kept = lines.filter((line, i) => { + if (line.startsWith(marker)) return false; + // A command line goes only together with the test marker right above it. + const previous = i > 0 ? lines[i - 1] : ""; + if (!previous.startsWith(marker)) return true; + return !line.includes(` --cron-title=${previous.slice("# bun-cron: ".length)} `); + }); if (kept.length !== lines.length) writeCrontab(kept.join("\n")); } @@ -582,7 +585,7 @@ describe.skipIf(!hasCrontab)("cron registration (Linux)", () => { const [marker, command] = entryLines(readCrontab(), "test-register"); expect(marker).toBe("# bun-cron: test-register"); expect(command).toStartWith("30 2 * * 1 "); - expect(command).toContain(scriptPath); + expect(command).toEndWith(` '${scriptPath}'`); }); test("crontab entry contains correct format", async () => { @@ -755,6 +758,9 @@ describe.skipIf(!hasCrontab)("cron removal (Linux)", () => { "30 2 * * 1 /usr/bin/some-other-job", "# bun-cron: not-a-test-job", "* * * * * /usr/bin/not-a-test-job", + // A marker whose command line went missing: only the marker goes. + "# bun-cron: test-orphaned-marker", + "0 0 * * * /usr/bin/unrelated-line", "", ].join("\n"), ); @@ -766,6 +772,7 @@ describe.skipIf(!hasCrontab)("cron removal (Linux)", () => { "30 2 * * 1 /usr/bin/some-other-job", "# bun-cron: not-a-test-job", "* * * * * /usr/bin/not-a-test-job", + "0 0 * * * /usr/bin/unrelated-line", "", ].join("\n"), ); From 4352b6a9f0184bcf5d3ce722d94929d7ee34de48 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 16 Aug 2026 21:58:30 +0000 Subject: [PATCH 3/3] test(cron): seed a foreign line with the replaced schedule; sweep fixture with adjacent stale blocks --- test/js/bun/cron/cron.test.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/js/bun/cron/cron.test.ts b/test/js/bun/cron/cron.test.ts index 76bbe32f82ba..716a9aa913cc 100644 --- a/test/js/bun/cron/cron.test.ts +++ b/test/js/bun/cron/cron.test.ts @@ -614,11 +614,15 @@ describe.skipIf(!hasCrontab)("cron registration (Linux)", () => { "job.ts": `export default { scheduled() {} };`, }); + // A foreign line shares the schedule that gets replaced. + writeCrontab("0 * * * * /usr/bin/some-other-job\n"); await Bun.cron(`${dir}/job.ts`, "0 * * * *", "test-replace"); await Bun.cron(`${dir}/job.ts`, "30 2 * * 1", "test-replace"); + const crontab = readCrontab(); + expect(crontab).toContain("0 * * * * /usr/bin/some-other-job\n"); // One marker and one command line: the first entry's pair is gone. - expect(entryLines(readCrontab(), "test-replace")).toEqual([ + expect(entryLines(crontab, "test-replace")).toEqual([ "# bun-cron: test-replace", expect.stringMatching(/^30 2 \* \* 1 /), ]); @@ -753,8 +757,11 @@ describe.skipIf(!hasCrontab)("cron removal (Linux)", () => { using _restore = saveCrontabState(); writeCrontab( [ + // Two runs died in different tests, leaving adjacent blocks. "# bun-cron: test-stale", "30 2 * * 1 '/old/bun' run --cron-title=test-stale --cron-period='30 2 * * 1' '/gone/job.ts'", + "# bun-cron: test-rm-keep", + "0 * * * * '/old/bun' run --cron-title=test-rm-keep --cron-period='0 * * * *' '/gone/a.ts'", "30 2 * * 1 /usr/bin/some-other-job", "# bun-cron: not-a-test-job", "* * * * * /usr/bin/not-a-test-job",