Merge branch 'master' of https://github.com/robfig/cron

; Conflicts:
;	cron.go

Author: unknwon

cron.go

@@ -171,11 +171,11 @@ func (c *Cron) runWithRecovery(j Job) {
 	j.Run()
 }
 
-// Run the scheduler.. this is private just due to the need to synchronize
+// Run the scheduler. this is private just due to the need to synchronize
 // access to the 'running' state variable.
 func (c *Cron) run() {
 	// Figure out the next activation times for each entry.
-	now := time.Now().In(c.location)
+	now := c.now()
 	for _, entry := range c.entries {
 		entry.Next = entry.Schedule.Next(now)
 	}
@@ -184,46 +184,47 @@ func (c *Cron) run() {
 		// Determine the next entry to run.
 		sort.Sort(byTime(c.entries))
 
-		var effective time.Time
+		var timer *time.Timer
 		if len(c.entries) == 0 || c.entries[0].Next.IsZero() {
 			// If there are no entries yet, just sleep - it still handles new entries
 			// and stop requests.
-			effective = now.AddDate(10, 0, 0)
+			timer = time.NewTimer(100000 * time.Hour)
 		} else {
-			effective = c.entries[0].Next
+			timer = time.NewTimer(c.entries[0].Next.Sub(now))
 		}
 
-		timer := time.NewTimer(effective.Sub(now))
-		select {
-		case now = <-timer.C:
-			now = now.In(c.location)
-			// Run every entry whose next time was this effective time.
-			for _, e := range c.entries {
-				if e.Next != effective {
-					break
+		for {
+			select {
+			case now = <-timer.C:
+				now = now.In(c.location)
+				// Run every entry whose next time was less than now
+				for _, e := range c.entries {
+					if e.Next.After(now) || e.Next.IsZero() {
+						break
+					}
+					go c.runWithRecovery(e.Job)
+					e.ExecTimes++
+					e.Prev = e.Next
+					e.Next = e.Schedule.Next(now)
 				}
-				go c.runWithRecovery(e.Job)
-				e.ExecTimes++
-				e.Prev = e.Next
-				e.Next = e.Schedule.Next(now)
-			}
-			continue
 
-		case newEntry := <-c.add:
-			c.entries = append(c.entries, newEntry)
-			newEntry.Next = newEntry.Schedule.Next(time.Now().In(c.location))
+			case newEntry := <-c.add:
+				timer.Stop()
+				now = c.now()
+				newEntry.Next = newEntry.Schedule.Next(now)
+				c.entries = append(c.entries, newEntry)
 
-		case <-c.snapshot:
-			c.snapshot <- c.entrySnapshot()
+			case <-c.snapshot:
+				c.snapshot <- c.entrySnapshot()
+				continue
 
-		case <-c.stop:
-			timer.Stop()
-			return
-		}
+			case <-c.stop:
+				timer.Stop()
+				return
+			}
 
-		// 'now' should be updated after newEntry and snapshot cases.
-		now = time.Now().In(c.location)
-		timer.Stop()
+			break
+		}
 	}
 }
 
@@ -261,3 +262,8 @@ func (c *Cron) entrySnapshot() []*Entry {
 	}
 	return entries
 }
+
+// now returns current time in c location
+func (c *Cron) now() time.Time {
+	return time.Now().In(c.location)
+}

cron_test.go

@@ -10,7 +10,7 @@ import (
 // Many tests schedule a job for every second, and then wait at most a second
 // for it to run.  This amount is just slightly larger than 1 second to
 // compensate for a few milliseconds of runtime.
-const ONE_SECOND = 1*time.Second + 10*time.Millisecond
+const OneSecond = 1*time.Second + 10*time.Millisecond
 
 func TestFuncPanicRecovery(t *testing.T) {
 	cron := New()
@@ -19,7 +19,7 @@ func TestFuncPanicRecovery(t *testing.T) {
 	cron.AddFunc("", "* * * * * ?", func() { panic("YOLO") })
 
 	select {
-	case <-time.After(ONE_SECOND):
+	case <-time.After(OneSecond):
 		return
 	}
 }
@@ -39,7 +39,7 @@ func TestJobPanicRecovery(t *testing.T) {
 	cron.AddJob("", "* * * * * ?", job)
 
 	select {
-	case <-time.After(ONE_SECOND):
+	case <-time.After(OneSecond):
 		return
 	}
 }
@@ -50,8 +50,8 @@ func TestNoEntries(t *testing.T) {
 	cron.Start()
 
 	select {
-	case <-time.After(ONE_SECOND):
-		t.FailNow()
+	case <-time.After(OneSecond):
+		t.Fatal("expected cron will be stopped immediately")
 	case <-stop(cron):
 	}
 }
@@ -67,10 +67,10 @@ func TestStopCausesJobsToNotRun(t *testing.T) {
 	cron.AddFunc("", "* * * * * ?", func() { wg.Done() })
 
 	select {
-	case <-time.After(ONE_SECOND):
+	case <-time.After(OneSecond):
 		// No job ran!
 	case <-wait(wg):
-		t.FailNow()
+		t.Fatal("expected stopped cron does not run any job")
 	}
 }
 
@@ -86,8 +86,8 @@ func TestAddBeforeRunning(t *testing.T) {
 
 	// Give cron 2 seconds to run our job (which is always activated).
 	select {
-	case <-time.After(ONE_SECOND):
-		t.FailNow()
+	case <-time.After(OneSecond):
+		t.Fatal("expected job runs")
 	case <-wait(wg):
 	}
 }
@@ -103,8 +103,8 @@ func TestAddWhileRunning(t *testing.T) {
 	cron.AddFunc("", "* * * * * ?", func() { wg.Done() })
 
 	select {
-	case <-time.After(ONE_SECOND):
-		t.FailNow()
+	case <-time.After(OneSecond):
+		t.Fatal("expected job runs")
 	case <-wait(wg):
 	}
 }
@@ -118,10 +118,9 @@ func TestAddWhileRunningWithDelay(t *testing.T) {
 	var calls = 0
 	cron.AddFunc("", "* * * * * *", func() { calls += 1 })
 
-	<-time.After(ONE_SECOND)
+	<-time.After(OneSecond)
 	if calls != 1 {
-		fmt.Printf("called %d times, expected 1\n", calls)
-		t.Fail()
+		t.Errorf("called %d times, expected 1\n", calls)
 	}
 }
 
@@ -137,14 +136,14 @@ func TestSnapshotEntries(t *testing.T) {
 
 	// Cron should fire in 2 seconds. After 1 second, call Entries.
 	select {
-	case <-time.After(ONE_SECOND):
+	case <-time.After(OneSecond):
 		cron.Entries()
 	}
 
 	// Even though Entries was called, the cron should fire at the 2 second mark.
 	select {
-	case <-time.After(ONE_SECOND):
-		t.FailNow()
+	case <-time.After(OneSecond):
+		t.Error("expected job runs at 2 second mark")
 	case <-wait(wg):
 	}
 
@@ -168,8 +167,8 @@ func TestMultipleEntries(t *testing.T) {
 	defer cron.Stop()
 
 	select {
-	case <-time.After(ONE_SECOND):
-		t.FailNow()
+	case <-time.After(OneSecond):
+		t.Error("expected job run in proper order")
 	case <-wait(wg):
 	}
 }
@@ -188,8 +187,8 @@ func TestRunningJobTwice(t *testing.T) {
 	defer cron.Stop()
 
 	select {
-	case <-time.After(2 * ONE_SECOND):
-		t.FailNow()
+	case <-time.After(2 * OneSecond):
+		t.Error("expected job fires 2 times")
 	case <-wait(wg):
 	}
 }
@@ -210,8 +209,8 @@ func TestRunningMultipleSchedules(t *testing.T) {
 	defer cron.Stop()
 
 	select {
-	case <-time.After(2 * ONE_SECOND):
-		t.FailNow()
+	case <-time.After(2 * OneSecond):
+		t.Error("expected job fires 2 times")
 	case <-wait(wg):
 	}
 }
@@ -221,7 +220,7 @@ func TestLocalTimezone(t *testing.T) {
 	wg := &sync.WaitGroup{}
 	wg.Add(2)
 
-	now := time.Now().Local()
+	now := time.Now()
 	spec := fmt.Sprintf("%d,%d %d %d %d %d ?",
 		now.Second()+1, now.Second()+2, now.Minute(), now.Hour(), now.Day(), now.Month())
 
@@ -231,8 +230,8 @@ func TestLocalTimezone(t *testing.T) {
 	defer cron.Stop()
 
 	select {
-	case <-time.After(ONE_SECOND * 2):
-		t.FailNow()
+	case <-time.After(OneSecond * 2):
+		t.Error("expected job fires 2 times")
 	case <-wait(wg):
 	}
 }
@@ -258,8 +257,8 @@ func TestNonLocalTimezone(t *testing.T) {
 	defer cron.Stop()
 
 	select {
-	case <-time.After(ONE_SECOND * 2):
-		t.FailNow()
+	case <-time.After(OneSecond * 2):
+		t.Error("expected job fires 2 times")
 	case <-wait(wg):
 	}
 }
@@ -280,6 +279,67 @@ func (t testJob) Run() {
 	t.wg.Done()
 }
 
+// Test that adding an invalid job spec returns an error
+func TestInvalidJobSpec(t *testing.T) {
+	cron := New()
+	_, err := cron.AddJob("", "this will not parse", nil)
+	if err == nil {
+		t.Errorf("expected an error with invalid spec, got nil")
+	}
+}
+
+// Test blocking run method behaves as Start()
+func TestBlockingRun(t *testing.T) {
+	wg := &sync.WaitGroup{}
+	wg.Add(1)
+
+	cron := New()
+	cron.AddFunc("", "* * * * * ?", func() { wg.Done() })
+
+	var unblockChan = make(chan struct{})
+
+	go func() {
+		cron.Run()
+		close(unblockChan)
+	}()
+	defer cron.Stop()
+
+	select {
+	case <-time.After(OneSecond):
+		t.Error("expected job fires")
+	case <-unblockChan:
+		t.Error("expected that Run() blocks")
+	case <-wait(wg):
+	}
+}
+
+// Test that double-running is a no-op
+func TestStartNoop(t *testing.T) {
+	var tickChan = make(chan struct{}, 2)
+
+	cron := New()
+	cron.AddFunc("", "* * * * * ?", func() {
+		tickChan <- struct{}{}
+	})
+
+	cron.Start()
+	defer cron.Stop()
+
+	// Wait for the first firing to ensure the runner is going
+	<-tickChan
+
+	cron.Start()
+
+	<-tickChan
+
+	// Fail if this job fires again in a short period, indicating a double-run
+	select {
+	case <-time.After(time.Millisecond):
+	case <-tickChan:
+		t.Error("expected job fires exactly twice")
+	}
+}
+
 // Simple test using Runnables.
 func TestJob(t *testing.T) {
 	wg := &sync.WaitGroup{}
@@ -297,7 +357,7 @@ func TestJob(t *testing.T) {
 	defer cron.Stop()
 
 	select {
-	case <-time.After(ONE_SECOND):
+	case <-time.After(OneSecond):
 		t.FailNow()
 	case <-wait(wg):
 	}
@@ -312,12 +372,31 @@ func TestJob(t *testing.T) {
 
 	for i, expected := range expecteds {
 		if actuals[i] != expected {
-			t.Errorf("Jobs not in the right order.  (expected) %s != %s (actual)", expecteds, actuals)
-			t.FailNow()
+			t.Fatalf("Jobs not in the right order.  (expected) %s != %s (actual)", expecteds, actuals)
 		}
 	}
 }
 
+type ZeroSchedule struct{}
+
+func (*ZeroSchedule) Next(time.Time) time.Time {
+	return time.Time{}
+}
+
+// Tests that job without time does not run
+func TestJobWithZeroTimeDoesNotRun(t *testing.T) {
+	cron := New()
+	calls := 0
+	cron.AddFunc("", "* * * * * *", func() { calls += 1 })
+	cron.Schedule("", "", new(ZeroSchedule), FuncJob(func() { t.Error("expected zero task will not run") }))
+	cron.Start()
+	defer cron.Stop()
+	<-time.After(OneSecond)
+	if calls != 1 {
+		t.Errorf("called %d times, expected 1\n", calls)
+	}
+}
+
 func wait(wg *sync.WaitGroup) chan bool {
 	ch := make(chan bool)
 	go func() {

doc.go

@@ -84,16 +84,16 @@ You may use one of several pre-defined schedules in place of a cron expression.
 
 Intervals
 
-You may also schedule a job to execute at fixed intervals.  This is supported by
-formatting the cron spec like this:
+You may also schedule a job to execute at fixed intervals, starting at the time it's added 
+or cron is run. This is supported by formatting the cron spec like this:
 
     @every <duration>
 
 where "duration" is a string accepted by time.ParseDuration
 (http://golang.org/pkg/time/#ParseDuration).
 
-For example, "@every 1h30m10s" would indicate a schedule that activates every
-1 hour, 30 minutes, 10 seconds.
+For example, "@every 1h30m10s" would indicate a schedule that activates immediately,
+and then every 1 hour, 30 minutes, 10 seconds.
 
 Note: The interval does not take the job runtime into account.  For example,
 if a job takes 3 minutes to run, and it is scheduled to run every 5 minutes,