Resolve the problem of 59th second while discovering timeperiods
The problem was described during discussion at dd4d270d92fd64d694ab7b07ccd79294a6d84440:
We have two problems here, and I just left one of them (and we can discuss if this is good or not).
- (...)
- The specific case, and the bug inside of
InPeriod()
, is that the period is not discovered properly if you're at the 59 second of the minute. Let's consider two example time moments and analyze howInPeriod()
will work:
- We have a period pattern
* 0 14 * * * *
.- For time
2017-02-21T14:00:00
theexpression.Next(now)
will return us2017-02-21T14:00:01
. The difference between the current time andnextIn
is inside of [-1s, 1s] interval, so we're assuming that the time is inside of defined period. The same would happen for each second up to2017-02-21T14:00:58
. Each timenextIn
will return a time with +1s value.- For time
2017-02-21T14:00:59
however, theexpression.Next(now)
will return us2017-02-22T14:00:00
. This is done because of howgithub.com/gorhill/cronexpr
works. In this case the difference betweennextIn
and current time is-23h59m1s
which is a way out of the defined [-1s, 1s] interval.Now, the problem of 59th second is a bug, but it's forced by the behavior of used library. We should consider if this is really a bug, and if we decide that it is, we should send a patch to the library's upstream. However, it's hard for me to imagine that someone will want to define the
OffPeakPeriod
with a resolution measured in seconds. Then, maybe, the easiest solution would be to cut the first part of the period (which forgithub.com/gorhill/cronexpr
means seconds) and force 0 there.The fix that I've made is however not a solution for the bug, but for randomly failing tests. The previous version was not trustworthy, because it was failing randomly only when the test was executed at
hh:mm:59
. In any other time it was passing without problems. This was occurring because we were partially controlling the environment - using strict hours and minutes, but not seconds. With current version the test will not fail in random moments. But to notice the problem of 59th second I'll push a updated version in a moment.
We should decide if:
- we're OK with the problem of 59th second,
- this is a bug and we should fix this in Runner (e.g. by altering the seconds field of each periodPattern),
- this is a bug and we should send a fix to the library's upstream.