Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] - Time reported by NextRun when running WithSingletonMode and WithStartDateTime incorrect #704

Closed
aneesh1 opened this issue Mar 27, 2024 · 2 comments · Fixed by #705
Labels
bug Something isn't working

Comments

@aneesh1
Copy link

aneesh1 commented Mar 27, 2024

Describe the bug

The time reported by NextRun isn't right when running WithSingletonMode and WithStartDateTime. For instance, suppose we have a job that starts at time t (in the future) and repeats with some interval i. When calling NextRun at some time t_i that is before t, the NextRun is reported correctly. However, after the job is run for the first time, the time reported by NextRun is t + 2i when it should be t + i instead. Here's a table depicting the pattern I observed

Interval when NextRun is called Actual Return Value Expected Return Value
0 -> t t t
t -> t + i t + 2i t + i
t + i -> t + 2i t + 4i t + 2i

To Reproduce

Here's a test that I wrote in the job_test.go file:

func TestJob_NextRun(t *testing.T) {
	testTime := time.Now()

	s := newTestScheduler(t)

        // run a job every 10 milliseconds that starts 10 milliseconds after the current time
	j, err := s.NewJob(
		DurationJob(
			10*time.Millisecond,
		),
		NewTask(
			func() {},
		),
		WithStartAt(WithStartDateTime(testTime.Add(10*time.Millisecond))),
		WithSingletonMode(LimitModeReschedule),
	)
	require.NoError(t, err)

	s.Start()
	nextRun, err := j.NextRun()
	require.NoError(t, err)

        // `NextRun` should report `testTime.Add(10*time.Millisecond)`
	assert.Equal(t, testTime.Add(10*time.Millisecond), nextRun)

        // sleep for 11ms to to wait for the next job
	time.Sleep(11 * time.Millisecond)

	nextRun, err = j.NextRun()
	assert.NoError(t, err)

        // `NextRun` should report a time 20 milliseconds after `testTime`, but instead reports a value that is `30ms` after
	assert.Equal(t, testTime.Add(20*time.Millisecond), nextRun)
	assert.Equal(t, 20*time.Millisecond, nextRun.Sub(testTime))

	err = s.Shutdown()
	require.NoError(t, err)
}

Version

v2.2.9

Expected behavior

I expect that these two asserts:

	assert.Equal(t, testTime.Add(20*time.Millisecond), nextRun)
	assert.Equal(t, 20*time.Millisecond, nextRun.Sub(testTime))

should not fail. I also modified this test to see what happens when more jobs are run:

func TestJob_NextRun(t *testing.T) {
	testTime := time.Now()

	s := newTestScheduler(t)

        // run a job every 10 milliseconds that starts 10 milliseconds after the current time
	j, err := s.NewJob(
		DurationJob(
			10*time.Millisecond,
		),
		NewTask(
			func() {},
		),
		WithStartAt(WithStartDateTime(testTime.Add(10*time.Millisecond))),
		WithSingletonMode(LimitModeReschedule),
	)
	require.NoError(t, err)

	s.Start()
	nextRun, err := j.NextRun()
	require.NoError(t, err)

        // `NextRun` should report `testTime.Add(10*time.Millisecond)`
	assert.Equal(t, testTime.Add(10*time.Millisecond), nextRun)

        // sleep for 11ms to to wait for the next job
	time.Sleep(11 * time.Millisecond)

	nextRun, err = j.NextRun()
	assert.NoError(t, err)

        // `NextRun` should report a time 20 milliseconds after `testTime`, but instead reports a value that is `30ms` after
	assert.Equal(t, testTime.Add(20*time.Millisecond), nextRun)
	assert.Equal(t, 20*time.Millisecond, nextRun.Sub(testTime))

        // sleep for 11ms to to wait for the next job
	time.Sleep(11 * time.Millisecond)

	nextRun, err = j.NextRun()
	assert.NoError(t, err)

         // `NextRun` should report a time 30 milliseconds after `testTime`, but instead reports a value that is `50ms` after
	assert.Equal(t, testTime.Add(30*time.Millisecond), nextRun)
	assert.Equal(t, 30*time.Millisecond, nextRun.Sub(testTime))

	err = s.Shutdown()
	require.NoError(t, err)

Fortunately what is working: my job is running at the specified interval provided to DurationJob.

@aneesh1 aneesh1 added the bug Something isn't working label Mar 27, 2024
@aneesh1
Copy link
Author

aneesh1 commented Mar 27, 2024

Further context: it looks gocron v2.2.1 reports correct values for lastRun and nextRun. However, the fix for #661 is not in this release.

@aneesh1
Copy link
Author

aneesh1 commented Mar 29, 2024

Follow up question: when I disable WithSingletonMode the nextRun time is reported correctly - why?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant