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

Already on GitHub? Sign in to your account

Add additional test for daylight savings glitch #251

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Owner

dsyer commented Mar 10, 2013

The problem was that clocks go forward at 2am, so 2am doesn't exist once a
year. Users might be surprised that their cron trigger doesn't go off one
night, but that is arguably correct (and what happens now). The test can be
modified if we decide to change the trigger behaviour.

@ghost ghost assigned sbrannen Mar 10, 2013

Member

sbrannen commented Mar 10, 2013

Thanks for creating this additional test, Dave!

I'll take a look at it now.

Member

sbrannen commented Mar 10, 2013

Unfortunately this test currently fails on my machine with the following:

java.lang.AssertionError: expected:<Sun Mar 31 03:10:00 CEST 2013> but was:<Mon Apr 01 02:10:00 CEST 2013>

Can you please investigate this?

Thanks,

Sam

Owner

dsyer commented Mar 11, 2013

When it fails, what is the value of timeZone? Maybe the test timeZone.equals(TimeZone.getTimeZone("CET")) is failing, but you are actually in that time zone?

Add additional test for daylight savings glitch
The problem was that clocks go forward *at* 2am, so
2am doesn't exist once a year.  Users might be surprised
that their cron trigger doesn't go off one night, but that
is arguably correct (and what happens now).  The test can be
modified if we decide to change the trigger behaviour.
Owner

dsyer commented Mar 11, 2013

New patch fixes timezone explicitly instead of using default. Should pass even in Europe now!

sbrannen added a commit that referenced this pull request Mar 11, 2013

Merge pull request #251 from dsyer/feature/crontest
# By Dave Syer
* dsyer-feature/crontest:
  Add additional test for daylight savings glitch

@sbrannen sbrannen closed this Mar 11, 2013

Member

sbrannen commented Mar 11, 2013

OK. That seems to work now. I've merged this into 3.2.x.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment