Browse Source

Merge branch 'master' into parser-options

Luke Scott 9 years ago
parent
commit
470ac64214
6 changed files with 413 additions and 94 deletions
  1. 52 9
      cron.go
  2. 75 0
      cron_test.go
  3. 102 59
      parser.go
  4. 138 25
      parser_test.go
  5. 1 1
      spec.go
  6. 45 0
      spec_test.go

+ 52 - 9
cron.go

@@ -3,6 +3,8 @@
 package cron
 
 import (
+	"log"
+	"runtime"
 	"sort"
 	"time"
 )
@@ -16,6 +18,8 @@ type Cron struct {
 	add      chan *Entry
 	snapshot chan []*Entry
 	running  bool
+	ErrorLog *log.Logger
+	location *time.Location
 }
 
 // Job is an interface for submitted cron jobs.
@@ -66,14 +70,21 @@ func (s byTime) Less(i, j int) bool {
 	return s[i].Next.Before(s[j].Next)
 }
 
-// New returns a new Cron job runner.
+// New returns a new Cron job runner, in the Local time zone.
 func New() *Cron {
+	return NewWithLocation(time.Now().Location())
+}
+
+// NewWithLocation returns a new Cron job runner.
+func NewWithLocation(location *time.Location) *Cron {
 	return &Cron{
 		entries:  nil,
 		add:      make(chan *Entry),
 		stop:     make(chan struct{}),
 		snapshot: make(chan []*Entry),
 		running:  false,
+		ErrorLog: nil,
+		location: location,
 	}
 }
 
@@ -87,7 +98,7 @@ func (c *Cron) AddFunc(spec string, cmd func()) error {
 	return c.AddJob(spec, FuncJob(cmd))
 }
 
-// AddFunc adds a Job to the Cron to be run on the given schedule.
+// AddJob adds a Job to the Cron to be run on the given schedule.
 func (c *Cron) AddJob(spec string, cmd Job) error {
 	schedule, err := Parse(spec)
 	if err != nil {
@@ -121,17 +132,37 @@ func (c *Cron) Entries() []*Entry {
 	return c.entrySnapshot()
 }
 
-// Start the cron scheduler in its own go-routine.
+// Location gets the time zone location
+func (c *Cron) Location() *time.Location {
+	return c.location
+}
+
+// Start the cron scheduler in its own go-routine, or no-op if already started.
 func (c *Cron) Start() {
+	if c.running {
+		return
+	}
 	c.running = true
 	go c.run()
 }
 
+func (c *Cron) runWithRecovery(j Job) {
+	defer func() {
+		if r := recover(); r != nil {
+			const size = 64 << 10
+			buf := make([]byte, size)
+			buf = buf[:runtime.Stack(buf, false)]
+			c.logf("cron: panic running job: %v\n%s", r, buf)
+		}
+	}()
+	j.Run()
+}
+
 // 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().Local()
+	now := time.Now().In(c.location)
 	for _, entry := range c.entries {
 		entry.Next = entry.Schedule.Next(now)
 	}
@@ -149,32 +180,44 @@ func (c *Cron) run() {
 			effective = c.entries[0].Next
 		}
 
+		timer := time.NewTimer(effective.Sub(now))
 		select {
-		case now = <-time.After(effective.Sub(now)):
+		case now = <-timer.C:
 			// Run every entry whose next time was this effective time.
 			for _, e := range c.entries {
 				if e.Next != effective {
 					break
 				}
-				go e.Job.Run()
+				go c.runWithRecovery(e.Job)
 				e.Prev = e.Next
-				e.Next = e.Schedule.Next(effective)
+				e.Next = e.Schedule.Next(now)
 			}
 			continue
 
 		case newEntry := <-c.add:
 			c.entries = append(c.entries, newEntry)
-			newEntry.Next = newEntry.Schedule.Next(now)
+			newEntry.Next = newEntry.Schedule.Next(time.Now().In(c.location))
 
 		case <-c.snapshot:
 			c.snapshot <- c.entrySnapshot()
 
 		case <-c.stop:
+			timer.Stop()
 			return
 		}
 
 		// 'now' should be updated after newEntry and snapshot cases.
-		now = time.Now().Local()
+		now = time.Now().In(c.location)
+		timer.Stop()
+	}
+}
+
+// Logs an error to stderr or to the configured error log
+func (c *Cron) logf(format string, args ...interface{}) {
+	if c.ErrorLog != nil {
+		c.ErrorLog.Printf(format, args...)
+	} else {
+		log.Printf(format, args...)
 	}
 }
 

+ 75 - 0
cron_test.go

@@ -12,6 +12,38 @@ import (
 // compensate for a few milliseconds of runtime.
 const ONE_SECOND = 1*time.Second + 10*time.Millisecond
 
+func TestFuncPanicRecovery(t *testing.T) {
+	cron := New()
+	cron.Start()
+	defer cron.Stop()
+	cron.AddFunc("* * * * * ?", func() { panic("YOLO") })
+
+	select {
+	case <-time.After(ONE_SECOND):
+		return
+	}
+}
+
+type DummyJob struct{}
+
+func (d DummyJob) Run() {
+	panic("YOLO")
+}
+
+func TestJobPanicRecovery(t *testing.T) {
+	var job DummyJob
+
+	cron := New()
+	cron.Start()
+	defer cron.Stop()
+	cron.AddJob("* * * * * ?", job)
+
+	select {
+	case <-time.After(ONE_SECOND):
+		return
+	}
+}
+
 // Start and stop cron with no entries.
 func TestNoEntries(t *testing.T) {
 	cron := New()
@@ -77,6 +109,22 @@ func TestAddWhileRunning(t *testing.T) {
 	}
 }
 
+// Test for #34. Adding a job after calling start results in multiple job invocations
+func TestAddWhileRunningWithDelay(t *testing.T) {
+	cron := New()
+	cron.Start()
+	defer cron.Stop()
+	time.Sleep(5 * time.Second)
+	var calls = 0
+	cron.AddFunc("* * * * * *", func() { calls += 1 })
+
+	<-time.After(ONE_SECOND)
+	if calls != 1 {
+		fmt.Printf("called %d times, expected 1\n", calls)
+		t.Fail()
+	}
+}
+
 // Test timing with Entries.
 func TestSnapshotEntries(t *testing.T) {
 	wg := &sync.WaitGroup{}
@@ -189,6 +237,33 @@ func TestLocalTimezone(t *testing.T) {
 	}
 }
 
+// Test that the cron is run in the given time zone (as opposed to local).
+func TestNonLocalTimezone(t *testing.T) {
+	wg := &sync.WaitGroup{}
+	wg.Add(1)
+
+	loc, err := time.LoadLocation("Atlantic/Cape_Verde")
+	if err != nil {
+		fmt.Printf("Failed to load time zone Atlantic/Cape_Verde: %+v", err)
+		t.Fail()
+	}
+
+	now := time.Now().In(loc)
+	spec := fmt.Sprintf("%d %d %d %d %d ?",
+		now.Second()+1, now.Minute(), now.Hour(), now.Day(), now.Month())
+
+	cron := NewWithLocation(loc)
+	cron.AddFunc(spec, func() { wg.Done() })
+	cron.Start()
+	defer cron.Stop()
+
+	select {
+	case <-time.After(ONE_SECOND):
+		t.FailNow()
+	case <-wait(wg):
+	}
+}
+
 // Test that calling stop before start silently returns without
 // blocking the stop channel.
 func TestStopWithoutStart(t *testing.T) {

+ 102 - 59
parser.go

@@ -2,7 +2,6 @@ package cron
 
 import (
 	"fmt"
-	"log"
 	"math"
 	"strconv"
 	"strings"
@@ -54,16 +53,9 @@ func NewParser(options ParseOption) Parser {
 	return Parser{options, optionals}
 }
 
-func (p Parser) Parse(spec string) (_ Schedule, err error) {
-	// Convert panics into errors
-	defer func() {
-		if recovered := recover(); recovered != nil {
-			err = fmt.Errorf("%v", recovered)
-		}
-	}()
-
+func (p Parser) Parse(spec string) (Schedule, error) {
 	if spec[0] == '@' && p.options&Descriptor > 0 {
-		return parseDescriptor(spec), nil
+		return parseDescriptor(spec)
 	}
 
 	// Figure out how many fields we need
@@ -81,25 +73,45 @@ func (p Parser) Parse(spec string) (_ Schedule, err error) {
 	// Validate number of fields
 	if count := len(fields); count < min || count > max {
 		if min == max {
-			log.Panicf("Expected %d fields, found %d: %s", min, count, spec)
+			return nil, fmt.Errorf("Expected exactly %d fields, found %d: %s", min, count, spec)
 		} else {
-			log.Panicf("Expected %d to %d fields, found %d: %s", min, max, count, spec)
+			return nil, fmt.Errorf("Expected %d to %d fields, found %d: %s", min, max, count, spec)
 		}
 	}
 
 	// Fill in missing fields
 	fields = expandFields(fields, p.options)
 
-	schedule := &SpecSchedule{
-		Second: getField(fields[0], seconds),
-		Minute: getField(fields[1], minutes),
-		Hour:   getField(fields[2], hours),
-		Dom:    getField(fields[3], dom),
-		Month:  getField(fields[4], months),
-		Dow:    getField(fields[5], dow),
+	var err error
+	field := func(field string, r bounds) uint64 {
+		if err != nil {
+			return 0
+		}
+		var bits uint64
+		bits, err = getField(field, r)
+		return bits
 	}
 
-	return schedule, nil
+	var (
+		second     = field(fields[0], seconds)
+		minute     = field(fields[1], minutes)
+		hour       = field(fields[2], hours)
+		dayofmonth = field(fields[3], dom)
+		month      = field(fields[4], months)
+		dayofweek  = field(fields[5], dow)
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	return &SpecSchedule{
+		Second: second,
+		Minute: minute,
+		Hour:   hour,
+		Dom:    dayofmonth,
+		Month:  month,
+		Dow:    dayofweek,
+	}, nil
 }
 
 func expandFields(fields []string, options ParseOption) []string {
@@ -119,6 +131,22 @@ func expandFields(fields []string, options ParseOption) []string {
 	return expFields
 }
 
+var standardParser = NewParser(
+	Minute | Hour | Dom | Month | Dow | Descriptor,
+)
+
+// ParseStandard returns a new crontab schedule representing the given standardSpec
+// (https://en.wikipedia.org/wiki/Cron). It differs from Parse requiring to always
+// pass 5 entries representing: minute, hour, day of month, month and day of week,
+// in that order. It returns a descriptive error if the spec is not valid.
+//
+// It accepts
+//   - Standard crontab specs, e.g. "* * * * ?"
+//   - Descriptors, e.g. "@midnight", "@every 1h30m"
+func ParseStandard(standardSpec string) (Schedule, error) {
+	return standardParser.Parse(standardSpec)
+}
+
 var defaultParser = NewParser(
 	Second | Minute | Hour | Dom | Month | DowOptional | Descriptor,
 )
@@ -129,31 +157,36 @@ var defaultParser = NewParser(
 // It accepts
 //   - Full crontab specs, e.g. "* * * * * ?"
 //   - Descriptors, e.g. "@midnight", "@every 1h30m"
-func Parse(spec string) (_ Schedule, err error) {
+func Parse(spec string) (Schedule, error) {
 	return defaultParser.Parse(spec)
 }
 
 // getField returns an Int with the bits set representing all of the times that
-// the field represents.  A "field" is a comma-separated list of "ranges".
-func getField(field string, r bounds) uint64 {
-	// list = range {"," range}
+// the field represents or error parsing field value.  A "field" is a comma-separated
+// list of "ranges".
+func getField(field string, r bounds) (uint64, error) {
 	var bits uint64
 	ranges := strings.FieldsFunc(field, func(r rune) bool { return r == ',' })
 	for _, expr := range ranges {
-		bits |= getRange(expr, r)
+		bit, err := getRange(expr, r)
+		if err != nil {
+			return bits, err
+		}
+		bits |= bit
 	}
-	return bits
+	return bits, nil
 }
 
 // getRange returns the bits indicated by the given expression:
 //   number | number "-" number [ "/" number ]
-func getRange(expr string, r bounds) uint64 {
-
+// or error parsing range.
+func getRange(expr string, r bounds) (uint64, error) {
 	var (
 		start, end, step uint
 		rangeAndStep     = strings.Split(expr, "/")
 		lowAndHigh       = strings.Split(rangeAndStep[0], "-")
 		singleDigit      = len(lowAndHigh) == 1
+		err              error
 	)
 
 	var extra uint64
@@ -162,14 +195,20 @@ func getRange(expr string, r bounds) uint64 {
 		end = r.max
 		extra = starBit
 	} else {
-		start = parseIntOrName(lowAndHigh[0], r.names)
+		start, err = parseIntOrName(lowAndHigh[0], r.names)
+		if err != nil {
+			return 0, err
+		}
 		switch len(lowAndHigh) {
 		case 1:
 			end = start
 		case 2:
-			end = parseIntOrName(lowAndHigh[1], r.names)
+			end, err = parseIntOrName(lowAndHigh[1], r.names)
+			if err != nil {
+				return 0, err
+			}
 		default:
-			log.Panicf("Too many hyphens: %s", expr)
+			return 0, fmt.Errorf("Too many hyphens: %s", expr)
 		}
 	}
 
@@ -177,50 +216,56 @@ func getRange(expr string, r bounds) uint64 {
 	case 1:
 		step = 1
 	case 2:
-		step = mustParseInt(rangeAndStep[1])
+		step, err = mustParseInt(rangeAndStep[1])
+		if err != nil {
+			return 0, err
+		}
 
 		// Special handling: "N/step" means "N-max/step".
 		if singleDigit {
 			end = r.max
 		}
 	default:
-		log.Panicf("Too many slashes: %s", expr)
+		return 0, fmt.Errorf("Too many slashes: %s", expr)
 	}
 
 	if start < r.min {
-		log.Panicf("Beginning of range (%d) below minimum (%d): %s", start, r.min, expr)
+		return 0, fmt.Errorf("Beginning of range (%d) below minimum (%d): %s", start, r.min, expr)
 	}
 	if end > r.max {
-		log.Panicf("End of range (%d) above maximum (%d): %s", end, r.max, expr)
+		return 0, fmt.Errorf("End of range (%d) above maximum (%d): %s", end, r.max, expr)
 	}
 	if start > end {
-		log.Panicf("Beginning of range (%d) beyond end of range (%d): %s", start, end, expr)
+		return 0, fmt.Errorf("Beginning of range (%d) beyond end of range (%d): %s", start, end, expr)
+	}
+	if step == 0 {
+		return 0, fmt.Errorf("Step of range should be a positive number: %s", expr)
 	}
 
-	return getBits(start, end, step) | extra
+	return getBits(start, end, step) | extra, nil
 }
 
 // parseIntOrName returns the (possibly-named) integer contained in expr.
-func parseIntOrName(expr string, names map[string]uint) uint {
+func parseIntOrName(expr string, names map[string]uint) (uint, error) {
 	if names != nil {
 		if namedInt, ok := names[strings.ToLower(expr)]; ok {
-			return namedInt
+			return namedInt, nil
 		}
 	}
 	return mustParseInt(expr)
 }
 
-// mustParseInt parses the given expression as an int or panics.
-func mustParseInt(expr string) uint {
+// mustParseInt parses the given expression as an int or returns an error.
+func mustParseInt(expr string) (uint, error) {
 	num, err := strconv.Atoi(expr)
 	if err != nil {
-		log.Panicf("Failed to parse int from %s: %s", expr, err)
+		return 0, fmt.Errorf("Failed to parse int from %s: %s", expr, err)
 	}
 	if num < 0 {
-		log.Panicf("Negative number (%d) not allowed: %s", num, expr)
+		return 0, fmt.Errorf("Negative number (%d) not allowed: %s", num, expr)
 	}
 
-	return uint(num)
+	return uint(num), nil
 }
 
 // getBits sets all bits in the range [min, max], modulo the given step size.
@@ -244,10 +289,9 @@ func all(r bounds) uint64 {
 	return getBits(r.min, r.max, 1) | starBit
 }
 
-// parseDescriptor returns a pre-defined schedule for the expression, or panics
-// if none matches.
-func parseDescriptor(spec string) Schedule {
-	switch spec {
+// parseDescriptor returns a predefined schedule for the expression, or error if none matches.
+func parseDescriptor(descriptor string) (Schedule, error) {
+	switch descriptor {
 	case "@yearly", "@annually":
 		return &SpecSchedule{
 			Second: 1 << seconds.min,
@@ -256,7 +300,7 @@ func parseDescriptor(spec string) Schedule {
 			Dom:    1 << dom.min,
 			Month:  1 << months.min,
 			Dow:    all(dow),
-		}
+		}, nil
 
 	case "@monthly":
 		return &SpecSchedule{
@@ -266,7 +310,7 @@ func parseDescriptor(spec string) Schedule {
 			Dom:    1 << dom.min,
 			Month:  all(months),
 			Dow:    all(dow),
-		}
+		}, nil
 
 	case "@weekly":
 		return &SpecSchedule{
@@ -276,7 +320,7 @@ func parseDescriptor(spec string) Schedule {
 			Dom:    all(dom),
 			Month:  all(months),
 			Dow:    1 << dow.min,
-		}
+		}, nil
 
 	case "@daily", "@midnight":
 		return &SpecSchedule{
@@ -286,7 +330,7 @@ func parseDescriptor(spec string) Schedule {
 			Dom:    all(dom),
 			Month:  all(months),
 			Dow:    all(dow),
-		}
+		}, nil
 
 	case "@hourly":
 		return &SpecSchedule{
@@ -296,18 +340,17 @@ func parseDescriptor(spec string) Schedule {
 			Dom:    all(dom),
 			Month:  all(months),
 			Dow:    all(dow),
-		}
+		}, nil
 	}
 
 	const every = "@every "
-	if strings.HasPrefix(spec, every) {
-		duration, err := time.ParseDuration(spec[len(every):])
+	if strings.HasPrefix(descriptor, every) {
+		duration, err := time.ParseDuration(descriptor[len(every):])
 		if err != nil {
-			log.Panicf("Failed to parse duration %s: %s", spec, err)
+			return nil, fmt.Errorf("Failed to parse duration %s: %s", descriptor, err)
 		}
-		return Every(duration)
+		return Every(duration), nil
 	}
 
-	log.Panicf("Unrecognized descriptor: %s", spec)
-	return nil
+	return nil, fmt.Errorf("Unrecognized descriptor: %s", descriptor)
 }

+ 138 - 25
parser_test.go

@@ -2,36 +2,55 @@ package cron
 
 import (
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 )
 
 func TestRange(t *testing.T) {
+	zero := uint64(0)
 	ranges := []struct {
 		expr     string
 		min, max uint
 		expected uint64
+		err      string
 	}{
-		{"5", 0, 7, 1 << 5},
-		{"0", 0, 7, 1 << 0},
-		{"7", 0, 7, 1 << 7},
+		{"5", 0, 7, 1 << 5, ""},
+		{"0", 0, 7, 1 << 0, ""},
+		{"7", 0, 7, 1 << 7, ""},
 
-		{"5-5", 0, 7, 1 << 5},
-		{"5-6", 0, 7, 1<<5 | 1<<6},
-		{"5-7", 0, 7, 1<<5 | 1<<6 | 1<<7},
+		{"5-5", 0, 7, 1 << 5, ""},
+		{"5-6", 0, 7, 1<<5 | 1<<6, ""},
+		{"5-7", 0, 7, 1<<5 | 1<<6 | 1<<7, ""},
 
-		{"5-6/2", 0, 7, 1 << 5},
-		{"5-7/2", 0, 7, 1<<5 | 1<<7},
-		{"5-7/1", 0, 7, 1<<5 | 1<<6 | 1<<7},
+		{"5-6/2", 0, 7, 1 << 5, ""},
+		{"5-7/2", 0, 7, 1<<5 | 1<<7, ""},
+		{"5-7/1", 0, 7, 1<<5 | 1<<6 | 1<<7, ""},
 
-		{"*", 1, 3, 1<<1 | 1<<2 | 1<<3 | starBit},
-		{"*/2", 1, 3, 1<<1 | 1<<3 | starBit},
+		{"*", 1, 3, 1<<1 | 1<<2 | 1<<3 | starBit, ""},
+		{"*/2", 1, 3, 1<<1 | 1<<3 | starBit, ""},
+
+		{"5--5", 0, 0, zero, "Too many hyphens"},
+		{"jan-x", 0, 0, zero, "Failed to parse int from"},
+		{"2-x", 1, 5, zero, "Failed to parse int from"},
+		{"*/-12", 0, 0, zero, "Negative number"},
+		{"*//2", 0, 0, zero, "Too many slashes"},
+		{"1", 3, 5, zero, "below minimum"},
+		{"6", 3, 5, zero, "above maximum"},
+		{"5-3", 3, 5, zero, "beyond end of range"},
+		{"*/0", 0, 0, zero, "should be a positive number"},
 	}
 
 	for _, c := range ranges {
-		actual := getRange(c.expr, bounds{c.min, c.max, nil})
+		actual, err := getRange(c.expr, bounds{c.min, c.max, nil})
+		if len(c.err) != 0 && (err == nil || !strings.Contains(err.Error(), c.err)) {
+			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)
+		}
 		if actual != c.expected {
-			t.Errorf("%s => (expected) %d != %d (actual)", c.expr, c.expected, actual)
+			t.Errorf("%s => expected %d, got %d", c.expr, c.expected, actual)
 		}
 	}
 }
@@ -49,14 +68,14 @@ func TestField(t *testing.T) {
 	}
 
 	for _, c := range fields {
-		actual := getField(c.expr, bounds{c.min, c.max, nil})
+		actual, _ := getField(c.expr, bounds{c.min, c.max, nil})
 		if actual != c.expected {
-			t.Errorf("%s => (expected) %d != %d (actual)", c.expr, c.expected, actual)
+			t.Errorf("%s => expected %d, got %d", c.expr, c.expected, actual)
 		}
 	}
 }
 
-func TestBits(t *testing.T) {
+func TestAll(t *testing.T) {
 	allBits := []struct {
 		r        bounds
 		expected uint64
@@ -71,16 +90,17 @@ func TestBits(t *testing.T) {
 	for _, c := range allBits {
 		actual := all(c.r) // all() adds the starBit, so compensate for that..
 		if c.expected|starBit != actual {
-			t.Errorf("%d-%d/%d => (expected) %b != %b (actual)",
+			t.Errorf("%d-%d/%d => expected %b, got %b",
 				c.r.min, c.r.max, 1, c.expected|starBit, actual)
 		}
 	}
+}
 
+func TestBits(t *testing.T) {
 	bits := []struct {
 		min, max, step uint
 		expected       uint64
 	}{
-
 		{0, 0, 1, 0x1},
 		{1, 1, 1, 0x2},
 		{1, 5, 2, 0x2a}, // 101010
@@ -90,28 +110,121 @@ func TestBits(t *testing.T) {
 	for _, c := range bits {
 		actual := getBits(c.min, c.max, c.step)
 		if c.expected != actual {
-			t.Errorf("%d-%d/%d => (expected) %b != %b (actual)",
+			t.Errorf("%d-%d/%d => expected %b, got %b",
 				c.min, c.max, c.step, c.expected, actual)
 		}
 	}
 }
 
-func TestSpecSchedule(t *testing.T) {
+func TestParse(t *testing.T) {
 	entries := []struct {
 		expr     string
 		expected Schedule
+		err      string
 	}{
-		{"* 5 * * * *", &SpecSchedule{all(seconds), 1 << 5, all(hours), all(dom), all(months), all(dow)}},
-		{"@every 5m", ConstantDelaySchedule{time.Duration(5) * time.Minute}},
+		{
+			expr: "* 5 * * * *",
+			expected: &SpecSchedule{
+				Second: all(seconds),
+				Minute: 1 << 5,
+				Hour:   all(hours),
+				Dom:    all(dom),
+				Month:  all(months),
+				Dow:    all(dow),
+			},
+		},
+		{
+			expr: "* 5 j * * *",
+			err:  "Failed to parse int from",
+		},
+		{
+			expr:     "@every 5m",
+			expected: ConstantDelaySchedule{Delay: time.Duration(5) * time.Minute},
+		},
+		{
+			expr: "@every Xm",
+			err:  "Failed to parse duration",
+		},
+		{
+			expr: "@yearly",
+			expected: &SpecSchedule{
+				Second: 1 << seconds.min,
+				Minute: 1 << minutes.min,
+				Hour:   1 << hours.min,
+				Dom:    1 << dom.min,
+				Month:  1 << months.min,
+				Dow:    all(dow),
+			},
+		},
+		{
+			expr: "@annually",
+			expected: &SpecSchedule{
+				Second: 1 << seconds.min,
+				Minute: 1 << minutes.min,
+				Hour:   1 << hours.min,
+				Dom:    1 << dom.min,
+				Month:  1 << months.min,
+				Dow:    all(dow),
+			},
+		},
+		{
+			expr: "@unrecognized",
+			err:  "Unrecognized descriptor",
+		},
+		{
+			expr: "* * * *",
+			err:  "Expected 5 to 6 fields",
+		},
 	}
 
 	for _, c := range entries {
 		actual, err := Parse(c.expr)
-		if err != nil {
-			t.Error(err)
+		if len(c.err) != 0 && (err == nil || !strings.Contains(err.Error(), c.err)) {
+			t.Errorf("%s => expected %v, got %v", c.expr, c.err, err)
+		}
+		if len(c.err) == 0 && err != nil {
+			t.Errorf("%s => unexpected error %v", c.expr, err)
+		}
+		if !reflect.DeepEqual(actual, c.expected) {
+			t.Errorf("%s => expected %b, got %b", c.expr, c.expected, actual)
+		}
+	}
+}
+
+func TestStandardSpecSchedule(t *testing.T) {
+	entries := []struct {
+		expr     string
+		expected Schedule
+		err      string
+	}{
+		{
+			expr:     "5 * * * *",
+			expected: &SpecSchedule{1 << seconds.min, 1 << 5, all(hours), all(dom), all(months), all(dow)},
+		},
+		{
+			expr:     "@every 5m",
+			expected: ConstantDelaySchedule{time.Duration(5) * time.Minute},
+		},
+		{
+			expr: "5 j * * *",
+			err:  "Failed to parse int from",
+		},
+		{
+			expr: "* * * *",
+			err:  "Expected exactly 5 fields",
+		},
+	}
+
+	for _, c := range entries {
+		actual, err := ParseStandard(c.expr)
+		if len(c.err) != 0 && (err == nil || !strings.Contains(err.Error(), c.err)) {
+			t.Errorf("%s => expected %v, got %v", c.expr, c.err, err)
+		}
+		if len(c.err) == 0 && err != nil {
+			t.Errorf("%s => unexpected error %v", c.expr, err)
 		}
 		if !reflect.DeepEqual(actual, c.expected) {
-			t.Errorf("%s => (expected) %b != %b (actual)", c.expr, c.expected, actual)
+			t.Errorf("%s => expected %b, got %b", c.expr, c.expected, actual)
 		}
 	}
 }

+ 1 - 1
spec.go

@@ -108,7 +108,7 @@ WRAP:
 	for 1<<uint(t.Hour())&s.Hour == 0 {
 		if !added {
 			added = true
-			t = t.Truncate(time.Hour)
+			t = time.Date(t.Year(), t.Month(), t.Day(), t.Hour(), 0, 0, 0, t.Location())
 		}
 		t = t.Add(1 * time.Hour)
 

+ 45 - 0
spec_test.go

@@ -202,3 +202,48 @@ func getTime(value string) time.Time {
 
 	return t
 }
+
+func TestNextWithTz(t *testing.T) {
+	runs := []struct {
+		time, spec string
+		expected   string
+	}{
+		// Failing tests
+		{"2016-01-03T13:09:03+0530", "0 14 14 * * *", "2016-01-03T14:14:00+0530"},
+		{"2016-01-03T04:09:03+0530", "0 14 14 * * ?", "2016-01-03T14:14:00+0530"},
+
+		// Passing tests
+		{"2016-01-03T14:09:03+0530", "0 14 14 * * *", "2016-01-03T14:14:00+0530"},
+		{"2016-01-03T14:00:00+0530", "0 14 14 * * ?", "2016-01-03T14:14:00+0530"},
+	}
+	for _, c := range runs {
+		sched, err := Parse(c.spec)
+		if err != nil {
+			t.Error(err)
+			continue
+		}
+		actual := sched.Next(getTimeTZ(c.time))
+		expected := getTimeTZ(c.expected)
+		if !actual.Equal(expected) {
+			t.Errorf("%s, \"%s\": (expected) %v != %v (actual)", c.time, c.spec, expected, actual)
+		}
+	}
+}
+
+func getTimeTZ(value string) time.Time {
+	if value == "" {
+		return time.Time{}
+	}
+	t, err := time.Parse("Mon Jan 2 15:04 2006", value)
+	if err != nil {
+		t, err = time.Parse("Mon Jan 2 15:04:05 2006", value)
+		if err != nil {
+			t, err = time.Parse("2006-01-02T15:04:05-0700", value)
+			if err != nil {
+				panic(err)
+			}
+		}
+	}
+
+	return t
+}