From 43d13a3edce4f3fc2f6924820c3550f95b4e377a Mon Sep 17 00:00:00 2001 From: Steven Date: Fri, 11 Oct 2024 21:05:07 +0800 Subject: [PATCH] chore: tweak linter --- internal/cron/chain.go | 3 ++- internal/cron/chain_test.go | 26 +++++++++------------- internal/cron/cron.go | 24 ++++++++++---------- internal/cron/cron_test.go | 12 +++++----- internal/cron/parser.go | 38 ++++++++++++++++---------------- internal/cron/parser_test.go | 4 ++-- plugin/idp/oauth2/oauth2_test.go | 2 +- test/util/resource_name_test.go | 2 +- 8 files changed, 54 insertions(+), 57 deletions(-) diff --git a/internal/cron/chain.go b/internal/cron/chain.go index 873b9078..82b387a0 100644 --- a/internal/cron/chain.go +++ b/internal/cron/chain.go @@ -1,6 +1,7 @@ package cron import ( + "errors" "fmt" "runtime" "sync" @@ -48,7 +49,7 @@ func Recover(logger Logger) JobWrapper { buf = buf[:runtime.Stack(buf, false)] err, ok := r.(error) if !ok { - err = fmt.Errorf("%v", r) + err = errors.New("panic: " + fmt.Sprint(r)) } logger.Error(err, "panic", "stack", "...\n"+string(buf)) } diff --git a/internal/cron/chain_test.go b/internal/cron/chain_test.go index a0433b31..3c05c157 100644 --- a/internal/cron/chain_test.go +++ b/internal/cron/chain_test.go @@ -46,7 +46,7 @@ func TestChainRecover(t *testing.T) { panic("panickingJob panics") }) - t.Run("panic exits job by default", func(t *testing.T) { + t.Run("panic exits job by default", func(*testing.T) { defer func() { if err := recover(); err == nil { t.Errorf("panic expected, but none received") @@ -56,13 +56,13 @@ func TestChainRecover(t *testing.T) { Run() }) - t.Run("Recovering JobWrapper recovers", func(t *testing.T) { + t.Run("Recovering JobWrapper recovers", func(*testing.T) { NewChain(Recover(PrintfLogger(log.New(io.Discard, "", 0)))). Then(panickingJob). Run() }) - t.Run("composed with the *IfStillRunning wrappers", func(t *testing.T) { + t.Run("composed with the *IfStillRunning wrappers", func(*testing.T) { NewChain(Recover(PrintfLogger(log.New(io.Discard, "", 0)))). Then(panickingJob). Run() @@ -99,8 +99,7 @@ func (j *countJob) Done() int { } func TestChainDelayIfStillRunning(t *testing.T) { - - t.Run("runs immediately", func(t *testing.T) { + t.Run("runs immediately", func(*testing.T) { var j countJob wrappedJob := NewChain(DelayIfStillRunning(DiscardLogger)).Then(&j) go wrappedJob.Run() @@ -110,7 +109,7 @@ func TestChainDelayIfStillRunning(t *testing.T) { } }) - t.Run("second run immediate if first done", func(t *testing.T) { + t.Run("second run immediate if first done", func(*testing.T) { var j countJob wrappedJob := NewChain(DelayIfStillRunning(DiscardLogger)).Then(&j) go func() { @@ -124,7 +123,7 @@ func TestChainDelayIfStillRunning(t *testing.T) { } }) - t.Run("second run delayed if first not done", func(t *testing.T) { + t.Run("second run delayed if first not done", func(*testing.T) { var j countJob j.delay = 10 * time.Millisecond wrappedJob := NewChain(DelayIfStillRunning(DiscardLogger)).Then(&j) @@ -149,12 +148,10 @@ func TestChainDelayIfStillRunning(t *testing.T) { t.Error("expected both jobs done, got", started, done) } }) - } func TestChainSkipIfStillRunning(t *testing.T) { - - t.Run("runs immediately", func(t *testing.T) { + t.Run("runs immediately", func(*testing.T) { var j countJob wrappedJob := NewChain(SkipIfStillRunning(DiscardLogger)).Then(&j) go wrappedJob.Run() @@ -164,7 +161,7 @@ func TestChainSkipIfStillRunning(t *testing.T) { } }) - t.Run("second run immediate if first done", func(t *testing.T) { + t.Run("second run immediate if first done", func(*testing.T) { var j countJob wrappedJob := NewChain(SkipIfStillRunning(DiscardLogger)).Then(&j) go func() { @@ -178,7 +175,7 @@ func TestChainSkipIfStillRunning(t *testing.T) { } }) - t.Run("second run skipped if first not done", func(t *testing.T) { + t.Run("second run skipped if first not done", func(*testing.T) { var j countJob j.delay = 10 * time.Millisecond wrappedJob := NewChain(SkipIfStillRunning(DiscardLogger)).Then(&j) @@ -204,7 +201,7 @@ func TestChainSkipIfStillRunning(t *testing.T) { } }) - t.Run("skip 10 jobs on rapid fire", func(t *testing.T) { + t.Run("skip 10 jobs on rapid fire", func(*testing.T) { var j countJob j.delay = 10 * time.Millisecond wrappedJob := NewChain(SkipIfStillRunning(DiscardLogger)).Then(&j) @@ -218,7 +215,7 @@ func TestChainSkipIfStillRunning(t *testing.T) { } }) - t.Run("different jobs independent", func(t *testing.T) { + t.Run("different jobs independent", func(*testing.T) { var j1, j2 countJob j1.delay = 10 * time.Millisecond j2.delay = 10 * time.Millisecond @@ -238,5 +235,4 @@ func TestChainSkipIfStillRunning(t *testing.T) { t.Error("expected both jobs executed once, got", done1, "and", done2) } }) - } diff --git a/internal/cron/cron.go b/internal/cron/cron.go index c7e91766..6055da7a 100644 --- a/internal/cron/cron.go +++ b/internal/cron/cron.go @@ -26,7 +26,7 @@ type Cron struct { jobWaiter sync.WaitGroup } -// ScheduleParser is an interface for schedule spec parsers that return a Schedule +// ScheduleParser is an interface for schedule spec parsers that return a Schedule. type ScheduleParser interface { Parse(spec string) (Schedule, error) } @@ -43,7 +43,7 @@ type Schedule interface { Next(time.Time) time.Time } -// EntryID identifies an entry within a Cron instance +// EntryID identifies an entry within a Cron instance. type EntryID int // Entry consists of a schedule and the func to execute on that schedule. @@ -97,17 +97,17 @@ func (s byTime) Less(i, j int) bool { // // Available Settings // -// Time Zone -// Description: The time zone in which schedules are interpreted -// Default: time.Local +// Time Zone +// Description: The time zone in which schedules are interpreted +// Default: time.Local // -// Parser -// Description: Parser converts cron spec strings into cron.Schedules. -// Default: Accepts this spec: https://en.wikipedia.org/wiki/Cron +// Parser +// Description: Parser converts cron spec strings into cron.Schedules. +// Default: Accepts this spec: https://en.wikipedia.org/wiki/Cron // -// Chain -// Description: Wrap submitted jobs to customize behavior. -// Default: A chain that recovers panics and logs them to stderr. +// Chain +// Description: Wrap submitted jobs to customize behavior. +// Default: A chain that recovers panics and logs them to stderr. // // See "cron.With*" to modify the default behavior. func New(opts ...Option) *Cron { @@ -130,7 +130,7 @@ func New(opts ...Option) *Cron { return c } -// FuncJob is a wrapper that turns a func() into a cron.Job +// FuncJob is a wrapper that turns a func() into a cron.Job. type FuncJob func() func (f FuncJob) Run() { f() } diff --git a/internal/cron/cron_test.go b/internal/cron/cron_test.go index 36f06bf7..a5334773 100644 --- a/internal/cron/cron_test.go +++ b/internal/cron/cron_test.go @@ -59,7 +59,7 @@ func TestFuncPanicRecovery(t *testing.T) { type DummyJob struct{} -func (d DummyJob) Run() { +func (DummyJob) Run() { panic("YOLO") } @@ -556,7 +556,7 @@ func TestJobWithZeroTimeDoesNotRun(t *testing.T) { } func TestStopAndWait(t *testing.T) { - t.Run("nothing running, returns immediately", func(t *testing.T) { + t.Run("nothing running, returns immediately", func(*testing.T) { cron := newWithSeconds() cron.Start() ctx := cron.Stop() @@ -567,7 +567,7 @@ func TestStopAndWait(t *testing.T) { } }) - t.Run("repeated calls to Stop", func(t *testing.T) { + t.Run("repeated calls to Stop", func(*testing.T) { cron := newWithSeconds() cron.Start() _ = cron.Stop() @@ -580,7 +580,7 @@ func TestStopAndWait(t *testing.T) { } }) - t.Run("a couple fast jobs added, still returns immediately", func(t *testing.T) { + t.Run("a couple fast jobs added, still returns immediately", func(*testing.T) { cron := newWithSeconds() cron.AddFunc("* * * * * *", func() {}) cron.Start() @@ -596,7 +596,7 @@ func TestStopAndWait(t *testing.T) { } }) - t.Run("a couple fast jobs and a slow job added, waits for slow job", func(t *testing.T) { + t.Run("a couple fast jobs and a slow job added, waits for slow job", func(*testing.T) { cron := newWithSeconds() cron.AddFunc("* * * * * *", func() {}) cron.Start() @@ -623,7 +623,7 @@ func TestStopAndWait(t *testing.T) { } }) - t.Run("repeated calls to stop, waiting for completion and after", func(t *testing.T) { + t.Run("repeated calls to stop, waiting for completion and after", func(*testing.T) { cron := newWithSeconds() cron.AddFunc("* * * * * *", func() {}) cron.AddFunc("* * * * * *", func() { time.Sleep(2 * time.Second) }) diff --git a/internal/cron/parser.go b/internal/cron/parser.go index ec80e3fd..d19ccc6e 100644 --- a/internal/cron/parser.go +++ b/internal/cron/parser.go @@ -1,11 +1,12 @@ package cron import ( - "fmt" "math" "strconv" "strings" "time" + + "github.com/pkg/errors" ) // Configuration options for creating a parser. Most options specify which @@ -86,7 +87,7 @@ func NewParser(options ParseOption) Parser { // It accepts crontab specs and features configured by NewParser. func (p Parser) Parse(spec string) (Schedule, error) { if len(spec) == 0 { - return nil, fmt.Errorf("empty spec string") + return nil, errors.New("empty spec string") } // Extract timezone if present @@ -96,7 +97,7 @@ func (p Parser) Parse(spec string) (Schedule, error) { i := strings.Index(spec, " ") eq := strings.Index(spec, "=") if loc, err = time.LoadLocation(spec[eq+1 : i]); err != nil { - return nil, fmt.Errorf("provided bad location %s: %v", spec[eq+1:i], err) + return nil, errors.Wrap(err, "provided bad location") } spec = strings.TrimSpace(spec[i:]) } @@ -104,7 +105,7 @@ func (p Parser) Parse(spec string) (Schedule, error) { // Handle named schedules (descriptors), if configured if strings.HasPrefix(spec, "@") { if p.options&Descriptor == 0 { - return nil, fmt.Errorf("parser does not accept descriptors: %v", spec) + return nil, errors.New("descriptors not enabled") } return parseDescriptor(spec, loc) } @@ -168,7 +169,7 @@ func normalizeFields(fields []string, options ParseOption) ([]string, error) { optionals++ } if optionals > 1 { - return nil, fmt.Errorf("multiple optionals may not be configured") + return nil, errors.New("multiple optionals may not be configured") } // Figure out how many fields we need @@ -183,9 +184,9 @@ func normalizeFields(fields []string, options ParseOption) ([]string, error) { // Validate number of fields if count := len(fields); count < min || count > max { if min == max { - return nil, fmt.Errorf("expected exactly %d fields, found %d: %s", min, count, fields) + return nil, errors.New("incorrect number of fields") } - return nil, fmt.Errorf("expected %d to %d fields, found %d: %s", min, max, count, fields) + return nil, errors.New("incorrect number of fields, expected " + strconv.Itoa(min) + "-" + strconv.Itoa(max)) } // Populate the optional field if not provided @@ -196,7 +197,7 @@ func normalizeFields(fields []string, options ParseOption) ([]string, error) { case options&SecondOptional > 0: fields = append([]string{defaults[0]}, fields...) default: - return nil, fmt.Errorf("unknown optional field") + return nil, errors.New("unexpected optional field") } } @@ -278,7 +279,7 @@ func getRange(expr string, r bounds) (uint64, error) { return 0, err } default: - return 0, fmt.Errorf("too many hyphens: %s", expr) + return 0, errors.New("too many hyphens: " + expr) } } @@ -299,20 +300,20 @@ func getRange(expr string, r bounds) (uint64, error) { extra = 0 } default: - return 0, fmt.Errorf("too many slashes: %s", expr) + return 0, errors.New("too many slashes: " + expr) } if start < r.min { - return 0, fmt.Errorf("beginning of range (%d) below minimum (%d): %s", start, r.min, expr) + return 0, errors.New("beginning of range below minimum: " + expr) } if end > r.max { - return 0, fmt.Errorf("end of range (%d) above maximum (%d): %s", end, r.max, expr) + return 0, errors.New("end of range above maximum: " + expr) } if start > end { - return 0, fmt.Errorf("beginning of range (%d) beyond end of range (%d): %s", start, end, expr) + return 0, errors.New("beginning of range after end: " + expr) } if step == 0 { - return 0, fmt.Errorf("step of range should be a positive number: %s", expr) + return 0, errors.New("step cannot be zero: " + expr) } return getBits(start, end, step) | extra, nil @@ -332,10 +333,10 @@ func parseIntOrName(expr string, names map[string]uint) (uint, error) { func mustParseInt(expr string) (uint, error) { num, err := strconv.Atoi(expr) if err != nil { - return 0, fmt.Errorf("failed to parse int from %s: %s", expr, err) + return 0, errors.Wrap(err, "failed to parse number") } if num < 0 { - return 0, fmt.Errorf("negative number (%d) not allowed: %s", num, expr) + return 0, errors.New("number must be positive") } return uint(num), nil @@ -419,17 +420,16 @@ func parseDescriptor(descriptor string, loc *time.Location) (Schedule, error) { Dow: all(dow), Location: loc, }, nil - } const every = "@every " if strings.HasPrefix(descriptor, every) { duration, err := time.ParseDuration(descriptor[len(every):]) if err != nil { - return nil, fmt.Errorf("failed to parse duration %s: %s", descriptor, err) + return nil, errors.Wrap(err, "failed to parse duration") } return Every(duration), nil } - return nil, fmt.Errorf("unrecognized descriptor: %s", descriptor) + return nil, errors.New("unrecognized descriptor: " + descriptor) } diff --git a/internal/cron/parser_test.go b/internal/cron/parser_test.go index 41c8c520..a0b03ed2 100644 --- a/internal/cron/parser_test.go +++ b/internal/cron/parser_test.go @@ -255,7 +255,7 @@ func TestNormalizeFields(t *testing.T) { } for _, test := range tests { - t.Run(test.name, func(t *testing.T) { + t.Run(test.name, func(*testing.T) { actual, err := normalizeFields(test.input, test.options) if err != nil { t.Errorf("unexpected error: %v", err) @@ -300,7 +300,7 @@ func TestNormalizeFields_Errors(t *testing.T) { }, } for _, test := range tests { - t.Run(test.name, func(t *testing.T) { + t.Run(test.name, func(*testing.T) { actual, err := normalizeFields(test.input, test.options) if err == nil { t.Errorf("expected an error, got none. results: %v", actual) diff --git a/plugin/idp/oauth2/oauth2_test.go b/plugin/idp/oauth2/oauth2_test.go index 90e3f3ed..b91f0372 100644 --- a/plugin/idp/oauth2/oauth2_test.go +++ b/plugin/idp/oauth2/oauth2_test.go @@ -67,7 +67,7 @@ func TestNewIdentityProvider(t *testing.T) { }, } for _, test := range tests { - t.Run(test.name, func(t *testing.T) { + t.Run(test.name, func(*testing.T) { _, err := NewIdentityProvider(test.config) assert.ErrorContains(t, err, test.containsErr) }) diff --git a/test/util/resource_name_test.go b/test/util/resource_name_test.go index fecd3354..bf20e595 100644 --- a/test/util/resource_name_test.go +++ b/test/util/resource_name_test.go @@ -27,7 +27,7 @@ func TestUIDMatcher(t *testing.T) { } for _, test := range tests { - t.Run(test.input, func(t *testing.T) { + t.Run(test.input, func(*testing.T) { result := util.UIDMatcher.MatchString(test.input) if result != test.expected { t.Errorf("For input '%s', expected %v but got %v", test.input, test.expected, result)