search

bradymholt / cRonstrue

JavaScript library that translates Cron expressions into human readable descriptions
http://bradymholt.github.io/cRonstrue/
MIT License
1.28k stars 168 forks source link

Fix "every 45 minutes" → "every 45th minutes" #231

Closed trungdq88 closed 2 years ago

trungdq88 commented 2 years ago

Thanks for this awesome library!

I found an edge case:

https://github.com/bradymholt/cRonstrue/blob/master/test/cronstrue.ts#L66-L68

That test case is inaccurate.

Same for */45 * * * *.

bradymholt commented 2 years ago

@trungdq88 - Are you sure about that? You mentioned "per cronjob specs", can you point to where you are referencing? As far as I know and can see based on crontab documentation (source), step values specify an "every" type recurrence. If you wanted an "every 45th minutes" type expression, I would expect an expression like this: 45 * * * * with no step value.

trungdq88 commented 2 years ago

@bradymholt to be honest I have not read the whole cronjob specs, but it was my understanding the whole time.

I'm a bit lazy to find the correct wording, but I crossed check the "next run time" with any cron job parsers (or see a preview on the internet: https://crontab.guru/, https://cronjob.xyz/), it clearly shows that */45 is not "every 45 minutes".

CleanShot 2022-03-29 at 09 36 27@2x
trungdq88 commented 2 years ago

This Stack Overflow post also gives some info: https://stackoverflow.com/questions/29074816/cron-expression-to-be-executed-every-45-minutes

bradymholt commented 2 years ago

I think the output on https://cronjob.xyz/ in wrong. I would expect that output for 0/45 * * * *, not */45 * * * *. As for https://crontab.guru/, I think I agree with its output of "At every 45th minute.", as */45 * * * * should simply fire every 45 minutes, regardless of the time it started.

Per the crontab man:

CleanShot 2022-03-29 at 14 52 19

I tested this on a Ubuntu Linux box and confirmed it:

*/45 * * * * /usr/bin/printf '%s %s\n' "$(date)" >> /tmp/timestamp
trungdq88 commented 2 years ago

Can you share the output of the /tmp/timestamp file?

bradymholt commented 2 years ago

Can you share the output of the /tmp/timestamp file?

Wed Mar 29 11:03:03 MDT 2022
Wed Mar 29 11:48:03 MDT 2022
Wed Mar 29 12:33:03 MDT 2022
trungdq88 commented 2 years ago

Hmm, that is strange indeed. I don't have time to dig into this further, but maybe I'll revisit later.

In the meantime, feel free to close the PR. Thanks for your time looking into this, and thanks for the awesome lib!

bradymholt commented 2 years ago

Sure thing, thanks!