Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug with Cron DayOfWeek when moving to next month #215

Closed
zakrhol opened this issue Jul 3, 2017 · 1 comment
Closed

Bug with Cron DayOfWeek when moving to next month #215

zakrhol opened this issue Jul 3, 2017 · 1 comment
Milestone

Comments

@zakrhol
Copy link

zakrhol commented Jul 3, 2017

Hi, some tests on our system was failing on friday (2017/06/30).

with this cron expression 0 0 8 ? * MON-FRI it gives me a wrong date if the current day is the last Mo-Fr of this month. More precisely it gives me SATURDAY :((

See the attached test case. This fails with 5.0.5 and also with 6.0.2.

@Test
    public void testWorkdayBugWithNextMonth() {

        testWorkdays8( LocalDateTime.of( 2017, 7, 7, 10, 00 ), LocalDateTime.of( 2017, 7, 10, 8, 00 ) ); //good
        testWorkdays8( LocalDateTime.of( 2017, 8, 31, 10, 00 ), LocalDateTime.of( 2017, 9, 1, 8, 00 ) ); //good
        testWorkdays8( LocalDateTime.of( 2017, 6, 30, 10, 00 ), LocalDateTime.of( 2017, 7, 3, 8, 00 ) ); //not good
        testWorkdays8( LocalDateTime.of( 2017, 9, 29, 10, 00 ), LocalDateTime.of( 2017, 10, 2, 8, 00 ) ); //not good
    }

    private void testWorkdays8( LocalDateTime startDate, LocalDateTime expectedNextExecution ) {
        CronParser parser = new CronParser( CronDefinitionBuilder.instanceDefinitionFor( CronType.QUARTZ ) );
        Cron quartzCron = parser.parse( "0 0 8 ? * MON-FRI" );
        ExecutionTime executionTime = ExecutionTime.forCron( quartzCron );
        ZonedDateTime zonedDateTime = ZonedDateTime.of( startDate, ZoneId.systemDefault() );
        org.threeten.bp.ZonedDateTime xx = timeToThreeten( zonedDateTime );
        Optional<org.threeten.bp.ZonedDateTime> next = executionTime.nextExecution( xx );
        assertEquals( timeToThreeten( ZonedDateTime.of( expectedNextExecution, ZoneId.systemDefault() ) ), next.get() );
    }

    private org.threeten.bp.ZonedDateTime timeToThreeten( ZonedDateTime zonedDateTime ) {
        org.threeten.bp.ZonedDateTime xx =
                        org.threeten.bp.ZonedDateTime.of( zonedDateTime.getYear(), zonedDateTime.getMonthValue(), zonedDateTime.getDayOfMonth(), zonedDateTime.getHour(), zonedDateTime.getMinute(), zonedDateTime.getSecond(), zonedDateTime.getNano(), org.threeten.bp.ZoneId.of( zonedDateTime.getZone().getId() ) );
        return xx;
    }
java.lang.AssertionError: expected:<2017-07-03T08:00+02:00[Europe/Berlin]> but was:<2017-07-01T08:00+02:00[Europe/Berlin]>
	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.failNotEquals(Assert.java:834)
	at org.junit.Assert.assertEquals(Assert.java:118)
	at org.junit.Assert.assertEquals(Assert.java:144)
	at com.inet.taskplanner.server.api.trigger.time.TimeTriggerTest.testWorkdays8(TimeTriggerTest.java:670)
@jmrozanec
Copy link
Owner

@zakrhol thank you for reporting this! Wouldn't you mind contributing the tests as PR? We will be eager to merge them into master. Thanks!

@jmrozanec jmrozanec added this to the 6.0.3 milestone Jul 3, 2017
zakrhol pushed a commit to zakrhol/cron-utils that referenced this issue Jul 4, 2017
pangyikhei pushed a commit to pangyikhei/cron-utils that referenced this issue Jul 27, 2017
… the valid day of week (numeric) values

- also removed usage of Java 8 time package
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants