From f4bb03f488f489ba01d6efb4e8eae18f1b6da422 Mon Sep 17 00:00:00 2001
From: Jef Roosens <roosensjef@gmail.com>
Date: Tue, 12 Apr 2022 09:48:25 +0200
Subject: [PATCH] Tests n bug fixes

---
 src/cron/expression.v            | 50 +++++++++++++++++++++++---------
 src/cron/expression_parse_test.v | 47 +++++++++++++++++++++---------
 2 files changed, 70 insertions(+), 27 deletions(-)

diff --git a/src/cron/expression.v b/src/cron/expression.v
index 60e1b74..46f92f9 100644
--- a/src/cron/expression.v
+++ b/src/cron/expression.v
@@ -1,6 +1,5 @@
 module cron
 
-import math
 import time
 
 const days_in_month = [31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31]
@@ -121,28 +120,50 @@ fn parse_range(s string, min int, max int, mut bitv []bool) ? {
 	mut start := min
 	mut interval := 1
 
-	if s != '*' {
-		exps := s.split('/')
+	exps := s.split('/')
 
-		start = math.min(max, math.max(exps[0].int(), min))
+	if exps[0] != '*' {
+		start = exps[0].int()
 
-		if exps.len > 1 {
-			interval = exps[1].int()
+		// The builtin parsing functions return zero if the string can't be
+		// parsed into a number, so we have to explicitely check whether they
+		// actually entered zero or if it's an invalid number.
+		if start == 0 && exps[0] != '0' {
+			return error('Invalid number.')
 		}
-		// Here, s solely consists of a number, so that's the only value we
-		// should return.
-		else {
-			bitv[start - min - 1] = true
-			return
+
+		// Check whether the start value is out of range
+		if start < min || start > max {
+			return error('Out of range.')
 		}
 	}
 
-	if interval == 0 {
+	if exps.len > 1 {
+		interval = exps[1].int()
+
+		// interval being zero is always invalid, but we want to check why
+		// it's invalid for better error messages.
+		if interval == 0 {
+			if exps[1] != '0' {
+				return error('Invalid number.')
+			}else{
+				return error('Step size zero not allowed.')
+			}
+		}
+
+		if interval > max - min {
+			return error('Step size too large.')
+		}
+	}
+	// Here, s solely consists of a number, so that's the only value we
+	// should return.
+	else if exps[0] != '*' {
+		bitv[start - min] = true
 		return
 	}
 
 	for start <= max {
-		bitv[start - min - 1] = true
+		bitv[start - min] = true
 		start += interval
 	}
 }
@@ -171,7 +192,8 @@ fn parse_part(s string, min int, max int) ?[]int {
 
 // min hour day month day-of-week
 fn parse_expression(exp string) ?CronExpression {
-	mut parts := exp.split(' ')
+	// The filter allows for multiple spaces between parts
+	mut parts := exp.split(' ').filter(it != '')
 
 	if parts.len < 2 || parts.len > 4 {
 		return error('Expression must contain between 2 and 4 space-separated parts.')
diff --git a/src/cron/expression_parse_test.v b/src/cron/expression_parse_test.v
index 7a4974d..8f22850 100644
--- a/src/cron/expression_parse_test.v
+++ b/src/cron/expression_parse_test.v
@@ -13,53 +13,74 @@ fn parse_range_error(s string, min int, max int) string {
 }
 
 // =====parse_range=====
-fn test_parse_star_range() ? {
+fn test_range_star_range() ? {
 	mut bitv := []bool{init: false, len: 6}
 	parse_range('*', 0, 5, mut bitv) ?
 
 	assert bitv == [true, true, true, true, true, true]
 }
 
-fn test_parse_number() ? {
+fn test_range_number() ? {
 	mut bitv := []bool{init: false, len: 6}
 	parse_range('4', 0, 5, mut bitv) ?
 
 	assert bitv_to_ints(bitv, 0) == [4]
 }
 
-fn test_parse_number_too_large() ? {
+fn test_range_number_too_large() ? {
 	assert parse_range_error('10', 0, 6) == 'Out of range.'
 }
 
-fn test_parse_number_too_small() ? {
+fn test_range_number_too_small() ? {
 	assert parse_range_error('0', 2, 6) == 'Out of range.'
 }
 
-fn test_parse_step_star() ? {
+fn test_range_number_invalid() ? {
+	assert parse_range_error('x', 0, 6) == 'Invalid number.'
+}
+
+fn test_range_step_star_1() ? {
 	mut bitv := []bool{init: false, len: 21}
 	parse_range('*/4', 0, 20, mut bitv) ?
 
 	assert bitv_to_ints(bitv, 0) == [0, 4, 8, 12, 16, 20]
 }
 
-fn test_parse_step_star_too_large() ? {
-	assert parse_range_error('*/21', 0, 20) == 'Step too large.'
+fn test_range_step_star_2() ? {
+	mut bitv := []bool{init: false, len: 9}
+	parse_range('*/3', 1, 8, mut bitv) ?
+
+	assert bitv_to_ints(bitv, 1) == [1, 4, 7]
 }
 
-fn test_parse_step_zero() ? {
+fn test_range_step_star_too_large() ? {
+	assert parse_range_error('*/21', 0, 20) == 'Step size too large.'
+}
+
+fn test_range_step_zero() ? {
 	assert parse_range_error('*/0', 0, 20) == 'Step size zero not allowed.'
 }
 
-fn test_parse_step_number() ? {
+fn test_range_step_number() ? {
 	mut bitv := []bool{init: false, len: 21}
-	parse_range('5/4', 0, 20, mut bitv) ?
-	assert bitv_to_ints(bitv, 0) == [5, 9, 13, 17]
+	parse_range('5/4', 2, 22, mut bitv) ?
+
+	assert bitv_to_ints(bitv, 2) == [5, 9, 13, 17, 21]
 }
 
-fn test_parse_step_number_too_large() ? {
+fn test_range_step_number_too_large() ? {
 	assert parse_range_error('10/4', 0, 5) == 'Out of range.'
 }
 
-fn test_parse_step_number_too_small() ? {
+fn test_range_step_number_too_small() ? {
 	assert parse_range_error('2/4', 5, 10) == 'Out of range.'
 }
+
+// =====parse_part=====
+fn test_part_single() ? {
+	assert parse_part('*', 0, 5) ? == [0, 1, 2, 3, 4, 5]
+}
+
+fn test_part_multiple() ? {
+	assert parse_part('*/2,2/3', 1, 8) ? == [1, 2, 3, 5, 7, 8]
+}