From 4cd9500523bf9c5c09efdcb4c49b6e31acfb9e88 Mon Sep 17 00:00:00 2001
From: Jacob Wallraff <thyeggman@github.com>
Date: Mon, 9 Jan 2023 16:52:53 -0800
Subject: [PATCH 01/11] Port over josh's changes

---
 .../src/model/converter/cron.ts               | 95 +++++++++++++++++++
 .../src/model/converter/events.ts             |  5 +-
 2 files changed, 97 insertions(+), 3 deletions(-)
 create mode 100644 actions-workflow-parser/src/model/converter/cron.ts

diff --git a/actions-workflow-parser/src/model/converter/cron.ts b/actions-workflow-parser/src/model/converter/cron.ts
new file mode 100644
index 0000000..5af4803
--- /dev/null
+++ b/actions-workflow-parser/src/model/converter/cron.ts
@@ -0,0 +1,95 @@
+const MONTHS = {
+    jan: 1,
+    feb: 2,
+    mar: 3,
+    apr: 4,
+    may: 5,
+    jun: 6,
+    jul: 7,
+    aug: 8,
+    sep: 9,
+    oct: 10,
+    nov: 11,
+    dec: 12,
+  }
+  
+  const DAYS = {
+    sun: 0,
+    mon: 1,
+    tue: 2,
+    wed: 3,
+    thu: 4,
+    fri: 5,
+    sat: 6,
+  }
+  
+  // TODO: make this parseCron
+  export function isValidCron(cron: string): boolean {
+    // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule
+  
+    const parts = cron.split(" ")
+    if (parts.length != 5) {
+      return false
+    }
+  
+    const [minutes, hours, dom, months, dow] = parts
+  
+    return (
+      validateRange(minutes, { min: 0, max: 59 }) &&
+      validateRange(hours, { min: 0, max: 23 }) &&
+      validateRange(dom, { min: 1, max: 31 }) &&
+      validateRange(months, { min: 1, max: 12, names: MONTHS }) &&
+      validateRange(dow, { min: 0, max: 6, names: DAYS })
+    )
+  }
+  
+  type Range = {
+    min: number
+    max: number
+    names?: Record<string, number>
+  }
+  
+  function validateRange(
+    value: string,
+    range: Range,
+    allowSeparators = true
+  ): boolean {
+    if (range.names && range.names[value.toLowerCase()] !== undefined) {
+      return true
+    }
+  
+    if (value === "*") {
+      return true
+    }
+  
+    if (value.includes(",")) {
+      if (!allowSeparators) {
+        return false
+      }
+      return value.split(",").every((v) => validateRange(v, range, false))
+    }
+  
+    if (value.includes("/")) {
+      if (!allowSeparators) {
+        return false
+      }
+  
+      const [start, step, ...rest] = value.split("/")
+      if (rest.length > 0) {
+        return false
+      }
+      return (
+        validateRange(start, range, false) && validateRange(step, range, false)
+      )
+    }
+  
+    if (value.includes("-")) {
+      const [start, end] = value.split("-")
+      return (
+        validateRange(start, range, false) && validateRange(end, range, false)
+      )
+    }
+  
+    const number = parseInt(value, 10)
+    return !isNaN(number) && number >= range.min && number <= range.max
+  }
\ No newline at end of file
diff --git a/actions-workflow-parser/src/model/converter/events.ts b/actions-workflow-parser/src/model/converter/events.ts
index 4504e2d..719c31c 100644
--- a/actions-workflow-parser/src/model/converter/events.ts
+++ b/actions-workflow-parser/src/model/converter/events.ts
@@ -18,6 +18,7 @@ import {
   TypesFilterConfig,
   WorkflowFilterConfig,
 } from "../workflow-template"
+import { isValidCron } from "./cron"
 import { convertStringList } from "./string-list"
 import { convertEventWorkflowDispatchInputs } from "./workflow-dispatch"
 
@@ -160,9 +161,7 @@ function convertSchedule(
       if (scheduleKey.value == "cron") {
         const cron = schedule.value.assertString(`schedule cron`)
         // Validate the cron string
-        if (
-          !cron.value.match(/((((\d+,)+\d+|(\d+(\/|-)\d+)|\d+|\*) ?){5,7})/)
-        ) {
+        if (!isValidCron(cron.value)) {
           context.error(cron, "Invalid cron string")
         }
         result.push({ cron: cron.value })

From d02fa0bb678d0af9769ce21220f0ddd58dff95e3 Mon Sep 17 00:00:00 2001
From: Jacob Wallraff <thyeggman@github.com>
Date: Mon, 9 Jan 2023 17:51:44 -0800
Subject: [PATCH 02/11] Add tests and correct a lot of validation

---
 .../src/model/converter/cron.test.ts          |  43 ++++
 .../src/model/converter/cron.ts               | 188 ++++++++++--------
 2 files changed, 145 insertions(+), 86 deletions(-)
 create mode 100644 actions-workflow-parser/src/model/converter/cron.test.ts

diff --git a/actions-workflow-parser/src/model/converter/cron.test.ts b/actions-workflow-parser/src/model/converter/cron.test.ts
new file mode 100644
index 0000000..6f5839f
--- /dev/null
+++ b/actions-workflow-parser/src/model/converter/cron.test.ts
@@ -0,0 +1,43 @@
+import { isValidCron } from "./cron"
+
+describe("isValidCron", () => {
+  const valid = [
+    "0 0 * * *",
+    "15 * * * *",
+    "2,10 4,5 * * *",
+    "30 4-6 * * *",
+    "20/15 * * * *",
+    "30 5,17 * * *",
+    "30 5 * * 1,3",
+    "30 5 * * 2,4",
+    "28 */4 * * *",
+    "28 5,*/4 * * *",
+    "28 5,*/4,6 * * *",
+    "28 5,*/4,6-8 * * *",
+    "0 0 * * SUN",
+    "0 0 * * SUN-TUE",
+    "0 0 * * SUN-2",
+    "0 * * */FEB */TUE",
+  ]
+
+  for (const cron of valid) {
+    it(`${cron} should be valid`, () => {
+      expect(isValidCron(cron)).toBe(true)
+    })
+  }
+
+  const invalid = [
+    "0 0 * *",
+    "0 0 * * BUN",
+    "0 0 * SUN JAN",
+    "0 0 * * FRI-TUE",
+    "0 12-4 * * *",
+    "0 */0 * * *"
+  ]
+
+  for (const cron of invalid) {
+    it(`${cron} should be invalid`, () => {
+      expect(isValidCron(cron)).toBe(false)
+    })
+  }
+})
\ No newline at end of file
diff --git a/actions-workflow-parser/src/model/converter/cron.ts b/actions-workflow-parser/src/model/converter/cron.ts
index 5af4803..52b1cab 100644
--- a/actions-workflow-parser/src/model/converter/cron.ts
+++ b/actions-workflow-parser/src/model/converter/cron.ts
@@ -1,95 +1,111 @@
 const MONTHS = {
-    jan: 1,
-    feb: 2,
-    mar: 3,
-    apr: 4,
-    may: 5,
-    jun: 6,
-    jul: 7,
-    aug: 8,
-    sep: 9,
-    oct: 10,
-    nov: 11,
-    dec: 12,
+  jan: 1,
+  feb: 2,
+  mar: 3,
+  apr: 4,
+  may: 5,
+  jun: 6,
+  jul: 7,
+  aug: 8,
+  sep: 9,
+  oct: 10,
+  nov: 11,
+  dec: 12,
+}
+
+const DAYS = {
+  sun: 0,
+  mon: 1,
+  tue: 2,
+  wed: 3,
+  thu: 4,
+  fri: 5,
+  sat: 6,
+}
+
+// TODO: make this parseCron
+export function isValidCron(cron: string): boolean {
+  // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule
+
+  const parts = cron.split(" ")
+  if (parts.length != 5) {
+    return false
   }
-  
-  const DAYS = {
-    sun: 0,
-    mon: 1,
-    tue: 2,
-    wed: 3,
-    thu: 4,
-    fri: 5,
-    sat: 6,
+
+  const [minutes, hours, dom, months, dow] = parts
+
+  return (
+    validateRange(minutes, { min: 0, max: 59 }) &&
+    validateRange(hours, { min: 0, max: 23 }) &&
+    validateRange(dom, { min: 1, max: 31 }) &&
+    validateRange(months, { min: 1, max: 12, names: MONTHS }) &&
+    validateRange(dow, { min: 0, max: 6, names: DAYS })
+  )
+}
+
+type Range = {
+  min: number
+  max: number
+  names?: Record<string, number>
+}
+
+function validateRange(
+  value: string,
+  range: Range,
+  allowSeparators = true
+): boolean {
+  if (range.names && range.names[value.toLowerCase()] !== undefined) {
+    return true
   }
-  
-  // TODO: make this parseCron
-  export function isValidCron(cron: string): boolean {
-    // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule
-  
-    const parts = cron.split(" ")
-    if (parts.length != 5) {
+
+  if (value === "*") {
+    return true
+  }
+
+  if (value.includes(",")) {
+    if (!allowSeparators) {
+      return false
+    }
+    // Allow separators
+    return value.split(",").every((v) => validateRange(v, range))
+  }
+
+  if (value.includes("/")) {
+    if (!allowSeparators) {
+      return false
+    }
+
+    const [start, step, ...rest] = value.split("/")
+    // Supports */TUE and similar, which nees to be verified with the go cron library
+    const stepNumber = convertToNumber(range, step)
+    if (rest.length > 0 || stepNumber <= 0) {
       return false
     }
-  
-    const [minutes, hours, dom, months, dow] = parts
-  
     return (
-      validateRange(minutes, { min: 0, max: 59 }) &&
-      validateRange(hours, { min: 0, max: 23 }) &&
-      validateRange(dom, { min: 1, max: 31 }) &&
-      validateRange(months, { min: 1, max: 12, names: MONTHS }) &&
-      validateRange(dow, { min: 0, max: 6, names: DAYS })
+      validateRange(start, range, false) && validateRange(step, range, false)
     )
   }
-  
-  type Range = {
-    min: number
-    max: number
-    names?: Record<string, number>
+
+  if (value.includes("-")) {
+    const [start, end] = value.split("-")
+
+    // Convert name to integers so we can make sure end >= start
+    const startNumber = convertToNumber(range, start)
+    const endNumber = convertToNumber(range, end)
+    return (
+      validateRange(start, range, false) && validateRange(end, range, false) && endNumber >= startNumber
+    )
   }
-  
-  function validateRange(
-    value: string,
-    range: Range,
-    allowSeparators = true
-  ): boolean {
-    if (range.names && range.names[value.toLowerCase()] !== undefined) {
-      return true
-    }
-  
-    if (value === "*") {
-      return true
-    }
-  
-    if (value.includes(",")) {
-      if (!allowSeparators) {
-        return false
-      }
-      return value.split(",").every((v) => validateRange(v, range, false))
-    }
-  
-    if (value.includes("/")) {
-      if (!allowSeparators) {
-        return false
-      }
-  
-      const [start, step, ...rest] = value.split("/")
-      if (rest.length > 0) {
-        return false
-      }
-      return (
-        validateRange(start, range, false) && validateRange(step, range, false)
-      )
-    }
-  
-    if (value.includes("-")) {
-      const [start, end] = value.split("-")
-      return (
-        validateRange(start, range, false) && validateRange(end, range, false)
-      )
-    }
-  
-    const number = parseInt(value, 10)
-    return !isNaN(number) && number >= range.min && number <= range.max
-  }
\ No newline at end of file
+
+  const number = +value
+  return !isNaN(number) && number >= range.min && number <= range.max
+}
+
+function convertToNumber(range: Range, value: string): number {
+  if (range.names && range.names[value.toLowerCase()] !== undefined) {
+    return +range.names[value.toLowerCase()]
+  }
+  else {
+    return +value
+  }
+}
\ No newline at end of file

From 7be6d9619c2b40b71d339ff881e8cff0cc1520ce Mon Sep 17 00:00:00 2001
From: Jacob Wallraff <thyeggman@github.com>
Date: Mon, 9 Jan 2023 18:02:30 -0800
Subject: [PATCH 03/11] More test cases

---
 actions-workflow-parser/src/model/converter/cron.test.ts | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/actions-workflow-parser/src/model/converter/cron.test.ts b/actions-workflow-parser/src/model/converter/cron.test.ts
index 6f5839f..56edded 100644
--- a/actions-workflow-parser/src/model/converter/cron.test.ts
+++ b/actions-workflow-parser/src/model/converter/cron.test.ts
@@ -18,6 +18,8 @@ describe("isValidCron", () => {
     "0 0 * * SUN-TUE",
     "0 0 * * SUN-2",
     "0 * * */FEB */TUE",
+    "0 2-4/5 * * *",
+    // "0 29/5 * * *" <- crontab guru says this is valid, need to test with go library
   ]
 
   for (const cron of valid) {
@@ -32,7 +34,9 @@ describe("isValidCron", () => {
     "0 0 * SUN JAN",
     "0 0 * * FRI-TUE",
     "0 12-4 * * *",
-    "0 */0 * * *"
+    "0 */0 * * *",
+    "0 2/4-5 * * *", // TODO
+    "0 2-4-6/5 * * *", // TODO
   ]
 
   for (const cron of invalid) {

From 5626b7fb85c06509842b51887405b9b22032ceba Mon Sep 17 00:00:00 2001
From: Jacob Wallraff <thyeggman@github.com>
Date: Tue, 10 Jan 2023 09:57:07 -0800
Subject: [PATCH 04/11] Add more test cases and fix more validation

---
 .../src/model/converter/cron.test.ts          | 18 +++++++++++++++---
 .../src/model/converter/cron.ts               | 19 ++++++++++++++-----
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/actions-workflow-parser/src/model/converter/cron.test.ts b/actions-workflow-parser/src/model/converter/cron.test.ts
index 56edded..533350e 100644
--- a/actions-workflow-parser/src/model/converter/cron.test.ts
+++ b/actions-workflow-parser/src/model/converter/cron.test.ts
@@ -3,6 +3,7 @@ import { isValidCron } from "./cron"
 describe("isValidCron", () => {
   const valid = [
     "0 0 * * *",
+    "0 000 001 * *",
     "15 * * * *",
     "2,10 4,5 * * *",
     "30 4-6 * * *",
@@ -19,7 +20,6 @@ describe("isValidCron", () => {
     "0 0 * * SUN-2",
     "0 * * */FEB */TUE",
     "0 2-4/5 * * *",
-    // "0 29/5 * * *" <- crontab guru says this is valid, need to test with go library
   ]
 
   for (const cron of valid) {
@@ -30,13 +30,25 @@ describe("isValidCron", () => {
 
   const invalid = [
     "0 0 * *",
+    "0 0 * * * * *",
+    "0 -1 * * *",
+    "0 1- * * *",
+    "0 /1 * * *",
+    "0 1/ * * *",
+    "0 ,1 * * *",
+    "0 1, * * *",
+    "0 -- * * *",
+    "0 // * * *",
+    "0 ,, * * *",
+    "0 ** * * *",
     "0 0 * * BUN",
     "0 0 * SUN JAN",
     "0 0 * * FRI-TUE",
     "0 12-4 * * *",
     "0 */0 * * *",
-    "0 2/4-5 * * *", // TODO
-    "0 2-4-6/5 * * *", // TODO
+    "0 2/4-5 * * *",
+    "0 2-4-6 * * *",
+    "0 2/4/6 * * *",
   ]
 
   for (const cron of invalid) {
diff --git a/actions-workflow-parser/src/model/converter/cron.ts b/actions-workflow-parser/src/model/converter/cron.ts
index 52b1cab..6875928 100644
--- a/actions-workflow-parser/src/model/converter/cron.ts
+++ b/actions-workflow-parser/src/model/converter/cron.ts
@@ -62,12 +62,15 @@ function validateRange(
     return true
   }
 
+  // Operator precedence: , > / > -
   if (value.includes(",")) {
     if (!allowSeparators) {
       return false
     }
     // Allow separators
-    return value.split(",").every((v) => validateRange(v, range))
+    return value.split(",").every((v) => {
+      v && validateRange(v, range)
+    })
   }
 
   if (value.includes("/")) {
@@ -76,18 +79,24 @@ function validateRange(
     }
 
     const [start, step, ...rest] = value.split("/")
-    // Supports */TUE and similar, which nees to be verified with the go cron library
+    // Supports */TUE and similar, which needs to be verified with the go cron library
     const stepNumber = convertToNumber(range, step)
-    if (rest.length > 0 || stepNumber <= 0) {
+    if (rest.length > 0 || stepNumber <= 0 || !start || !step) {
       return false
     }
     return (
-      validateRange(start, range, false) && validateRange(step, range, false)
+      validateRange(start, range) && validateRange(step, range, false)
     )
   }
 
   if (value.includes("-")) {
-    const [start, end] = value.split("-")
+    if (!allowSeparators) {
+      return false
+    }
+    const [start, end, ...rest] = value.split("-")
+    if (rest.length > 0 || !start || !end) {
+      return false
+    }
 
     // Convert name to integers so we can make sure end >= start
     const startNumber = convertToNumber(range, start)

From a813ec880e17964df8b5ebdeabcff6ec0d2bf38c Mon Sep 17 00:00:00 2001
From: Jacob Wallraff <thyeggman@github.com>
Date: Tue, 10 Jan 2023 09:57:51 -0800
Subject: [PATCH 05/11] Add validation test for invalid cron string

---
 actions-languageservice/src/validate.test.ts | 30 ++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/actions-languageservice/src/validate.test.ts b/actions-languageservice/src/validate.test.ts
index eaaa38e..ea0e993 100644
--- a/actions-languageservice/src/validate.test.ts
+++ b/actions-languageservice/src/validate.test.ts
@@ -186,4 +186,34 @@ jobs:
       }
     } as Diagnostic);
   });
+
+  it("invalid cron string", async () => {
+    const result = await validate(
+      createDocument(
+        "wf.yaml",
+        `on:
+  schedule:
+    - cron: '0 0 * *'
+jobs:
+  build:
+    runs-on: ubuntu-latest`
+      ),
+      {valueProviderConfig: defaultValueProviders}
+    );
+
+    expect(result.length).toBe(1);
+    expect(result[0]).toEqual({
+      message: "Invalid cron string",
+      range: {
+        end: {
+          character: 21,
+          line: 2
+        },
+        start: {
+          character: 12,
+          line: 2
+        }
+      }
+    } as Diagnostic);
+  });
 });

From b0da5195d15ffe6c9abdfa3be888e192c098aabf Mon Sep 17 00:00:00 2001
From: Jacob Wallraff <thyeggman@github.com>
Date: Tue, 10 Jan 2023 10:22:25 -0800
Subject: [PATCH 06/11] More tests and a fix

---
 actions-workflow-parser/src/model/converter/cron.test.ts | 5 +++--
 actions-workflow-parser/src/model/converter/cron.ts      | 8 +++-----
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/actions-workflow-parser/src/model/converter/cron.test.ts b/actions-workflow-parser/src/model/converter/cron.test.ts
index 533350e..99fa09a 100644
--- a/actions-workflow-parser/src/model/converter/cron.test.ts
+++ b/actions-workflow-parser/src/model/converter/cron.test.ts
@@ -37,8 +37,8 @@ describe("isValidCron", () => {
     "0 1/ * * *",
     "0 ,1 * * *",
     "0 1, * * *",
-    "0 -- * * *",
-    "0 // * * *",
+    "0 5--5 * * *",
+    "0 *//5 * * *",
     "0 ,, * * *",
     "0 ** * * *",
     "0 0 * * BUN",
@@ -49,6 +49,7 @@ describe("isValidCron", () => {
     "0 2/4-5 * * *",
     "0 2-4-6 * * *",
     "0 2/4/6 * * *",
+    "0 2/4/6 * * *",
   ]
 
   for (const cron of invalid) {
diff --git a/actions-workflow-parser/src/model/converter/cron.ts b/actions-workflow-parser/src/model/converter/cron.ts
index 6875928..911494d 100644
--- a/actions-workflow-parser/src/model/converter/cron.ts
+++ b/actions-workflow-parser/src/model/converter/cron.ts
@@ -67,10 +67,7 @@ function validateRange(
     if (!allowSeparators) {
       return false
     }
-    // Allow separators
-    return value.split(",").every((v) => {
-      v && validateRange(v, range)
-    })
+    return value.split(",").every((v) => v && validateRange(v, range))
   }
 
   if (value.includes("/")) {
@@ -79,11 +76,12 @@ function validateRange(
     }
 
     const [start, step, ...rest] = value.split("/")
-    // Supports */TUE and similar, which needs to be verified with the go cron library
     const stepNumber = convertToNumber(range, step)
     if (rest.length > 0 || stepNumber <= 0 || !start || !step) {
       return false
     }
+
+    // Separators are only allowed in the part before the `/`, e.g. `1-5/2`
     return (
       validateRange(start, range) && validateRange(step, range, false)
     )

From 92db0c1af79771799ee2d085bfd093ec8664f14e Mon Sep 17 00:00:00 2001
From: Jacob Wallraff <thyeggman@github.com>
Date: Tue, 10 Jan 2023 10:32:35 -0800
Subject: [PATCH 07/11] Remove duplicate test

---
 actions-workflow-parser/src/model/converter/cron.test.ts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/actions-workflow-parser/src/model/converter/cron.test.ts b/actions-workflow-parser/src/model/converter/cron.test.ts
index 99fa09a..9454c83 100644
--- a/actions-workflow-parser/src/model/converter/cron.test.ts
+++ b/actions-workflow-parser/src/model/converter/cron.test.ts
@@ -49,7 +49,6 @@ describe("isValidCron", () => {
     "0 2/4-5 * * *",
     "0 2-4-6 * * *",
     "0 2/4/6 * * *",
-    "0 2/4/6 * * *",
   ]
 
   for (const cron of invalid) {

From f877c22e9cec77a87dc6548364c30cca5da40812 Mon Sep 17 00:00:00 2001
From: Jacob Wallraff <thyeggman@github.com>
Date: Tue, 10 Jan 2023 11:54:41 -0800
Subject: [PATCH 08/11] Remove support for */TUE

---
 actions-workflow-parser/src/model/converter/cron.test.ts | 3 ++-
 actions-workflow-parser/src/model/converter/cron.ts      | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/actions-workflow-parser/src/model/converter/cron.test.ts b/actions-workflow-parser/src/model/converter/cron.test.ts
index 9454c83..6f525ec 100644
--- a/actions-workflow-parser/src/model/converter/cron.test.ts
+++ b/actions-workflow-parser/src/model/converter/cron.test.ts
@@ -18,7 +18,6 @@ describe("isValidCron", () => {
     "0 0 * * SUN",
     "0 0 * * SUN-TUE",
     "0 0 * * SUN-2",
-    "0 * * */FEB */TUE",
     "0 2-4/5 * * *",
   ]
 
@@ -40,6 +39,7 @@ describe("isValidCron", () => {
     "0 5--5 * * *",
     "0 *//5 * * *",
     "0 ,, * * *",
+    "0 , , , ,",
     "0 ** * * *",
     "0 0 * * BUN",
     "0 0 * SUN JAN",
@@ -49,6 +49,7 @@ describe("isValidCron", () => {
     "0 2/4-5 * * *",
     "0 2-4-6 * * *",
     "0 2/4/6 * * *",
+    "0 * * */FEB */TUE",
   ]
 
   for (const cron of invalid) {
diff --git a/actions-workflow-parser/src/model/converter/cron.ts b/actions-workflow-parser/src/model/converter/cron.ts
index 911494d..1e65280 100644
--- a/actions-workflow-parser/src/model/converter/cron.ts
+++ b/actions-workflow-parser/src/model/converter/cron.ts
@@ -76,8 +76,8 @@ function validateRange(
     }
 
     const [start, step, ...rest] = value.split("/")
-    const stepNumber = convertToNumber(range, step)
-    if (rest.length > 0 || stepNumber <= 0 || !start || !step) {
+    const stepNumber = +step
+    if (rest.length > 0 || isNaN(stepNumber) || stepNumber <= 0 || !start || !step) {
       return false
     }
 

From 78e2e325187e02f70176e9d655ca47c789c76b2a Mon Sep 17 00:00:00 2001
From: Jacob Wallraff <thyeggman@github.com>
Date: Tue, 10 Jan 2023 12:03:56 -0800
Subject: [PATCH 09/11] Match one or more whitespace characters

---
 actions-workflow-parser/src/model/converter/cron.test.ts | 1 +
 actions-workflow-parser/src/model/converter/cron.ts      | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/actions-workflow-parser/src/model/converter/cron.test.ts b/actions-workflow-parser/src/model/converter/cron.test.ts
index 6f525ec..cc5426c 100644
--- a/actions-workflow-parser/src/model/converter/cron.test.ts
+++ b/actions-workflow-parser/src/model/converter/cron.test.ts
@@ -19,6 +19,7 @@ describe("isValidCron", () => {
     "0 0 * * SUN-TUE",
     "0 0 * * SUN-2",
     "0 2-4/5 * * *",
+    "0   0  *  *               *",
   ]
 
   for (const cron of valid) {
diff --git a/actions-workflow-parser/src/model/converter/cron.ts b/actions-workflow-parser/src/model/converter/cron.ts
index 1e65280..aa46ca5 100644
--- a/actions-workflow-parser/src/model/converter/cron.ts
+++ b/actions-workflow-parser/src/model/converter/cron.ts
@@ -27,7 +27,7 @@ const DAYS = {
 export function isValidCron(cron: string): boolean {
   // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule
 
-  const parts = cron.split(" ")
+  const parts = cron.split(/ +/)
   if (parts.length != 5) {
     return false
   }

From e6a7271ba5dcedd549f1176ce41846d54dcf92f4 Mon Sep 17 00:00:00 2001
From: Jacob Wallraff <thyeggman@github.com>
Date: Tue, 10 Jan 2023 13:08:29 -0800
Subject: [PATCH 10/11] Add test reasons

---
 .../src/model/converter/cron.test.ts          | 86 +++++++++----------
 1 file changed, 42 insertions(+), 44 deletions(-)

diff --git a/actions-workflow-parser/src/model/converter/cron.test.ts b/actions-workflow-parser/src/model/converter/cron.test.ts
index cc5426c..8707515 100644
--- a/actions-workflow-parser/src/model/converter/cron.test.ts
+++ b/actions-workflow-parser/src/model/converter/cron.test.ts
@@ -2,59 +2,57 @@ import { isValidCron } from "./cron"
 
 describe("isValidCron", () => {
   const valid = [
-    "0 0 * * *",
-    "0 000 001 * *",
-    "15 * * * *",
-    "2,10 4,5 * * *",
-    "30 4-6 * * *",
-    "20/15 * * * *",
-    "30 5,17 * * *",
-    "30 5 * * 1,3",
-    "30 5 * * 2,4",
-    "28 */4 * * *",
-    "28 5,*/4 * * *",
-    "28 5,*/4,6 * * *",
-    "28 5,*/4,6-8 * * *",
-    "0 0 * * SUN",
-    "0 0 * * SUN-TUE",
-    "0 0 * * SUN-2",
-    "0 2-4/5 * * *",
-    "0   0  *  *               *",
+    ["0 0 * * *", "every day at midnight"],
+    ["0 000 001 * *", "accepts leading zeros"],
+    ["15 * * * *", "accepts numbers in range"],
+    ["2,10 4,5 * * *", "accepts comma separated values"],
+    ["30 4-6 * * *", "accepts range"],
+    ["0 4-4 * * *", "accepts range with two equal values"],
+    ["20/15 * * * *", "accepts step with numerical values"],
+    ["30 5,17 * * *", "accepts numbers and ranges"],
+    ["28 */4 * * *", "accepts step with * and numerical value"],
+    ["28 5,*/4 * * *", "accepts comma separated value with step"],
+    ["28 5,*/4,6-8 * * *", "accepts comma separated value with step and range"],
+    ["0 0 * * SUN", "accepts day of week short name"],
+    ["0 0 * * SUN-TUE", "accepts day of week short name range"],
+    ["0 0 * * SUN-2", "accepts day of week range combined with number"],
+    ["0 2-4/5 * * *", "accepts range with step"],
+    ["0   0  *  *               *", "accepts multiple spaces"],
   ]
 
-  for (const cron of valid) {
-    it(`${cron} should be valid`, () => {
+  for (const [cron, reason] of valid) {
+    it(`${cron} should be valid: ${reason}`, () => {
       expect(isValidCron(cron)).toBe(true)
     })
   }
 
   const invalid = [
-    "0 0 * *",
-    "0 0 * * * * *",
-    "0 -1 * * *",
-    "0 1- * * *",
-    "0 /1 * * *",
-    "0 1/ * * *",
-    "0 ,1 * * *",
-    "0 1, * * *",
-    "0 5--5 * * *",
-    "0 *//5 * * *",
-    "0 ,, * * *",
-    "0 , , , ,",
-    "0 ** * * *",
-    "0 0 * * BUN",
-    "0 0 * SUN JAN",
-    "0 0 * * FRI-TUE",
-    "0 12-4 * * *",
-    "0 */0 * * *",
-    "0 2/4-5 * * *",
-    "0 2-4-6 * * *",
-    "0 2/4/6 * * *",
-    "0 * * */FEB */TUE",
+    ["0 0 * *", "too few parts"],
+    ["0 0 * * * * *", "too many parts"],
+    ["0 -1 * * *", "should not accept negative numbers"],
+    ["0 1- * * *", "should not accept trailing -"],
+    ["0 /1 * * *", "should not accept leading / (empty value)"],
+    ["0 1/ * * *", "should not accept trailing / (empty value)"],
+    ["0 ,1 * * *", "should not accept leading , (empty value)"],
+    ["0 1, * * *", "should not accept trailing , (empty value)"],
+    ["0 5--5 * * *", "should not accept multiple -"],
+    ["0 *//5 * * *", "should not accept multiple /"],
+    ["0 ,, * * *", "should not accept multiple ,"],
+    [", , , , ,", "comma is not a valid part"],
+    ["0 ** * * *", "should not accept multiple *"],
+    ["0 0 * * BUN", "invalid short name"],
+    ["0 0 * SUN JAN", "short name in incorrect position"],
+    ["0 0 * * FRI-TUE", "should not accept short name range with start > end"],
+    ["0 12-4 * * *", "should not accept nuerical range with start > end"],
+    ["0 */0 * * *", "step size may not be 0"],
+    ["0 2/4-5 * * *", "step size may not be a range"],
+    ["0 2-4-6 * * *", "range may not contain multiple -"],
+    ["0 2/4/6 * * *", "step size may not contain multiple /"],
+    ["0 * * */FEB */TUE", "step size may not be a short name"],
   ]
 
-  for (const cron of invalid) {
-    it(`${cron} should be invalid`, () => {
+  for (const [cron, reason] of invalid) {
+    it(`${cron} should be invalid: ${reason}`, () => {
       expect(isValidCron(cron)).toBe(false)
     })
   }

From 404b4294881ee9eb3ce521515b9f4f182059193a Mon Sep 17 00:00:00 2001
From: Jacob Wallraff <thyeggman@github.com>
Date: Tue, 10 Jan 2023 13:16:44 -0800
Subject: [PATCH 11/11] Add more tests

---
 actions-workflow-parser/src/model/converter/cron.test.ts | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/actions-workflow-parser/src/model/converter/cron.test.ts b/actions-workflow-parser/src/model/converter/cron.test.ts
index 8707515..d407558 100644
--- a/actions-workflow-parser/src/model/converter/cron.test.ts
+++ b/actions-workflow-parser/src/model/converter/cron.test.ts
@@ -37,6 +37,10 @@ describe("isValidCron", () => {
     ["0 1, * * *", "should not accept trailing , (empty value)"],
     ["0 5--5 * * *", "should not accept multiple -"],
     ["0 *//5 * * *", "should not accept multiple /"],
+    ["0 */* * * *", "step start and size may not both be *"],
+    ["0 5/* * * *", "steps size may not be *"],
+    ["0 *-4 5-* *-* *", "range may not contain *"],
+    ["0 *//5 * * *", "should not accept multiple /"],
     ["0 ,, * * *", "should not accept multiple ,"],
     [", , , , ,", "comma is not a valid part"],
     ["0 ** * * *", "should not accept multiple *"],
@@ -47,7 +51,7 @@ describe("isValidCron", () => {
     ["0 */0 * * *", "step size may not be 0"],
     ["0 2/4-5 * * *", "step size may not be a range"],
     ["0 2-4-6 * * *", "range may not contain multiple -"],
-    ["0 2/4/6 * * *", "step size may not contain multiple /"],
+    ["0 2/4/6 * * *", "step may not contain multiple /"],
     ["0 * * */FEB */TUE", "step size may not be a short name"],
   ]