search

MichaelXavier / cron

Cron data structure and parser for Haskell
Other
53 stars 33 forks source link

Incorrect timing for combinations of StepField and RangeField #18

Closed andrewthad closed 9 years ago

andrewthad commented 9 years ago

When you do this:

1-59/2 * * * *

it treats it as though you did this

*/2 * * * *
MichaelXavier commented 9 years ago

I added a test case for this and it passed. Please have a look.

andrewthad commented 9 years ago

That indicates that it should be working. I'll check this out again when I get back to the office. Thanks for looking at it.

MichaelXavier commented 9 years ago

Any update on this?

andrewthad commented 9 years ago

Sorry I dropped the ball on this. I've tried it again with a more minimal case. It looks like this:

scheduleAllJobs :: App -> IO ()                                                                       
scheduleAllJobs app = do
  _ <- execSchedule $ do
    addJob (heartbeat app) "* * * * *"
    addJob (flip runReaderT app $ Task.log "Odd Minute") "1-59/2 * * * *"
    addJob (flip runReaderT app $ Task.log "Even Minute") "0-59/2 * * * *"

I would expect to see the heartbeat every minute and the odd and even notifications on alternating minutes. Here's what I get instead:

17/Jul/2015:15:17:59 -0400 Even Minute
17/Jul/2015:15:17:59 -0400 Odd Minute
17/Jul/2015:15:17:59 -0400 Repeated jobs heartbeat
17/Jul/2015:15:18:59 -0400 Repeated jobs heartbeat
17/Jul/2015:15:19:59 -0400 Even Minute
17/Jul/2015:15:19:59 -0400 Odd Minute
17/Jul/2015:15:19:59 -0400 Repeated jobs heartbeat
17/Jul/2015:15:20:59 -0400 Repeated jobs heartbeat
17/Jul/2015:15:21:59 -0400 Even Minute
17/Jul/2015:15:21:59 -0400 Odd Minute
17/Jul/2015:15:21:59 -0400 Repeated jobs heartbeat

So, even though your test shows that it parses correctly, I think that the parsed result is being used incorrectly somewhere. The event that should fire on the odd minutes is firing on the even minutes.

andrewthad commented 9 years ago

I suspect that there's a problem in the scheduleMatches function, but it's hard for me to understand how that function works.

andrewthad commented 9 years ago

I'm updating the name of this issue to reflect the understand of it that I now have.

MichaelXavier commented 9 years ago

Really sorry it took so long. Been pretty behind on my open source stuff. I found a bug in the range calculation function. Wrote a test for the exact times you specified and made it pass. Please give the above mentioned patch a try and let me know if it fixes your issue :)

andrewthad commented 9 years ago

That's alright. I'm always behind on my open source stuff too ;)

I'll give it a shot today. Thanks.

andrewthad commented 9 years ago

I just tried this branch with the example I gave earlier, and it has the correct behavior now. Thanks!

MichaelXavier commented 9 years ago

Released as version 0.3.1