Skip to content

Commit ea7766f

Browse files
evgeniy-scherbinaevgeniy-scherbina
committed
test: improve test coverage for schedules-overlap function
1 parent bdfcd9e commit ea7766f

File tree

2 files changed

+93
-23
lines changed

2 files changed

+93
-23
lines changed

‎coderd/schedule/cron/schedule_validation.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
)
99

1010
// ValidateSchedules checks if any schedules overlap
11-
func ValidateSchedules(schedules []*Schedule) error {
11+
func ValidateSchedules(schedules []string) error {
1212
for i := 0; i < len(schedules); i++ {
1313
for j := i + 1; j < len(schedules); j++ {
1414
overlap, err := SchedulesOverlap(schedules[i], schedules[j])
@@ -17,7 +17,7 @@ func ValidateSchedules(schedules []*Schedule) error {
1717
}
1818
if overlap {
1919
return xerrors.Errorf("schedules overlap: %s and %s",
20-
schedules[i].Cron(), schedules[j].Cron())
20+
schedules[i], schedules[j])
2121
}
2222
}
2323
}
@@ -26,10 +26,10 @@ func ValidateSchedules(schedules []*Schedule) error {
2626

2727
// SchedulesOverlap checks if two schedules overlap by checking
2828
// days, months, and hours separately
29-
func SchedulesOverlap(s1, s2 *Schedule) (bool, error) {
29+
func SchedulesOverlap(schedule1, schedule2 string) (bool, error) {
3030
// Get cron fields
31-
fields1 := strings.Fields(s1.Cron())
32-
fields2 := strings.Fields(s2.Cron())
31+
fields1 := strings.Fields(schedule1)
32+
fields2 := strings.Fields(schedule2)
3333

3434
// Check if months overlap
3535
monthsOverlap, err := MonthsOverlap(fields1[3], fields2[3])

‎coderd/schedule/cron/schedule_validation_test.go

Lines changed: 88 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -348,24 +348,107 @@ func TestSchedulesOverlap(t *testing.T) {
348348
overlap bool
349349
expectErr bool
350350
}{
351+
// Basic overlap cases
351352
{
352353
name: "Same schedule",
353354
s1: "* 9-18 * * 1-5",
354355
s2: "* 9-18 * * 1-5",
355356
overlap: true,
356357
},
357358
{
358-
name: "Different hours",
359+
name: "Different hours - no overlap",
359360
s1: "* 9-12 * * 1-5",
360361
s2: "* 13-18 * * 1-5",
361362
overlap: false,
362363
},
363364
{
364-
name: "Different DOW but wildcard DOM",
365+
name: "Different hours - partial overlap",
366+
s1: "* 9-14 * * 1-5",
367+
s2: "* 12-18 * * 1-5",
368+
overlap: true,
369+
},
370+
{
371+
name: "Different hours - one contained in another",
372+
s1: "* 9-18 * * 1-5",
373+
s2: "* 12-14 * * 1-5",
374+
overlap: true,
375+
},
376+
377+
// Day of week overlap cases (with wildcard DOM)
378+
{
379+
name: "Different DOW with wildcard DOM",
380+
s1: "* 9-18 * * 1,3,5",
381+
s2: "* 9-18 * * 2,4,6",
382+
overlap: true, // Overlaps because of wildcard DOM
383+
},
384+
{
385+
name: "Different DOW with wildcard DOM - complex ranges",
365386
s1: "* 9-18 * * 1-3",
366387
s2: "* 9-18 * * 4-5",
367-
overlap: true, // true because both have wildcard DOM
388+
overlap: true, // Overlaps because of wildcard DOM
389+
},
390+
391+
// Day of week overlap cases (with specific DOM)
392+
{
393+
name: "Different DOW with specific DOM - no overlap",
394+
s1: "* 9-18 1 * 1-3",
395+
s2: "* 9-18 2 * 4-5",
396+
overlap: false, // No overlap because different DOM and DOW
397+
},
398+
{
399+
name: "Different DOW with specific DOM - partial overlap",
400+
s1: "* 9-18 1 * 1-4",
401+
s2: "* 9-18 1 * 3-5",
402+
overlap: true, // Overlaps because same DOM
403+
},
404+
{
405+
name: "Different DOW with specific DOM - complex ranges",
406+
s1: "* 9-18 1 * 1,3,5",
407+
s2: "* 9-18 1 * 2,4,6",
408+
overlap: true, // Overlaps because same DOM
409+
},
410+
411+
// Wildcard cases
412+
{
413+
name: "Wildcard hours vs specific hours",
414+
s1: "* * * * 1-5",
415+
s2: "* 9-18 * * 1-5",
416+
overlap: true,
417+
},
418+
{
419+
name: "Wildcard DOW vs specific DOW",
420+
s1: "* 9-18 * * *",
421+
s2: "* 9-18 * * 1-5",
422+
overlap: true,
368423
},
424+
{
425+
name: "Both wildcard DOW",
426+
s1: "* 9-18 * * *",
427+
s2: "* 9-18 * * *",
428+
overlap: true,
429+
},
430+
431+
// Complex time ranges
432+
{
433+
name: "Complex hour ranges - no overlap",
434+
s1: "* 9-11,13-15 * * 1-5",
435+
s2: "* 12,16-18 * * 1-5",
436+
overlap: false,
437+
},
438+
{
439+
name: "Complex hour ranges - partial overlap",
440+
s1: "* 9-11,13-15 * * 1-5",
441+
s2: "* 10-12,14-16 * * 1-5",
442+
overlap: true,
443+
},
444+
{
445+
name: "Complex hour ranges - contained",
446+
s1: "* 9-18 * * 1-5",
447+
s2: "* 10-11,13-14 * * 1-5",
448+
overlap: true,
449+
},
450+
451+
// Error cases (keeping minimal)
369452
{
370453
name: "Invalid hour range",
371454
s1: "* 25-26 * * 1-5",
@@ -374,7 +457,7 @@ func TestSchedulesOverlap(t *testing.T) {
374457
},
375458
{
376459
name: "Invalid month range",
377-
s1: "* 9-18 13 * 1-5",
460+
s1: "* 9-18 * 13 1-5",
378461
s2: "* 9-18 * * 1-5",
379462
expectErr: true,
380463
},
@@ -384,21 +467,8 @@ func TestSchedulesOverlap(t *testing.T) {
384467
testCase := testCase
385468
t.Run(testCase.name, func(t *testing.T) {
386469
t.Parallel()
387-
s1, err := cron.Weekly(testCase.s1)
388-
if testCase.expectErr {
389-
require.Error(t, err)
390-
return
391-
}
392-
require.NoError(t, err)
393-
394-
s2, err := cron.Weekly(testCase.s2)
395-
if testCase.expectErr {
396-
require.Error(t, err)
397-
return
398-
}
399-
require.NoError(t, err)
400470

401-
overlap, err := cron.SchedulesOverlap(s1, s2)
471+
overlap, err := cron.SchedulesOverlap(testCase.s1, testCase.s2)
402472
if testCase.expectErr {
403473
require.Error(t, err)
404474
return

0 commit comments

Comments
 (0)