fix: break statement in execute with retries (#93)

quartz/scheduler.go

@@ -404,13 +404,14 @@ func executeWithRetries(ctx context.Context, jobDetail *JobDetail) {
 	if err == nil {
 		return
 	}
-	for i := 0; i < jobDetail.opts.MaxRetries; i++ {
+retryLoop:
+	for i := 1; i <= jobDetail.opts.MaxRetries; i++ {
 		timer := time.NewTimer(jobDetail.opts.RetryInterval)
 		select {
 		case <-timer.C:
 		case <-ctx.Done():
 			timer.Stop()
-			break
+			break retryLoop
 		}
 		logger.Tracef("Job %s retry %d", jobDetail.jobKey, i)
 		err = jobDetail.job.Execute(ctx)

quartz/scheduler_test.go

@@ -300,7 +300,7 @@ func TestSchedulerJobWithRetries(t *testing.T) {
 	sched := quartz.NewStdScheduler()
 	opts := quartz.NewDefaultJobDetailOptions()
 	opts.MaxRetries = 3
-	opts.RetryInterval = 100 * time.Millisecond
+	opts.RetryInterval = 50 * time.Millisecond
 	jobDetail := quartz.NewJobDetailWithOptions(
 		funcRetryJob,
 		quartz.NewJobKey("funcRetryJob"),
@@ -309,14 +309,67 @@ func TestSchedulerJobWithRetries(t *testing.T) {
 	err := sched.ScheduleJob(jobDetail, quartz.NewRunOnceTrigger(time.Millisecond))
 	assert.Equal(t, err, nil)
 
+	assert.Equal(t, funcRetryJob.JobStatus(), job.StatusNA)
+	assert.Equal(t, int(atomic.LoadInt32(&n)), 0)
+
 	sched.Start(ctx)
-	sched.Start(ctx)
+
+	time.Sleep(25 * time.Millisecond)
+	assert.Equal(t, funcRetryJob.JobStatus(), job.StatusFailure)
+	assert.Equal(t, int(atomic.LoadInt32(&n)), 1)
+
 	time.Sleep(50 * time.Millisecond)
 	assert.Equal(t, funcRetryJob.JobStatus(), job.StatusFailure)
+	assert.Equal(t, int(atomic.LoadInt32(&n)), 2)
+
 	time.Sleep(100 * time.Millisecond)
+	assert.Equal(t, funcRetryJob.JobStatus(), job.StatusOK)
+	assert.Equal(t, int(atomic.LoadInt32(&n)), 3)
+
+	sched.Stop()
+}
+
+func TestSchedulerJobWithRetriesCtxDone(t *testing.T) {
+	var n int32
+	funcRetryJob := job.NewFunctionJob(func(_ context.Context) (string, error) {
+		atomic.AddInt32(&n, 1)
+		if n < 3 {
+			return "", errors.New("less than 3")
+		}
+		return "ok", nil
+	})
+	ctx, cancel := context.WithCancel(context.Background())
+	sched := quartz.NewStdScheduler()
+	opts := quartz.NewDefaultJobDetailOptions()
+	opts.MaxRetries = 3
+	opts.RetryInterval = 50 * time.Millisecond
+	jobDetail := quartz.NewJobDetailWithOptions(
+		funcRetryJob,
+		quartz.NewJobKey("funcRetryJob"),
+		opts,
+	)
+	err := sched.ScheduleJob(jobDetail, quartz.NewRunOnceTrigger(time.Millisecond))
+	assert.Equal(t, err, nil)
+
+	assert.Equal(t, funcRetryJob.JobStatus(), job.StatusNA)
+	assert.Equal(t, int(atomic.LoadInt32(&n)), 0)
+
+	sched.Start(ctx)
+
+	time.Sleep(25 * time.Millisecond)
+	assert.Equal(t, funcRetryJob.JobStatus(), job.StatusFailure)
+	assert.Equal(t, int(atomic.LoadInt32(&n)), 1)
+
+	time.Sleep(50 * time.Millisecond)
 	assert.Equal(t, funcRetryJob.JobStatus(), job.StatusFailure)
+	assert.Equal(t, int(atomic.LoadInt32(&n)), 2)
+
+	cancel() // cancel the context after first retry
+
 	time.Sleep(100 * time.Millisecond)
-	assert.Equal(t, funcRetryJob.JobStatus(), job.StatusOK)
+	assert.Equal(t, funcRetryJob.JobStatus(), job.StatusFailure)
+	assert.Equal(t, int(atomic.LoadInt32(&n)), 2)
+
 	sched.Stop()
 }
 
@@ -344,3 +397,15 @@ func TestSchedulerArgumentValidationErrors(t *testing.T) {
 	_, err = sched.GetScheduledJob(nil)
 	assert.Equal(t, err.Error(), "jobKey is nil")
 }
+
+func TestSchedulerStartStop(t *testing.T) {
+	sched := quartz.NewStdScheduler()
+	ctx := context.Background()
+	sched.Start(ctx)
+	sched.Start(ctx)
+	assert.Equal(t, sched.IsStarted(), true)
+
+	sched.Stop()
+	sched.Stop()
+	assert.Equal(t, sched.IsStarted(), false)
+}