Bugfix for weekday validator #31
Conversation
|
Actually got a better repro.. Anytime the first x-day of the month is the 7th, it gets skipped |
Output from go test before:
```
--- FAIL: TestNextTickAfter (0.01s)
--- FAIL: TestNextTickAfter/next_run_after (0.01s)
--- FAIL: TestNextTickAfter/next_run_after/next_run_after_excl_#63:_0_0_*_*_1#1 (0.00s)
next_test.go:66: [0 0 * * 1#1][2009-11-23 00:00:00] expected 2009-12-07 00:00:00, got 2010-01-04 00:00:00
```
Now it all passes
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## main #31 +/- ##
=======================================
Coverage 94.78% 94.78%
=======================================
Files 9 9
Lines 594 594
=======================================
Hits 563 563
Misses 16 16
Partials 15 15
|
| @@ -142,5 +142,5 @@ func isValidWeekDay(val string, last int, ref time.Time) (bool, error) { | |||
| return false, nil | |||
| } | |||
|
|
|||
| return ref.Day()/7 == nth-1, nil | |||
| return (ref.Day()-1)/7 == nth-1, nil | |||
There was a problem hiding this comment.
strange this change still has all tests passed
There was a problem hiding this comment.
I think this is an edge case that wasn't covered in the previous unit tests. If we use a cron string that does first/second/etc monday/tuesday/etc of the month, then we get in a situation where days 1-6 are in week 0, but day 7 is week 1. This change makes it that days 1-7 are in week 0.
philodavies
changed the title
|
thank you very much (and yes cron (parsing) is not nearly as simple as it looks) |
Output from go test:
I haven't had time to dig into why this case is not working as expected, but I noticed something really weird here. If I say the first Monday of the month it doesn't always give the right answer:
Go Playground example: https://go.dev/play/p/0GJnCNPWCWC