From 2396b056d33318914280812c8b5d4ab045653950 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 30 Jun 2026 16:23:53 +0000 Subject: [PATCH 1/5] Scatter every-N-minutes schedules with workflow-derived offset Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/schedule_fuzzy_scatter.go | 21 +++++++++++ pkg/parser/schedule_parser.go | 4 +- pkg/parser/schedule_parser_stability_test.go | 30 +++++++++++++++ pkg/parser/schedule_parser_test.go | 12 +++--- pkg/workflow/schedule_preprocessing_test.go | 39 +++++++++++++++----- 5 files changed, 88 insertions(+), 18 deletions(-) diff --git a/pkg/parser/schedule_fuzzy_scatter.go b/pkg/parser/schedule_fuzzy_scatter.go index fa11256fa35..cc3d66a9ae0 100644 --- a/pkg/parser/schedule_fuzzy_scatter.go +++ b/pkg/parser/schedule_fuzzy_scatter.go @@ -196,6 +196,7 @@ func ScatterSchedule(fuzzyCron, workflowIdentifier string) (string, error) { handleDaily, handleHourlyWeekdays, handleHourly, + handleEveryMinute, handleWeeklyAround, handleWeeklySpecific, handleWeekly, @@ -325,6 +326,26 @@ func handleHourly(fuzzyCron, workflowIdentifier string) (string, bool, error) { return result, true, nil } +// handleEveryMinute scatters "FUZZY:EVERY_MINUTE/N * * * *" to "M/N * * * *", +// where M is a deterministic offset in [0, N-1] derived from the workflow identifier. +// This ensures that concurrent "every N minutes" workflows start on different minutes, +// distributing execution across the clock face and reducing simultaneous load spikes. +func handleEveryMinute(fuzzyCron, workflowIdentifier string) (string, bool, error) { + const prefix = "FUZZY:EVERY_MINUTE/" + if !strings.HasPrefix(fuzzyCron, prefix) { + return "", false, nil + } + interval, err := parseHourlyInterval(fuzzyCron, prefix, "invalid fuzzy every-minute pattern", "invalid interval in fuzzy every-minute pattern") + if err != nil { + return "", true, err + } + // Derive a start-minute offset in [0, interval-1] so the period is preserved. + offset := stableHash(workflowIdentifier, interval) + result := fmt.Sprintf("%d/%d * * * *", offset, interval) + scheduleFuzzyScatterLog.Printf("FUZZY:EVERY_MINUTE/%d scattered: offset=%d, result=%s", interval, offset, result) + return result, true, nil +} + func handleWeeklyAround(fuzzyCron, workflowIdentifier string) (string, bool, error) { const prefix = "FUZZY:WEEKLY_AROUND:" if !strings.HasPrefix(fuzzyCron, prefix) { diff --git a/pkg/parser/schedule_parser.go b/pkg/parser/schedule_parser.go index ddb37f912ab..5bbc2c7f0c3 100644 --- a/pkg/parser/schedule_parser.go +++ b/pkg/parser/schedule_parser.go @@ -283,7 +283,7 @@ func formatShortDurationCron(interval int, unit string, hasWeekdaysSuffix bool) if hasWeekdaysSuffix { return "", errors.New("minute intervals with 'on weekdays' are not supported") } - return fmt.Sprintf("*/%d * * * *", interval), nil + return fmt.Sprintf("FUZZY:EVERY_MINUTE/%d * * * *", interval), nil case "h": return formatHourlyIntervalCron(interval, hasWeekdaysSuffix), nil case "d": @@ -303,7 +303,7 @@ func formatLongIntervalCron(interval int, unit string, hasWeekdaysSuffix bool) ( if hasWeekdaysSuffix { return "", errors.New("minute intervals with 'on weekdays' are not supported") } - return fmt.Sprintf("*/%d * * * *", interval), nil + return fmt.Sprintf("FUZZY:EVERY_MINUTE/%d * * * *", interval), nil case "hours": return formatHourlyIntervalCron(interval, hasWeekdaysSuffix), nil case "days": diff --git a/pkg/parser/schedule_parser_stability_test.go b/pkg/parser/schedule_parser_stability_test.go index 2e65b36f75a..c01307b1ccc 100644 --- a/pkg/parser/schedule_parser_stability_test.go +++ b/pkg/parser/schedule_parser_stability_test.go @@ -101,6 +101,36 @@ func TestScatterScheduleCrossPlatformConsistency(t *testing.T) { workflowIdentifier: "workflow-a.md", expectedCron: "21 8 * * 1", }, + { + name: "every 10 minutes - workflow-a.md", + fuzzyCron: "FUZZY:EVERY_MINUTE/10 * * * *", + workflowIdentifier: "workflow-a.md", + expectedCron: "1/10 * * * *", + }, + { + name: "every 10 minutes - workflow-b.md", + fuzzyCron: "FUZZY:EVERY_MINUTE/10 * * * *", + workflowIdentifier: "workflow-b.md", + expectedCron: "2/10 * * * *", + }, + { + name: "every 5 minutes - workflow-a.md", + fuzzyCron: "FUZZY:EVERY_MINUTE/5 * * * *", + workflowIdentifier: "workflow-a.md", + expectedCron: "1/5 * * * *", + }, + { + name: "every 30 minutes - workflow-a.md", + fuzzyCron: "FUZZY:EVERY_MINUTE/30 * * * *", + workflowIdentifier: "workflow-a.md", + expectedCron: "21/30 * * * *", + }, + { + name: "every 15 minutes - workflow-b.md", + fuzzyCron: "FUZZY:EVERY_MINUTE/15 * * * *", + workflowIdentifier: "workflow-b.md", + expectedCron: "7/15 * * * *", + }, } for _, tt := range tests { diff --git a/pkg/parser/schedule_parser_test.go b/pkg/parser/schedule_parser_test.go index 8c323fdc110..e20d2af3b2a 100644 --- a/pkg/parser/schedule_parser_test.go +++ b/pkg/parser/schedule_parser_test.go @@ -640,19 +640,19 @@ func TestParseSchedule(t *testing.T) { { name: "every 10 minutes", input: "every 10 minutes", - expectedCron: "*/10 * * * *", + expectedCron: "FUZZY:EVERY_MINUTE/10 * * * *", expectedOrig: "every 10 minutes", }, { name: "every 5 minutes", input: "every 5 minutes", - expectedCron: "*/5 * * * *", + expectedCron: "FUZZY:EVERY_MINUTE/5 * * * *", expectedOrig: "every 5 minutes", }, { name: "every 30 minutes", input: "every 30 minutes", - expectedCron: "*/30 * * * *", + expectedCron: "FUZZY:EVERY_MINUTE/30 * * * *", expectedOrig: "every 30 minutes", }, { @@ -684,7 +684,7 @@ func TestParseSchedule(t *testing.T) { { name: "every 30m", input: "every 30m", - expectedCron: "*/30 * * * *", + expectedCron: "FUZZY:EVERY_MINUTE/30 * * * *", expectedOrig: "every 30m", }, { @@ -969,13 +969,13 @@ func TestParseSchedule(t *testing.T) { { name: "interval at minimum - 5m", input: "every 5m", - expectedCron: "*/5 * * * *", + expectedCron: "FUZZY:EVERY_MINUTE/5 * * * *", expectedOrig: "every 5m", }, { name: "interval at minimum - 5 minutes", input: "every 5 minutes", - expectedCron: "*/5 * * * *", + expectedCron: "FUZZY:EVERY_MINUTE/5 * * * *", expectedOrig: "every 5 minutes", }, diff --git a/pkg/workflow/schedule_preprocessing_test.go b/pkg/workflow/schedule_preprocessing_test.go index c590c4336e5..5904ed2c2c8 100644 --- a/pkg/workflow/schedule_preprocessing_test.go +++ b/pkg/workflow/schedule_preprocessing_test.go @@ -216,7 +216,7 @@ func TestSchedulePreprocessingShorthandOnString(t *testing.T) { frontmatter: map[string]any{ "on": "every 10 minutes", }, - expectedCron: "*/10 * * * *", + checkScattered: true, // Scattered minute-interval schedule expectWorkflowDispatch: true, }, { @@ -355,11 +355,13 @@ func TestSchedulePreprocessingShorthandOnString(t *testing.T) { func TestSchedulePreprocessing(t *testing.T) { tests := []struct { - name string - frontmatter map[string]any - expectedCron string - expectedError bool - errorSubstring string + name string + frontmatter map[string]any + workflowIdentifier string + expectedCron string + checkScattered bool + expectedError bool + errorSubstring string }{ { name: "daily schedule", @@ -398,7 +400,8 @@ func TestSchedulePreprocessing(t *testing.T) { }, }, }, - expectedCron: "*/10 * * * *", + workflowIdentifier: "test-workflow.md", + checkScattered: true, }, { name: "existing cron expression unchanged", @@ -469,7 +472,8 @@ func TestSchedulePreprocessing(t *testing.T) { "schedule": "every 10 minutes", }, }, - expectedCron: "*/10 * * * *", + workflowIdentifier: "test-workflow.md", + checkScattered: true, }, { name: "shorthand string format - existing cron", @@ -495,6 +499,9 @@ func TestSchedulePreprocessing(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { compiler := NewCompiler() + if tt.workflowIdentifier != "" { + compiler.SetWorkflowIdentifier(tt.workflowIdentifier) + } err := compiler.preprocessScheduleFields(tt.frontmatter, "", "") if tt.expectedError { @@ -519,8 +526,20 @@ func TestSchedulePreprocessing(t *testing.T) { firstSchedule := scheduleArray[0].(map[string]any) actualCron := firstSchedule["cron"].(string) - if actualCron != tt.expectedCron { - t.Errorf("expected cron '%s', got '%s'", tt.expectedCron, actualCron) + if tt.checkScattered { + // Should be scattered to a valid cron (not fuzzy) + if strings.HasPrefix(actualCron, "FUZZY:") { + t.Errorf("expected scattered cron, got fuzzy: %s", actualCron) + } + fields := strings.Fields(actualCron) + if len(fields) != 5 { + t.Errorf("expected 5 fields in cron expression, got %d: %s", len(fields), actualCron) + } + t.Logf("Successfully scattered schedule to: %s", actualCron) + } else if tt.expectedCron != "" { + if actualCron != tt.expectedCron { + t.Errorf("expected cron '%s', got '%s'", tt.expectedCron, actualCron) + } } }) } From eade16ded8ffb8414db69346d51c519ee260994f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 30 Jun 2026 16:28:02 +0000 Subject: [PATCH 2/5] Address review feedback: guard, comments, stronger offset-range test Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/schedule_fuzzy_scatter.go | 8 ++++++- pkg/workflow/schedule_preprocessing_test.go | 24 +++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/pkg/parser/schedule_fuzzy_scatter.go b/pkg/parser/schedule_fuzzy_scatter.go index cc3d66a9ae0..14080e3e916 100644 --- a/pkg/parser/schedule_fuzzy_scatter.go +++ b/pkg/parser/schedule_fuzzy_scatter.go @@ -335,11 +335,17 @@ func handleEveryMinute(fuzzyCron, workflowIdentifier string) (string, bool, erro if !strings.HasPrefix(fuzzyCron, prefix) { return "", false, nil } + // parseHourlyInterval extracts the integer N from a "PREFIX/N ..." pattern; + // it is reused here because the parsing logic is identical. interval, err := parseHourlyInterval(fuzzyCron, prefix, "invalid fuzzy every-minute pattern", "invalid interval in fuzzy every-minute pattern") if err != nil { return "", true, err } - // Derive a start-minute offset in [0, interval-1] so the period is preserved. + if interval < 1 { + return "", true, fmt.Errorf("invalid interval in fuzzy every-minute pattern: interval must be >= 1, got %d", interval) + } + // stableHash returns a value in [0, interval-1], so offset is always a valid + // starting minute that preserves the N-minute period. offset := stableHash(workflowIdentifier, interval) result := fmt.Sprintf("%d/%d * * * *", offset, interval) scheduleFuzzyScatterLog.Printf("FUZZY:EVERY_MINUTE/%d scattered: offset=%d, result=%s", interval, offset, result) diff --git a/pkg/workflow/schedule_preprocessing_test.go b/pkg/workflow/schedule_preprocessing_test.go index 5904ed2c2c8..a816b0fb5ae 100644 --- a/pkg/workflow/schedule_preprocessing_test.go +++ b/pkg/workflow/schedule_preprocessing_test.go @@ -4,6 +4,7 @@ package workflow import ( "fmt" + "strconv" "strings" "testing" ) @@ -343,6 +344,17 @@ func TestSchedulePreprocessingShorthandOnString(t *testing.T) { if len(fields) != 5 { t.Errorf("expected 5 fields in cron expression, got %d: %s", len(fields), actualCron) } + // If the minute field uses M/N start/step syntax, verify offset is in [0, N-1]. + minuteField := fields[0] + if parts := strings.SplitN(minuteField, "/", 2); len(parts) == 2 { + offset, offsetErr := strconv.Atoi(parts[0]) + interval, intervalErr := strconv.Atoi(parts[1]) + if offsetErr == nil && intervalErr == nil { + if offset < 0 || offset >= interval { + t.Errorf("offset %d is not in [0, %d): %s", offset, interval, actualCron) + } + } + } t.Logf("Successfully scattered schedule to: %s", actualCron) } else if tt.expectedCron != "" { if actualCron != tt.expectedCron { @@ -535,6 +547,18 @@ func TestSchedulePreprocessing(t *testing.T) { if len(fields) != 5 { t.Errorf("expected 5 fields in cron expression, got %d: %s", len(fields), actualCron) } + // If the minute field uses the M/N start/step syntax, verify + // offset is within [0, N-1] so the period is preserved. + minuteField := fields[0] + if parts := strings.SplitN(minuteField, "/", 2); len(parts) == 2 { + offset, offsetErr := strconv.Atoi(parts[0]) + interval, intervalErr := strconv.Atoi(parts[1]) + if offsetErr == nil && intervalErr == nil { + if offset < 0 || offset >= interval { + t.Errorf("offset %d is not in [0, %d): %s", offset, interval, actualCron) + } + } + } t.Logf("Successfully scattered schedule to: %s", actualCron) } else if tt.expectedCron != "" { if actualCron != tt.expectedCron { From d3b7890d5a111f25c115a510af580ee463d49c9f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 30 Jun 2026 17:38:17 +0000 Subject: [PATCH 3/5] docs(adr): add draft ADR-42524 for every-N-minutes schedule scattering Co-Authored-By: Claude Sonnet 4.6 --- ...42524-scatter-every-n-minutes-schedules.md | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 docs/adr/42524-scatter-every-n-minutes-schedules.md diff --git a/docs/adr/42524-scatter-every-n-minutes-schedules.md b/docs/adr/42524-scatter-every-n-minutes-schedules.md new file mode 100644 index 00000000000..713e0558960 --- /dev/null +++ b/docs/adr/42524-scatter-every-n-minutes-schedules.md @@ -0,0 +1,48 @@ +# ADR-42524: Scatter Every-N-Minutes Schedules via Fuzzy Token Pipeline + +**Date**: 2026-06-30 +**Status**: Draft +**Deciders**: Unknown + +--- + +### Context + +Workflows using "every N minutes" natural-language schedule syntax (e.g., `every 10 minutes`, `every 5m`) were compiled directly to standard cron `*/N * * * *` expressions. Because all workflows sharing the same interval produce the same cron pattern, they all fire simultaneously, creating predictable load spikes at minute boundaries. The codebase already contained a fuzzy-scatter pipeline (`ScatterSchedule`) that resolves intermediate `FUZZY:*` tokens to deterministically offset cron expressions for hourly and weekly schedules. Minute-interval schedules were not included in this pipeline, leaving a gap in the thundering-herd mitigation strategy. + +### Decision + +We will extend the existing fuzzy-scatter pipeline to cover minute-interval schedules. The schedule parser now emits `FUZZY:EVERY_MINUTE/N * * * *` instead of `*/N * * * *` for "every N minutes" inputs. A new `handleEveryMinute` handler in the scatter layer resolves this token to `M/N * * * *`, where `M = stableHash(workflowIdentifier, N) ∈ [0, N-1]`, preserving the period while distributing start minutes across the clock face. The offset is deterministic per workflow identifier so recompilation produces the same cron without drift. Raw cron expressions (`*/N * * * *` typed directly by users) bypass scatter as before. + +### Alternatives Considered + +#### Alternative 1: Keep `*/N * * * *` and accept simultaneous firing + +The simplest option: do nothing. Workflows with the same interval continue to fire at the same minute. This was rejected because simultaneous firing creates measurable load spikes, and the infrastructure to mitigate it already exists for other schedule types — extending it to minute intervals is low-cost and consistent. + +#### Alternative 2: Apply a random offset at compile time + +Assign a random offset `M ∈ [0, N-1]` when compiling the schedule, storing the result as a plain cron expression. This avoids the intermediate `FUZZY:` token. It was rejected because a random offset changes on every recompilation, causing schedule drift — a workflow would silently shift its firing time whenever its definition is reprocessed. The deterministic hash approach used in the decision ensures the same workflow always produces the same cron, which is safer for audit trails and change detection. + +#### Alternative 3: Offset by a fixed per-repo value rather than per-workflow hash + +Use a single fixed offset derived from the repository rather than the workflow identifier. This was rejected because all workflows in the same repo would still cluster together at the same offset, merely shifting the load spike rather than distributing it. + +### Consequences + +#### Positive +- Workflows sharing a minute interval are now spread across the clock face, reducing simultaneous firing and the associated load spikes. +- The offset is deterministic — same workflow identifier always produces the same cron, so recompilation is idempotent and auditable. +- The fix reuses the existing `stableHash` and `ScatterSchedule` infrastructure, keeping the implementation surface small and consistent with the hourly/weekly scatter approach. + +#### Negative +- The compiled cron output changes from the intuitive `*/N` form to the less-familiar `M/N` step-with-start form. Both are semantically equivalent but the latter may surprise users reading raw cron values. +- Existing stored cron expressions (`*/N * * * *`) generated before this change are not retroactively updated; workflows will only receive a scattered offset on their next recompilation. + +#### Neutral +- The intermediate `FUZZY:EVERY_MINUTE/N` token must now be understood by any tooling that inspects or stores cron expressions mid-pipeline. +- Tests for minute-interval schedules now assert offset-in-range (`[0, N-1]`) rather than exact cron strings, which is more robust to hash-function changes but loses exact value pinning (except in the cross-platform stability test). + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From 4221eea1923c76f5bee7b1036cf9ac8def957545 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 30 Jun 2026 18:46:36 +0000 Subject: [PATCH 4/5] Plan: fix checkScattered assertions and fuzz test FUZZY field count Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/issue-monster.lock.yml | 4 ++-- .github/workflows/pr-sous-chef.lock.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/issue-monster.lock.yml b/.github/workflows/issue-monster.lock.yml index b012ae1553d..985682dad12 100644 --- a/.github/workflows/issue-monster.lock.yml +++ b/.github/workflows/issue-monster.lock.yml @@ -65,8 +65,8 @@ on: # issues: read # pull-requests: read schedule: - - cron: "*/30 * * * *" - # Friendly format: every 30m + - cron: "5/30 * * * *" + # Friendly format: every 30m (scattered) # skip-if-check-failing: # Skip-if-check-failing processed as check status gate in pre-activation job # allow-pending: true # include: diff --git a/.github/workflows/pr-sous-chef.lock.yml b/.github/workflows/pr-sous-chef.lock.yml index f6565c2ae0e..3b60951e699 100644 --- a/.github/workflows/pr-sous-chef.lock.yml +++ b/.github/workflows/pr-sous-chef.lock.yml @@ -62,8 +62,8 @@ name: "PR Sous Chef" on: schedule: - - cron: "*/15 * * * *" - # Friendly format: every 15m + - cron: "8/15 * * * *" + # Friendly format: every 15m (scattered) # skip-if-no-match: is:pr is:open -is:draft # Skip-if-no-match processed as search check in pre-activation job workflow_dispatch: inputs: From b08c92d1ef8f106f2b8673714c46f8e4f991b279 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 30 Jun 2026 18:50:53 +0000 Subject: [PATCH 5/5] fix: strengthen checkScattered assertions and update fuzz test for EVERY_MINUTE 5-field FUZZY - Return early after len(fields)!=5 check to avoid potential panic - Fail explicitly when minute field offset is non-numeric (catches unscattered */N) - Allow FUZZY:EVERY_MINUTE/N to have 5 fields in fuzz test (other FUZZY types remain 4-field) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/schedule_parser_fuzz_test.go | 25 +++++++++++++------- pkg/workflow/schedule_preprocessing_test.go | 26 +++++++++++++-------- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/pkg/parser/schedule_parser_fuzz_test.go b/pkg/parser/schedule_parser_fuzz_test.go index 9fb98980db8..780595ba9a8 100644 --- a/pkg/parser/schedule_parser_fuzz_test.go +++ b/pkg/parser/schedule_parser_fuzz_test.go @@ -381,18 +381,25 @@ func FuzzScheduleParser(f *testing.F) { // 4. Validate cron expression format if successful if err == nil && cron != "" { - // Allow fuzzy schedules (FUZZY:*) which have 4 fields + // Allow fuzzy schedules (FUZZY:*) which have 4 fields, except + // FUZZY:EVERY_MINUTE/N which carries all 5 cron fields. if strings.HasPrefix(cron, "FUZZY:") { - // Fuzzy schedules have the format: - // - "FUZZY:DAILY * * *" (4 fields) - // - "FUZZY:HOURLY/N * * *" (4 fields) - // - "FUZZY:DAILY_AROUND:HH:MM * * *" (4 fields but with colon-separated time in first field) - // - "FUZZY:DAILY_BETWEEN:START_H:START_M:END_H:END_M * * *" (4 fields with 4 colon-separated values in first field) fields := strings.Fields(cron) - if len(fields) != 4 { - t.Errorf("ParseSchedule returned invalid fuzzy cron format with %d fields (expected 4): %q for input: %q", len(fields), cron, input) + if strings.HasPrefix(cron, "FUZZY:EVERY_MINUTE/") { + // "FUZZY:EVERY_MINUTE/N * * * *" — 5 fields + if len(fields) != 5 { + t.Errorf("ParseSchedule returned invalid FUZZY:EVERY_MINUTE format with %d fields (expected 5): %q for input: %q", len(fields), cron, input) + } + } else { + // All other fuzzy schedules have the format: + // - "FUZZY:DAILY * * *" (4 fields) + // - "FUZZY:HOURLY/N * * *" (4 fields) + // - "FUZZY:DAILY_AROUND:HH:MM * * *" (4 fields but with colon-separated time in first field) + // - "FUZZY:DAILY_BETWEEN:START_H:START_M:END_H:END_M * * *" (4 fields with 4 colon-separated values in first field) + if len(fields) != 4 { + t.Errorf("ParseSchedule returned invalid fuzzy cron format with %d fields (expected 4): %q for input: %q", len(fields), cron, input) + } } - // For FUZZY:DAILY_AROUND, validate the time format if strings.HasPrefix(cron, "FUZZY:DAILY_AROUND:") { // Extract the time part from FUZZY:DAILY_AROUND:HH:MM diff --git a/pkg/workflow/schedule_preprocessing_test.go b/pkg/workflow/schedule_preprocessing_test.go index a816b0fb5ae..9691e4cce36 100644 --- a/pkg/workflow/schedule_preprocessing_test.go +++ b/pkg/workflow/schedule_preprocessing_test.go @@ -343,16 +343,19 @@ func TestSchedulePreprocessingShorthandOnString(t *testing.T) { fields := strings.Fields(actualCron) if len(fields) != 5 { t.Errorf("expected 5 fields in cron expression, got %d: %s", len(fields), actualCron) + return } - // If the minute field uses M/N start/step syntax, verify offset is in [0, N-1]. + // If the minute field uses M/N start/step syntax, verify that the + // offset is a valid integer in [0, N-1]. A non-integer offset (e.g. + // "*" from an unscattered "*/N") is itself a failure. minuteField := fields[0] if parts := strings.SplitN(minuteField, "/", 2); len(parts) == 2 { offset, offsetErr := strconv.Atoi(parts[0]) interval, intervalErr := strconv.Atoi(parts[1]) - if offsetErr == nil && intervalErr == nil { - if offset < 0 || offset >= interval { - t.Errorf("offset %d is not in [0, %d): %s", offset, interval, actualCron) - } + if offsetErr != nil || intervalErr != nil { + t.Errorf("minute field %q is not a valid M/N expression: %s", minuteField, actualCron) + } else if offset < 0 || offset >= interval { + t.Errorf("offset %d is not in [0, %d): %s", offset, interval, actualCron) } } t.Logf("Successfully scattered schedule to: %s", actualCron) @@ -546,17 +549,20 @@ func TestSchedulePreprocessing(t *testing.T) { fields := strings.Fields(actualCron) if len(fields) != 5 { t.Errorf("expected 5 fields in cron expression, got %d: %s", len(fields), actualCron) + return } // If the minute field uses the M/N start/step syntax, verify - // offset is within [0, N-1] so the period is preserved. + // that the offset is a valid integer in [0, N-1] so the period + // is preserved. A non-integer offset (e.g. "*" from an + // unscattered "*/N") is itself a failure. minuteField := fields[0] if parts := strings.SplitN(minuteField, "/", 2); len(parts) == 2 { offset, offsetErr := strconv.Atoi(parts[0]) interval, intervalErr := strconv.Atoi(parts[1]) - if offsetErr == nil && intervalErr == nil { - if offset < 0 || offset >= interval { - t.Errorf("offset %d is not in [0, %d): %s", offset, interval, actualCron) - } + if offsetErr != nil || intervalErr != nil { + t.Errorf("minute field %q is not a valid M/N expression: %s", minuteField, actualCron) + } else if offset < 0 || offset >= interval { + t.Errorf("offset %d is not in [0, %d): %s", offset, interval, actualCron) } } t.Logf("Successfully scattered schedule to: %s", actualCron)