BUG in nth occurrence of weekday in a month when n=5

[tarzan]

This does not seem to be correct:

iex(1)> "0 9 * * mon#5" |> Crontab.CronExpression.Parser.parse!() |> Crontab.Scheduler.get_next_run_date()
{:ok, ~N[2024-11-02 09:00:00]}

as 2024-11-02 is neither a monday nor a fifth occurrence of it

The actual expected run_date is 2024-12-30 09:00:00

Should I try to fix this bug?

[coveralls]

coverage: 95.751% (+0.05%) from 95.702%
when pulling 946a2ac on DefactoSoftware:bug-in-n-occurrences
into c8cd678 on maennchen:main.

[maennchen]

@tarzan Thanks for the PR. It seems that we have an issue as well going backwards.

"0 9 * * mon#5"
|> Crontab.CronExpression.Parser.parse!()
|> Crontab.Scheduler.get_previous_run_date()
#=> {:ok, ~N[2024-07-29 09:00:00]}

I think it should be ~N[2024-09-30 09:00:00] instead. Do you want to have a look at that as well?

Found with:

diff --git a/test/crontab/functional_test.exs b/test/crontab/functional_test.exs
index 19e082b..3e34eba 100644
--- a/test/crontab/functional_test.exs
+++ b/test/crontab/functional_test.exs
@@ -130,7 +130,9 @@ defmodule Crontab.FunctionalTest do
     {"* * * * 5#1", "* * * * 5#1 *", ~N[2011-07-01 00:00:00], ~N[2011-07-01 00:00:00],
      ~N[2011-07-01 00:00:00], true},
     {"* * * * 3#4", "* * * * 3#4 *", ~N[2011-07-01 00:00:00], ~N[2011-07-27 00:00:00],
-     ~N[2011-06-22 23:59:00], false}
+     ~N[2011-06-22 23:59:00], false},
+    {"0 9 * * mon#5", "0 9 * * 1#5 *", ~N[2024-10-22 09:00:00], ~N[2024-12-30 09:00:00],
+     ~N[2024-09-30 09:00:00], false}
   ]
 
   for {cron_expression, written_cron_expression, start_date, next_search_date,
@@ -149,8 +151,12 @@ defmodule Crontab.FunctionalTest do
       {:ok, cron_expression} = Parser.parse(@cron_expression)
       assert Composer.compose(cron_expression) == @written_cron_expression
 
-      assert Crontab.Scheduler.get_next_run_date(cron_expression, @start_date) ==
-               {:ok, @next_search_date}
+      assert {:ok, next_search_date} =
+               Crontab.Scheduler.get_next_run_date(cron_expression, @start_date)
+
+      assert Crontab.DateChecker.matches_date?(cron_expression, next_search_date)
+
+      assert next_search_date == @next_search_date
 
       case @previous_search_date do
         :none ->
@@ -158,8 +164,12 @@ defmodule Crontab.FunctionalTest do
                    {:error, "No compliant date was found for your interval."}
 
         _ ->
-          assert Crontab.Scheduler.get_previous_run_date(cron_expression, @start_date) ==
-                   {:ok, @previous_search_date}
+          assert {:ok, previous_search_date} =
+                   Crontab.Scheduler.get_previous_run_date(cron_expression, @start_date)
+
+          assert Crontab.DateChecker.matches_date?(cron_expression, previous_search_date)
+
+          assert previous_search_date == @previous_search_date
       end
 
       assert Crontab.DateChecker.matches_date?(cron_expression, @start_date) == @matches_now

[tarzan]

@tarzan Thanks for the PR. It seems that we have an issue as well going backwards.

"0 9 * * mon#5"
|> Crontab.CronExpression.Parser.parse!()
|> Crontab.Scheduler.get_previous_run_date()
#=> {:ok, ~N[2024-07-29 09:00:00]}

I think it should be ~N[2024-09-30 09:00:00] instead. Do you want to have a look at that as well?

Sure! I added another failing test and then went on to fix it. Hope you approve of the changes.

[maennchen]

@tarzan I've backported the bugfix to 1.1.13 and released it as 1.1.14.

I unfortunately can't directly release 1.2 since there's an outstanding issue to solve first.