jmrozanec/cron-utils

Recent Version 9.1.6 introduced a bug on yearly schedules if a concrete year is sepecified in the cron expression

meincs opened this issue ยท 5 comments

Given the following two unit tests, both succeed with version 9.1.5, where the second one fails for version 9.1.6. The difference between the two tests is the concrete year 2017 used in the cron expression of the second test (testCustomQuarterlyScheduleForDesignatedYear2017). The concrete problem is that the last expected execution time 2017-11-16T00:00 is no longer generated.

import static org.junit.jupiter.api.Assertions.*;
import org.junit.jupiter.api.*;
import java.time.*;
import java.util.*;


@Test
void testCustomQuarterlySchedule() {
    final String customQuaterlyStartingWithDay47OfYear = "0 0 0 ? * ? * 47/91";
    final LocalDateTime[] expectedExecutionTimes = IntStream.range(0, 4).mapToObj(i -> LocalDateTime.of(2017, 1, 1, 0, 0).withDayOfYear(47 + i * 91)).toArray(LocalDateTime[]::new);
    final LocalDateTime[] scheduledDates = Crons.executionTimesFor(customQuaterlyStartingWithDay47OfYear, expectedExecutionTimes[0], expectedExecutionTimes[expectedExecutionTimes.length - 1].plusDays(1)).stream().toArray(LocalDateTime[]::new);

    assertArrayEquals(expectedExecutionTimes, scheduledDates, "Scheduled dates should match precalculated dates.");
}


@Test
@Disabled("broken since cronUtils version 9.1.6")
void testCustomQuarterlyScheduleForDesignatedYear2017() {
    final String customQuaterlyStartingWithDay47OfYear = "0 0 0 ? * ? 2017 47/91";
    final LocalDateTime[] expectedExecutionTimes = IntStream.range(0, 4).mapToObj(i -> LocalDateTime.of(2017, 1, 1, 0, 0).withDayOfYear(47 + i * 91)).toArray(LocalDateTime[]::new);
    final LocalDateTime[] scheduledDates = Crons.executionTimesFor(customQuaterlyStartingWithDay47OfYear, expectedExecutionTimes[0], expectedExecutionTimes[expectedExecutionTimes.length - 1].plusDays(1)).stream().toArray(LocalDateTime[]::new);

    assertArrayEquals(expectedExecutionTimes, scheduledDates, "Scheduled dates should match precalculated dates.");
}

Sorry, i missed to add info on the Crons utility class used in the test code above. I just added a PR with a working example.

Seems to be that bug was introduced with fix of #446. DST overlap must only be taken into account if and only if a next next executions exists. If a next next execution doesn't exist, a NoSuchValueException is thrown and forwarded as empty optional indicating that a next execution doesn't exist too. Instead the NoSuchValueException should be ignored and the existing next execution returned.

Fix added to open PR.

@meincs many thanks for this! ๐Ÿ˜„ ๐Ÿš€ Will review the PR shortly, merge, and release a new version. Thanks!

@jmrozanec when can we expect the the release of a new version comprising the provided bug fix? Many thanks in advance for your response.

@meincs release 9.1.7 was published! Thanks! ๐Ÿ˜„