Repository: go-co-op/gocron
Pull request #103 (2021-01-15 15:28:38)

fix bug with StartAt() in v0.5.0
### What does this do?
- fixes a bug we introduced in v0.5.0
- introduced `startAtTime` field on the job struct so we know when a job is scheduled to start at a specific time and we should go ahead and start the timer! 

### Which issue(s) does this PR fix/relate to?
<!--- Put `Resolves #XXX` here to auto-close the issue that your PR fixes (if such) --->

#102 #98 


### List any changes that modify/break current functionality


### Have you included tests for your changes?


### Did you document any new/modified functionality?

- [ ] Updated `example_test.go`
- [ ] Updated `README.md`

### Notes



Patch:
diff --git a/job.go b/job.go
index 1c128fe..162e52d 100644
--- a/job.go
+++ b/job.go
@@ -15,7 +15,8 @@ type Job struct {
 	unit              timeUnit                 // time units, ,e.g. 'minutes', 'hours'...
 	startsImmediately bool                     // if the Job should run upon scheduler start
 	jobFunc           string                   // the Job jobFunc to run, func[jobFunc]
-	atTime            time.Duration            // optional time at which this Job runs
+	atTime            time.Duration            // optional time at which this Job runs when interval is day
+	startAtTime       time.Time                // optional time at which the Job starts
 	err               error                    // error related to Job
 	lastRun           time.Time                // datetime of last run
 	nextRun           time.Time                // datetime of next run
@@ -98,6 +99,18 @@ func (j *Job) setAtTime(t time.Duration) {
 	j.atTime = t
 }
 
+func (j *Job) getStartAtTime() time.Time {
+	j.RLock()
+	defer j.RUnlock()
+	return j.startAtTime
+}
+
+func (j *Job) setStartAtTime(t time.Time) {
+	j.Lock()
+	defer j.Unlock()
+	j.startAtTime = t
+}
+
 // Err returns an error if one occurred while creating the Job
 func (j *Job) Err() error {
 	j.RLock()
@@ -241,6 +254,7 @@ func (j *Job) getMaxRuns() int {
 	return j.runConfig.maxRuns
 }
 
+// TODO: this method seems unnecessary as we could always remove after the run count has expired. Maybe remove this in the future?
 func (j *Job) getRemoveAfterLastRun() bool {
 	j.RLock()
 	defer j.RUnlock()
diff --git a/scheduler.go b/scheduler.go
index 91d6e2b..d7fd86f 100644
--- a/scheduler.go
+++ b/scheduler.go
@@ -68,7 +68,7 @@ func (s *Scheduler) runJobs(jobs []*Job) {
 			j.setStartsImmediately(false)
 		}
 		if !j.shouldRun() {
-			if j.getRemoveAfterLastRun() { // TODO: this method seems unnecessary as we could always remove after the run cout has expired. Maybe remove this in the future?
+			if j.getRemoveAfterLastRun() {
 				s.RemoveByReference(j)
 			}
 			continue
@@ -140,11 +140,7 @@ func (s *Scheduler) scheduleNextRun(job *Job) {
 	now := s.now()
 	lastRun := job.LastRun()
 
-	// job can be scheduled with .StartAt()
 	if job.neverRan() {
-		if !job.NextRun().IsZero() {
-			return // scheduled for future run and should skip scheduling
-		}
 		lastRun = now
 	}
 
@@ -156,28 +152,33 @@ func (s *Scheduler) scheduleNextRun(job *Job) {
 	}))
 }
 
-func (s *Scheduler) durationToNextRun(t time.Time, job *Job) time.Duration {
+func (s *Scheduler) durationToNextRun(lastRun time.Time, job *Job) time.Duration {
+	// job can be scheduled with .StartAt()
+	if job.getStartAtTime().After(lastRun) {
+		return job.getStartAtTime().Sub(s.now())
+	}
+
 	var duration time.Duration
 	switch job.unit {
 	case seconds, minutes, hours:
 		duration = s.calculateDuration(job)
 	case days:
-		duration = s.calculateDays(job, t)
+		duration = s.calculateDays(job, lastRun)
 	case weeks:
 		if job.scheduledWeekday != nil { // weekday selected, Every().Monday(), for example
-			duration = s.calculateWeekday(job, t)
+			duration = s.calculateWeekday(job, lastRun)
 		} else {
-			duration = s.calculateWeeks(job, t)
+			duration = s.calculateWeeks(job, lastRun)
 		}
 	case months:
-		duration = s.calculateMonths(job, t)
+		duration = s.calculateMonths(job, lastRun)
 	}
 	return duration
 }
 
 func (s *Scheduler) getJobLastRun(job *Job) time.Time {
 	if job.neverRan() {
-		return s.time.Now(s.Location())
+		return s.now()
 	}
 	return job.LastRun()
 }
@@ -293,7 +294,7 @@ func (s *Scheduler) roundToMidnight(t time.Time) time.Time {
 // NextRun datetime when the next Job should run.
 func (s *Scheduler) NextRun() (*Job, time.Time) {
 	if len(s.Jobs()) <= 0 {
-		return nil, s.time.Now(s.Location())
+		return nil, s.now()
 	}
 
 	sort.Sort(s)
@@ -464,7 +465,7 @@ func (s *Scheduler) SetTag(t []string) *Scheduler {
 // StartAt schedules the next run of the Job
 func (s *Scheduler) StartAt(t time.Time) *Scheduler {
 	job := s.getCurrentJob()
-	job.setNextRun(t)
+	job.setStartAtTime(t)
 	job.startsImmediately = false
 	return s
 }
@@ -477,7 +478,7 @@ func (s *Scheduler) shouldRun(j *Job) bool {
 		s.RemoveByReference(j)
 	}
 
-	return j.shouldRun() && s.time.Now(s.Location()).Unix() >= j.NextRun().Unix()
+	return j.shouldRun() && s.now().Unix() >= j.NextRun().Unix()
 }
 
 // setUnit sets the unit type


Test patch:
diff --git a/scheduler_test.go b/scheduler_test.go
index c4766b3..890a008 100644
--- a/scheduler_test.go
+++ b/scheduler_test.go
@@ -403,19 +403,39 @@ func TestScheduler_Stop(t *testing.T) {
 }
 
 func TestScheduler_StartAt(t *testing.T) {
-	scheduler := NewScheduler(time.Local)
-	now := time.Now()
-
-	// With StartAt
-	job, _ := scheduler.Every(3).Seconds().StartAt(now.Add(time.Second * 5)).Do(func() {})
-	assert.False(t, job.getStartsImmediately())
-	scheduler.start()
-	assert.Equal(t, now.Add(time.Second*5), job.NextRun())
-	scheduler.stop()
-
-	// Without StartAt
-	job, _ = scheduler.Every(3).Seconds().Do(func() {})
-	assert.True(t, job.getStartsImmediately())
+	t.Run("scheduling", func(t *testing.T) {
+		s := NewScheduler(time.Local)
+		now := time.Now()
+
+		// With StartAt
+		job, _ := s.Every(3).Seconds().StartAt(now.Add(time.Second * 5)).Do(func() {})
+		assert.False(t, job.getStartsImmediately())
+		s.start()
+		assert.Equal(t, now.Add(time.Second*5).Truncate(time.Second), job.NextRun().Truncate(time.Second))
+		s.stop()
+
+		// Without StartAt
+		job, _ = s.Every(3).Seconds().Do(func() {})
+		assert.True(t, job.getStartsImmediately())
+	})
+
+	t.Run("run", func(t *testing.T) {
+		s := NewScheduler(time.UTC)
+		semaphore := make(chan bool)
+
+		s.Every(1).Day().StartAt(s.time.Now(s.location).Add(time.Second)).Do(func() {
+			semaphore <- true
+		})
+
+		s.StartAsync()
+
+		select {
+		case <-time.After(2 * time.Second):
+			t.Fatal("job did not run at 1 second")
+		case <-semaphore:
+			// test passed
+		}
+	})
 }
 
 func TestScheduler_CalculateNextRun(t *testing.T) {