FluentScheduler icon indicating copy to clipboard operation
FluentScheduler copied to clipboard

JobEndInfo.Next not correct

Open kvroeden opened this issue 8 years ago • 5 comments

This maybe me not understanding but when I schedule a job like this:

JobManager.JobEnd += JobManager_JobEnd;
var aDate = DateTime.Now.AddSeconds(5);
registry.Schedule<TestJob>().ToRunOnceAt(aDate).AndEvery(3).Seconds();
JobManager.Initialize(registry);
JobManager.Start()

Then in JobManager_JobEnd(JobEndInfo obj) print out obj.NextRun, the first time it is reporting the start time instead of the next time (although the next job run is running at the correct time)

13:42:19:441 job start=13:42:19:426 next = 13:42:19:416 <--- should be 13:42:22 13:42:22:420 job start=13:42:22:420 next = 13:42:25:421
13:42:25:424 job start=13:42:25:423 next = 13:42:28:424 13:42:28:440 job start=13:42:28:439 next = 13:42:31:440

When I schedule the job to run now, it works as I expect

registry.Schedule<TestJob>().ToRunNow().AndEvery(3).Seconds();

13:43:09:612 job start=13:43:09:593 next = 13:43:12:593 13:43:12:593 job start=13:43:12:593 next = 13:43:15:594 13:43:15:607 job start=13:43:15:601 next = 13:43:18:605 13:43:18:611 job start=13:43:18:611 next = 13:43:21:612

Cheers, KJ

kvroeden avatar Nov 25 '16 12:11 kvroeden

Hi there, sorry for taking so long to answer!

I'm able to reproduce, it's a bug alright. It's not the easiest fix in the world and I right now I have a bunch of "awaiting release" to test and publish. I'll flag it as a bug and I'll fix it later.

tallesl avatar Apr 22 '17 21:04 tallesl

And again, has this already been fixed? I would also be happy to contribute by bugfixing!

pujux avatar Sep 19 '18 11:09 pujux

This one is still pending for version 5.0.0.0; I have not checked 5.3 yet.

mvburgh avatar Dec 09 '18 14:12 mvburgh

This issue is still present.

I have made this test to confirm that.

[Fact]
public void Should_Run_Exactly_Every_1_Minute()
{
   // Arrange
   var input = DateTime.Now;
   var expected = input.AddMinutes(1);

   // Act
   JobManager.AddJob(() => { }, s => s
       .WithName("run exactly every 1 minute")
       .ToRunOnceAt(input)
       .AndEvery(1)
       .Minutes());

   var actual = JobManager.AllSchedules.Last(s => s.Name == "run exactly every 1 minute").NextRun;

   // Assert
   Equal(expected, actual);
}

The test pass by changing this line : https://github.com/fluentscheduler/FluentScheduler/blob/16b0ee904b46e21f662a06cc0d5966c956af4a3b/FluentScheduler/JobManager.cs#L373 With :

childSchedule.NextRun = childSchedule.CalculateNextRun(schedule.NextRun)

It works because the child schedule need to be run after the NextRun of his parent, so the calculation need to be relative to the parent and not to Now. I don't know if the removing of the DelayRunFor in the calculation can cause a regression, I don't think because if the delay is applied to the parent and the child is relative to it, the same delay will be implicitly applied what I think is correct.

@tallesl If you think this change make sense I can make a PR.

kvpt avatar Sep 05 '20 00:09 kvpt

Duplicate with #261

kvpt avatar Sep 05 '20 00:09 kvpt