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

Test of time diff is not independent of system timezone #52

Closed
gbirke opened this issue Nov 1, 2017 · 2 comments
Closed

Test of time diff is not independent of system timezone #52

gbirke opened this issue Nov 1, 2017 · 2 comments

Comments

@gbirke
Copy link
Contributor

gbirke commented Nov 1, 2017

'DateTime#diff makes simple diffs' fails for the test that checks for a difference of 80 hours (24 * 3 + 8, lines 55-63 atm). On my machine, the diff functions returns 79 hours instead of the expected 80. This is probably because I'm using the German time zone (CET) and March 27, 2016 is the day where we switched to daylight savings time.

I'm not sure about the correct way to resolve this:

  • Use an explicit time zone in the DateTime tests where the test date does not run into DST conflicts and add an additional test for dates where there are DST switches?
  • Change dates in the test to a time period where it's unlikely to be DST changes?
  • Document how to provide a testing environment set to the correct time zone (i.e. using TZ=America/New_York gulp test instead of gulp test )?
@icambron
Copy link
Member

icambron commented Nov 1, 2017

Use an explicit time zone in the DateTime tests where the test date does not run into DST conflicts and add an additional test for dates where there are DST switches?

I don't want to do this, partially because it's a hassle and partially because there are some tests that want to test the local zone implementation directly.

Change dates in the test to a time period where it's unlikely to be DST changes?

This is challenging to get completely right. I remember trying it with Moment a few years ago, literally running all the tests in all the time zones and trying to patch them up. We could get them to run in most time zones, though, and I think we should. It basically means to avoid using times in March, April, November, and October.

I also saw you referenced this in your Docker proposal. I do think that Docker should set the TZ env variable, but I also think we should make the tests as unattached as possible. So let's go with changing the dates to be less likely to hit the DST.

@icambron
Copy link
Member

icambron commented Nov 3, 2017

Done in c900092 and d5d052c. Still some zones with failures, but very few of them.

@icambron icambron closed this as completed Nov 3, 2017
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