From b572abfcb3fadbfdd3ce671a27463d67bcb77534 Mon Sep 17 00:00:00 2001
From: Igor Zinkovsky <igor@deno.com>
Date: Thu, 16 Nov 2023 14:19:00 -0800
Subject: feat(ext/cron) modify Deno.cron API to make handler arg last (#21225)

This PR changes the `Deno.cron` API:
* Marks the existing function as deprecated
* Introduces 2 new overloads, where the handler arg is always last:
```ts
Deno.cron(
  name: string,
  schedule: string,
  handler: () => Promise<void> | void,
)

Deno.cron(
  name: string,
  schedule: string,
  options?: { backoffSchedule?: number[]; signal?: AbortSignal },
  handler: () => Promise<void> | void,
)
```

This PR also fixes a bug, when other crons continue execution after one
of the crons was closed using `signal`.
---
 cli/tests/unit/cron_test.ts | 88 +++++++++++++++++++++++++++++++--------------
 1 file changed, 62 insertions(+), 26 deletions(-)

(limited to 'cli/tests')

diff --git a/cli/tests/unit/cron_test.ts b/cli/tests/unit/cron_test.ts
index 1d46efdb4..79918ed2f 100644
--- a/cli/tests/unit/cron_test.ts
+++ b/cli/tests/unit/cron_test.ts
@@ -89,17 +89,18 @@ Deno.test(function invalidScheduleTest() {
 Deno.test(function invalidBackoffScheduleTest() {
   assertThrows(
     () =>
-      Deno.cron("abc", "*/1 * * * *", () => {}, {
-        backoffSchedule: [1, 1, 1, 1, 1, 1],
-      }),
+      Deno.cron(
+        "abc",
+        "*/1 * * * *",
+        { backoffSchedule: [1, 1, 1, 1, 1, 1] },
+        () => {},
+      ),
     TypeError,
     "Invalid backoff schedule",
   );
   assertThrows(
     () =>
-      Deno.cron("abc", "*/1 * * * *", () => {}, {
-        backoffSchedule: [3600001],
-      }),
+      Deno.cron("abc", "*/1 * * * *", { backoffSchedule: [3600001] }, () => {}),
     TypeError,
     "Invalid backoff schedule",
   );
@@ -109,16 +110,19 @@ Deno.test(async function tooManyCrons() {
   const crons: Promise<void>[] = [];
   const ac = new AbortController();
   for (let i = 0; i <= 100; i++) {
-    const c = Deno.cron(`abc_${i}`, "*/1 * * * *", () => {}, {
-      signal: ac.signal,
-    });
+    const c = Deno.cron(
+      `abc_${i}`,
+      "*/1 * * * *",
+      { signal: ac.signal },
+      () => {},
+    );
     crons.push(c);
   }
 
   try {
     assertThrows(
       () => {
-        Deno.cron("next-cron", "*/1 * * * *", () => {}, { signal: ac.signal });
+        Deno.cron("next-cron", "*/1 * * * *", { signal: ac.signal }, () => {});
       },
       TypeError,
       "Too many crons",
@@ -133,8 +137,7 @@ Deno.test(async function tooManyCrons() {
 
 Deno.test(async function duplicateCrons() {
   const ac = new AbortController();
-  const c = Deno.cron("abc", "*/20 * * * *", () => {
-  }, { signal: ac.signal });
+  const c = Deno.cron("abc", "*/20 * * * *", { signal: ac.signal }, () => {});
   try {
     assertThrows(
       () => Deno.cron("abc", "*/20 * * * *", () => {}),
@@ -153,12 +156,12 @@ Deno.test(async function basicTest() {
   let count = 0;
   const { promise, resolve } = Promise.withResolvers<void>();
   const ac = new AbortController();
-  const c = Deno.cron("abc", "*/20 * * * *", () => {
+  const c = Deno.cron("abc", "*/20 * * * *", { signal: ac.signal }, () => {
     count++;
     if (count > 5) {
       resolve();
     }
-  }, { signal: ac.signal });
+  });
   try {
     await promise;
   } finally {
@@ -179,18 +182,18 @@ Deno.test(async function multipleCrons() {
     void
   >();
   const ac = new AbortController();
-  const c0 = Deno.cron("abc", "*/20 * * * *", () => {
+  const c0 = Deno.cron("abc", "*/20 * * * *", { signal: ac.signal }, () => {
     count0++;
     if (count0 > 5) {
       resolve0();
     }
-  }, { signal: ac.signal });
-  const c1 = Deno.cron("xyz", "*/20 * * * *", () => {
+  });
+  const c1 = Deno.cron("xyz", "*/20 * * * *", { signal: ac.signal }, () => {
     count1++;
     if (count1 > 5) {
       resolve1();
     }
-  }, { signal: ac.signal });
+  });
   try {
     await promise0;
     await promise1;
@@ -212,11 +215,16 @@ Deno.test(async function overlappingExecutions() {
     void
   >();
   const ac = new AbortController();
-  const c = Deno.cron("abc", "*/20 * * * *", async () => {
-    resolve0();
-    count++;
-    await promise1;
-  }, { signal: ac.signal });
+  const c = Deno.cron(
+    "abc",
+    "*/20 * * * *",
+    { signal: ac.signal },
+    async () => {
+      resolve0();
+      count++;
+      await promise1;
+    },
+  );
   try {
     await promise0;
   } finally {
@@ -228,16 +236,44 @@ Deno.test(async function overlappingExecutions() {
   assertEquals(count, 1);
 });
 
-Deno.test(async function retriesWithBackkoffSchedule() {
+Deno.test(async function retriesWithBackoffSchedule() {
+  Deno.env.set("DENO_CRON_TEST_SCHEDULE_OFFSET", "5000");
+
+  let count = 0;
+  const ac = new AbortController();
+  const c = Deno.cron("abc", "*/20 * * * *", {
+    signal: ac.signal,
+    backoffSchedule: [10, 20],
+  }, async () => {
+    count += 1;
+    await sleep(10);
+    throw new TypeError("cron error");
+  });
+  try {
+    await sleep(6000);
+  } finally {
+    ac.abort();
+    await c;
+  }
+
+  // The cron should have executed 3 times (1st attempt and 2 retries).
+  assertEquals(count, 3);
+});
+
+Deno.test(async function retriesWithBackoffScheduleOldApi() {
   Deno.env.set("DENO_CRON_TEST_SCHEDULE_OFFSET", "5000");
 
   let count = 0;
   const ac = new AbortController();
-  const c = Deno.cron("abc", "*/20 * * * *", async () => {
+  const c = Deno.cron("abc2", "*/20 * * * *", async () => {
     count += 1;
     await sleep(10);
     throw new TypeError("cron error");
-  }, { signal: ac.signal, backoffSchedule: [10, 20] });
+  }, {
+    signal: ac.signal,
+    backoffSchedule: [10, 20],
+  });
+
   try {
     await sleep(6000);
   } finally {
-- 
cgit v1.2.3