Skip to content

Commit

Permalink
Update current DST and some code style issues
Browse files Browse the repository at this point in the history
  • Loading branch information
Mahmood Ali committed May 7, 2020
1 parent 702d69f commit 6319db4
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 51 deletions.
7 changes: 5 additions & 2 deletions api/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -648,9 +648,11 @@ func (p *PeriodicConfig) Canonicalize() {
// passed time.
func (p *PeriodicConfig) Next(fromTime time.Time) (time.Time, error) {
if *p.SpecType == PeriodicSpecCron {
if e, err := cronexpr.Parse(*p.Spec); err == nil {
return cronParseNext(e, fromTime, *p.Spec)
e, err := cronexpr.Parse(*p.Spec)
if err != nil {
return time.Time{}, fmt.Errorf("failed parsing cron expression %q: %v", *p.Spec, err)
}
return cronParseNext(e, fromTime, *p.Spec)
}

return time.Time{}, nil
Expand All @@ -670,6 +672,7 @@ func cronParseNext(e *cronexpr.Expression, fromTime time.Time, spec string) (t t

return e.Next(fromTime), nil
}

func (p *PeriodicConfig) GetLocation() (*time.Location, error) {
if p.TimeZone == nil || *p.TimeZone == "" {
return time.UTC, nil
Expand Down
24 changes: 9 additions & 15 deletions nomad/periodic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/hashicorp/nomad/nomad/structs"
"github.com/hashicorp/nomad/testutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

type MockJobEvalDispatcher struct {
Expand Down Expand Up @@ -173,29 +174,22 @@ func TestPeriodicDispatch_Add_UpdateJob(t *testing.T) {
t.Parallel()
p, _ := testPeriodicDispatcher(t)
job := mock.PeriodicJob()
if err := p.Add(job); err != nil {
t.Fatalf("Add failed %v", err)
}
err := p.Add(job)
require.NoError(t, err)

tracked := p.Tracked()
if len(tracked) != 1 {
t.Fatalf("Add didn't track the job: %v", tracked)
}
require.Lenf(t, tracked, 1, "did not track the job")

// Update the job and add it again.
job.Periodic.Spec = "foo"
if err := p.Add(job); err != nil {
t.Fatalf("Add failed %v", err)
}
err = p.Add(job)
require.Error(t, err)
require.Contains(t, err.Error(), "failed parsing cron expression")

tracked = p.Tracked()
if len(tracked) != 1 {
t.Fatalf("Add didn't update: %v", tracked)
}
require.Lenf(t, tracked, 1, "did not update")

if !reflect.DeepEqual(job, tracked[0]) {
t.Fatalf("Add didn't properly update: got %v; want %v", tracked[0], job)
}
require.Equalf(t, job, tracked[0], "add did not properly update")
}

func TestPeriodicDispatch_Add_Remove_Namespaced(t *testing.T) {
Expand Down
6 changes: 4 additions & 2 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -4425,9 +4425,11 @@ func CronParseNext(e *cronexpr.Expression, fromTime time.Time, spec string) (t t
func (p *PeriodicConfig) Next(fromTime time.Time) (time.Time, error) {
switch p.SpecType {
case PeriodicSpecCron:
if e, err := cronexpr.Parse(p.Spec); err == nil {
return CronParseNext(e, fromTime, p.Spec)
e, err := cronexpr.Parse(p.Spec)
if err != nil {
return time.Time{}, fmt.Errorf("failed parsing cron expression: %q: %v", p.Spec, err)
}
return CronParseNext(e, fromTime, p.Spec)
case PeriodicSpecTest:
split := strings.Split(p.Spec, ",")
if len(split) == 1 && split[0] == "" {
Expand Down
61 changes: 29 additions & 32 deletions nomad/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2903,45 +2903,42 @@ func TestPeriodicConfig_ValidCron(t *testing.T) {
}

func TestPeriodicConfig_NextCron(t *testing.T) {
require := require.New(t)

type testExpectation struct {
Time time.Time
HasError bool
ErrorMsg string
}

from := time.Date(2009, time.November, 10, 23, 22, 30, 0, time.UTC)
specs := []string{"0 0 29 2 * 1980",
"*/5 * * * *",
"1 15-0 * * 1-5"}
expected := []*testExpectation{

cases := []struct {
spec string
nextTime time.Time
errorMsg string
}{
{
Time: time.Time{},
HasError: false,
spec: "0 0 29 2 * 1980",
nextTime: time.Time{},
},
{
Time: time.Date(2009, time.November, 10, 23, 25, 0, 0, time.UTC),
HasError: false,
spec: "*/5 * * * *",
nextTime: time.Date(2009, time.November, 10, 23, 25, 0, 0, time.UTC),
},
{
Time: time.Time{},
HasError: true,
ErrorMsg: "failed parsing cron expression",
spec: "1 15-0 *",
nextTime: time.Time{},
errorMsg: "failed parsing cron expression",
},
}

for i, spec := range specs {
p := &PeriodicConfig{Enabled: true, SpecType: PeriodicSpecCron, Spec: spec}
p.Canonicalize()
n, err := p.Next(from)
nextExpected := expected[i]

require.Equal(nextExpected.Time, n)
require.Equal(err != nil, nextExpected.HasError)
if err != nil {
require.True(strings.Contains(err.Error(), nextExpected.ErrorMsg))
}
for i, c := range cases {
t.Run(fmt.Sprintf("case: %d: %s", i, c.spec), func(t *testing.T) {
p := &PeriodicConfig{Enabled: true, SpecType: PeriodicSpecCron, Spec: c.spec}
p.Canonicalize()
n, err := p.Next(from)

require.Equal(t, c.nextTime, n)
if c.errorMsg == "" {
require.NoError(t, err)
} else {
require.Error(t, err)
require.Contains(t, err.Error(), c.errorMsg)
}
})
}
}

Expand All @@ -2963,7 +2960,7 @@ func TestPeriodicConfig_DST(t *testing.T) {
p := &PeriodicConfig{
Enabled: true,
SpecType: PeriodicSpecCron,
Spec: "0 2 11-12 3 * 2017",
Spec: "0 2 11-13 3 * 2017",
TimeZone: "America/Los_Angeles",
}
p.Canonicalize()
Expand All @@ -2973,7 +2970,7 @@ func TestPeriodicConfig_DST(t *testing.T) {

// E1 is an 8 hour adjustment, E2 is a 7 hour adjustment
e1 := time.Date(2017, time.March, 11, 10, 0, 0, 0, time.UTC)
e2 := time.Date(2017, time.March, 12, 9, 0, 0, 0, time.UTC)
e2 := time.Date(2017, time.March, 13, 9, 0, 0, 0, time.UTC)

n1, err := p.Next(t1)
require.Nil(err)
Expand Down

0 comments on commit 6319db4

Please sign in to comment.