Skip to content

Commit

Permalink
fix(scheduler): ensure negative intervals given to Every return an …
Browse files Browse the repository at this point in the history
…immediate error #600 (#603)

* fix(scheduler): ensure negative intervals given to `Every` return an immediate error #600

* Update Every func doc

* fix error check in tests

Co-authored-by: John Roesler <johnrroesler@gmail.com>

* Apply suggestions from code review

* Apply suggestions from code review

* Update scheduler_test.go

---------

Co-authored-by: John Roesler <johnrroesler@gmail.com>
  • Loading branch information
husam-e and JohnRoesler authored Oct 30, 2023
1 parent 912b4a7 commit 9993f76
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 3 deletions.
7 changes: 7 additions & 0 deletions scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,7 @@ func (s *Scheduler) EveryRandom(lower, upper int) *Scheduler {
// Every schedules a new periodic Job with an interval.
// Interval can be an int, time.Duration or a string that
// parses with time.ParseDuration().
// Negative intervals will return an error.
// Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".
//
// The job is run immediately, unless:
Expand All @@ -553,6 +554,9 @@ func (s *Scheduler) Every(interval interface{}) *Scheduler {
job.error = wrapOrError(job.error, ErrInvalidInterval)
}
case time.Duration:
if interval <= 0 {
job.error = wrapOrError(job.error, ErrInvalidInterval)
}
job.setInterval(0)
job.setDuration(interval)
job.setUnit(duration)
Expand All @@ -561,6 +565,9 @@ func (s *Scheduler) Every(interval interface{}) *Scheduler {
if err != nil {
job.error = wrapOrError(job.error, err)
}
if d <= 0 {
job.error = wrapOrError(job.error, ErrInvalidInterval)
}
job.setDuration(d)
job.setUnit(duration)
default:
Expand Down
8 changes: 5 additions & 3 deletions scheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,10 @@ func TestScheduler_Every_InvalidInterval(t *testing.T) {
interval interface{}
expectedError string
}{
{"zero", 0, ErrInvalidInterval.Error()},
{"negative", -1, ErrInvalidInterval.Error()},
{"zero int", 0, ErrInvalidInterval.Error()},
{"negative int", -1, ErrInvalidInterval.Error()},
{"negative time.Duration", -1 * time.Millisecond, ErrInvalidInterval.Error()},
{"negative string duration", "-1ms", ErrInvalidInterval.Error()},
{"invalid string duration", "bad", "time: invalid duration \"bad\""},
}

Expand All @@ -93,7 +95,7 @@ func TestScheduler_Every_InvalidInterval(t *testing.T) {
t.Run(tc.description, func(t *testing.T) {
_, err := s.Every(tc.interval).Do(func() {})
require.Error(t, err)
assert.EqualError(t, err, tc.expectedError)
assert.ErrorContains(t, err, tc.expectedError)
})
}
}
Expand Down

0 comments on commit 9993f76

Please sign in to comment.