Merge pull request robfig#103 from bgaifullin/master

Some enhancements

Author: robfig

cron.go

@@ -165,11 +165,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)
 	}
@@ -178,45 +178,46 @@ 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.Prev = e.Next
+					e.Next = e.Schedule.Next(now)
 				}
-				go c.runWithRecovery(e.Job)
-				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
+		}
 	}
 }
 
@@ -251,3 +252,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):
 	}
 }
@@ -297,7 +296,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 +311,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() {

parser_test.go

@@ -47,7 +47,7 @@ func TestRange(t *testing.T) {
 			t.Errorf("%s => expected %v, got %v", c.expr, c.err, err)
 		}
 		if len(c.err) == 0 && err != nil {
-			t.Error("%s => unexpected error %v", c.expr, err)
+			t.Errorf("%s => unexpected error %v", c.expr, err)
 		}
 		if actual != c.expected {
 			t.Errorf("%s => expected %d, got %d", c.expr, c.expected, actual)