[BUG] - `StartAt` in the past causes next execution time to calculate based on `Now`

[calvinmclean]

Using StartAt with a date in the past doesn't behave as expected

If I am creating a scheduled job and use StartAt with a date in the past, I expected the next execution time to be calculated based on that start date and the defined interval, but instead it uses the current time and the interval.

I am not sure if that is a bug or intended behavior so I chose to open this issue before creating a PR. If you support this change, I can go ahead and do the PR myself.

To Reproduce

Steps to reproduce the behavior:

package main

import (
    "fmt"
    "time"

    "github.com/go-co-op/gocron"
)

func task() {
    fmt.Println("EXECUTING")
}

func main() {
    scheduler := gocron.NewScheduler(time.Local)

    job, _ := scheduler.
        Every(24).
        Hours().
        StartAt(time.Now().Add(-24 * time.Hour).Add(10 * Minute)).
        Do(task)

    scheduler.StartAsync()
    fmt.Println(job.NextRun())
}

Version

v0.5.1

Expected behavior

The expected output of the code above would be today, at the current time + 10 minutes. Instead, it is 24 hours from the current time which does not fit in the configured schedule.

Additional context

My use-case here is an automated garden where I have a Plant struct containing a StartDate to record when it was first planted and serve as the starting point for my watering interval. I want to be able to restart my application, read the StartDate from a database or configuration file and run the scheduled jobs based on that instead of based on when the program started.

I implemented a workaround in my code but wanted to have the opportunity to include it in goCron if that is a desirable feature/fix.

[JohnRoesler]

@calvinmclean thank for opening this issue!

I would say it’s currently working as designed. But, I don’t believe we specifically said that a date in the past would result in this behavior.

The name StartAt doesn’t imply relative to anything, so I am ok having it handle times in the past.

How would the implementation you’re considering calculate the next run?

[calvinmclean]

Originally I was thinking about changing this line to set lastRun = job.startAtTime, but realized this won't work if the start time is more than one interval in the past. Instead, we would have to loop and use durationToNextRun until the next run time is in the future. This might not be ideal if the StartAt is far in the past though.

[JohnRoesler]

Perhaps before that line, we could add an if check:

// scheduleNextRun Compute the instant when this Job should run next
func (s *Scheduler) scheduleNextRun(job *Job) {
    now := s.now()
    lastRun := job.LastRun()

    if job.neverRan() {
+       if job.startAtTime.Before(s.time.Now()) {
+           // override the `startAtTime` value to the next appropriate future time
+       }
        lastRun = now
    }

    durationToNextRun := s.durationToNextRun(lastRun, job)
    job.setNextRun(lastRun.Add(durationToNextRun))
    job.setTimer(time.AfterFunc(durationToNextRun, func() {
        s.run(job)
        s.scheduleNextRun(job)
    }))
}

[JohnRoesler]

This is released in v0.6.0!