Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 36 additions & 40 deletions crates/goose/src/cron_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,58 +3,54 @@ mod cron_parsing_tests {
use crate::scheduler::normalize_cron_expression;
use tokio_cron_scheduler::Job;

// Helper: drop the last field if we have 7 so tokio_cron_scheduler (6-field) can parse
fn to_tokio_spec(spec: &str) -> String {
let parts: Vec<&str> = spec.split_whitespace().collect();
if parts.len() == 7 {
parts[..6].join(" ")
} else {
spec.to_string()
}
}

#[test]
fn test_normalize_cron_expression() {
// Test 5-field to 6-field conversion
assert_eq!(normalize_cron_expression("0 12 * * *"), "0 0 12 * * *");
assert_eq!(normalize_cron_expression("*/5 * * * *"), "0 */5 * * * *");
assert_eq!(normalize_cron_expression("0 0 * * 1"), "0 0 0 * * 1");
// 5-field → 7-field
assert_eq!(normalize_cron_expression("0 12 * * *"), "0 0 12 * * * *");
assert_eq!(normalize_cron_expression("*/5 * * * *"), "0 */5 * * * * *");
assert_eq!(normalize_cron_expression("0 0 * * 1"), "0 0 0 * * 1 *");

// Test 6-field expressions (should remain unchanged)
assert_eq!(normalize_cron_expression("0 0 12 * * *"), "0 0 12 * * *");
// 6-field → 7-field (append *)
assert_eq!(normalize_cron_expression("0 0 12 * * *"), "0 0 12 * * * *");
assert_eq!(
normalize_cron_expression("*/30 */5 * * * *"),
"*/30 */5 * * * *"
"*/30 */5 * * * * *"
);

// Test invalid expressions (should remain unchanged but warn)
assert_eq!(normalize_cron_expression("* * *"), "* * *");
assert_eq!(normalize_cron_expression("* * * * * * *"), "* * * * * * *");
assert_eq!(normalize_cron_expression(""), "");
// Weekday expressions (unchanged apart from 7-field format)
assert_eq!(normalize_cron_expression("0 * * * 1-5"), "0 0 * * * 1-5 *");
assert_eq!(
normalize_cron_expression("*/20 * * * 1-5"),
"0 */20 * * * 1-5 *"
);
}

#[tokio::test]
async fn test_cron_expression_formats() {
// Test different cron formats to see which ones work
let test_expressions = vec![
("0 0 * * *", "5-field: every day at midnight"),
("0 0 0 * * *", "6-field: every day at midnight"),
("* * * * *", "5-field: every minute"),
("* * * * * *", "6-field: every second"),
("0 */5 * * *", "5-field: every 5 minutes"),
("0 0 */5 * * *", "6-field: every 5 minutes"),
("0 0 12 * * *", "6-field: every day at noon"),
("0 12 * * *", "5-field: every day at noon"),
let samples = [
"0 0 * * *", // 5-field
"0 0 0 * * *", // 6-field
"*/5 * * * *", // 5-field
];

for (expr, desc) in test_expressions {
println!("Testing cron expression: '{}' ({})", expr, desc);
let expr_owned = expr.to_string();

// Test with normalization
let normalized = normalize_cron_expression(expr);
println!(" Normalized to: '{}'", normalized);

match Job::new_async(&normalized, move |_uuid, _l| {
let expr_clone = expr_owned.clone();
Box::pin(async move {
println!("Job executed for: {}", expr_clone);
})
}) {
Ok(_) => println!(" ✅ Successfully parsed normalized: '{}'", normalized),
Err(e) => println!(" ❌ Failed to parse normalized '{}': {}", normalized, e),
}
println!();
for expr in samples {
let norm = normalize_cron_expression(expr);
let tokio_spec = to_tokio_spec(&norm);
assert!(
Job::new_async(&tokio_spec, |_id, _l| Box::pin(async {})).is_ok(),
"failed to parse {} -> {}",
expr,
norm
);
}
}
}
41 changes: 24 additions & 17 deletions crates/goose/src/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,32 +27,39 @@ use crate::session::storage::SessionMetadata;
type RunningTasksMap = HashMap<String, tokio::task::AbortHandle>;
type JobsMap = HashMap<String, (JobId, ScheduledJob)>;

/// Converts a 5-field cron expression to a 6-field expression by prepending "0" for seconds
/// If the expression already has 6 fields, returns it unchanged
/// If the expression is invalid, returns the original expression
pub fn normalize_cron_expression(cron_expr: &str) -> String {
let fields: Vec<&str> = cron_expr.split_whitespace().collect();

match fields.len() {
/// Normalize a cron string so that:
/// 1. It is always in **quartz 7-field format** expected by Temporal
/// (seconds minutes hours dom month dow year).
/// 2. Five-field → prepend seconds `0` and append year `*`.
/// Six-field → append year `*`.
/// 3. Everything else returned unchanged (with a warning).
pub fn normalize_cron_expression(src: &str) -> String {
let mut parts: Vec<&str> = src.split_whitespace().collect();

match parts.len() {
5 => {
// 5-field cron: minute hour day month weekday
// Convert to 6-field: second minute hour day month weekday
format!("0 {}", cron_expr)
// min hour dom mon dow → 0 min hour dom mon dow *
parts.insert(0, "0");
parts.push("*");
}
6 => {
// Already 6-field, return as-is
cron_expr.to_string()
// sec min hour dom mon dow → sec min hour dom mon dow *
parts.push("*");
}
7 => {
// already quartz – do nothing
}
_ => {
// Invalid number of fields, return original (will likely fail parsing later)
tracing::warn!(
"Invalid cron expression '{}': expected 5 or 6 fields, got {}",
cron_expr,
fields.len()
"Unrecognised cron expression '{}': expected 5, 6 or 7 fields (got {}). Leaving unchanged.",
src,
parts.len()
);
cron_expr.to_string()
return src.to_string();
}
}

parts.join(" ")
}

pub fn get_default_scheduler_storage_path() -> Result<PathBuf, io::Error> {
Expand Down
12 changes: 6 additions & 6 deletions crates/goose/src/temporal_scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ impl TemporalScheduler {
let request = JobRequest {
action: "create".to_string(),
job_id: Some(job.id.clone()),
cron: Some(normalized_cron),
cron: Some(normalized_cron.clone()),
recipe_path: Some(job.source.clone()),
execution_mode: job.execution_mode.clone(),
};
Expand Down Expand Up @@ -1336,15 +1336,15 @@ mod tests {
use crate::scheduler::normalize_cron_expression;

// Test cases that should be normalized
assert_eq!(normalize_cron_expression("0 12 * * *"), "0 0 12 * * *");
assert_eq!(normalize_cron_expression("*/5 * * * *"), "0 */5 * * * *");
assert_eq!(normalize_cron_expression("0 0 * * 1"), "0 0 0 * * 1");
assert_eq!(normalize_cron_expression("0 12 * * *"), "0 0 12 * * * *");
assert_eq!(normalize_cron_expression("*/5 * * * *"), "0 */5 * * * * *");
assert_eq!(normalize_cron_expression("0 0 * * 1"), "0 0 0 * * 1 *");

// Test cases that should remain unchanged
assert_eq!(normalize_cron_expression("0 0 12 * * *"), "0 0 12 * * *");
assert_eq!(normalize_cron_expression("0 0 12 * * *"), "0 0 12 * * * *");
assert_eq!(
normalize_cron_expression("*/30 */5 * * * *"),
"*/30 */5 * * * *"
"*/30 */5 * * * * *"
);

println!("✅ Cron normalization works correctly in TemporalScheduler");
Expand Down
Loading