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

Unit tests fail when run in Australia #5017

Closed
brettporter opened this Issue Nov 19, 2013 · 14 comments

Comments

@brettporter
Contributor

brettporter commented Nov 19, 2013

I'm receiving the same error reported in this comment: #3474 (comment)

I'm also in Australia/Sydney.

I was able to skip with --force, but it'd be good to get a clean bill of health for contributing!

Chrome 31.0.1650 (Mac OS X 10.9.0) ngMock TzDate should fake getHours method FAILED
    Expected 4 to be 3.
    Error: Expected 4 to be 3.
        at null.<anonymous> (/Users/brett/scm/github/angular.js/test/ngMock/angular-mocksSpec.js:60:29)
    Expected 1 to be 0.
    Error: Expected 1 to be 0.
        at null.<anonymous> (/Users/brett/scm/github/angular.js/test/ngMock/angular-mocksSpec.js:64:29)
    Expected 22 to match 21.
    Error: Expected 22 to match 21.
        at null.<anonymous> (/Users/brett/scm/github/angular.js/test/ngMock/angular-mocksSpec.js:68:29)
@wizardwerdna

This comment has been minimized.

Contributor

wizardwerdna commented Nov 26, 2013

Perhaps we should add geolocation information to pull requests?

@ghost ghost assigned btford Dec 31, 2013

@btford

This comment has been minimized.

Contributor

btford commented Dec 31, 2013

I get a similar stacktrace when I run the tests in Australia:

(92:86:sɾ˙ɔǝdssʞɔoɯ-ɹɐlnƃuɐ/ʞɔoɯƃu/ʇsǝʇ/sɾ˙ɹɐlnƃuɐ/qnɥʇıƃ/ɯɔs/ʇʇǝɹq/sɹǝsn/) <snoɯʎuouɐ>˙llnu ʇɐ        
˙12 ɥɔʇɐɯ oʇ 22 pǝʇɔǝdxǝ :ɹoɹɹǝ    
˙12 ɥɔʇɐɯ oʇ 22 pǝʇɔǝdxǝ    
(92:46:sɾ˙ɔǝdssʞɔoɯ-ɹɐlnƃuɐ/ʞɔoɯƃu/ʇsǝʇ/sɾ˙ɹɐlnƃuɐ/qnɥʇıƃ/ɯɔs/ʇʇǝɹq/sɹǝsn/) <snoɯʎuouɐ>˙llnu ʇɐ        
˙0 ǝq oʇ 1 pǝʇɔǝdxǝ :ɹoɹɹǝ    
˙0 ǝq oʇ 1 pǝʇɔǝdxǝ    
(92:06:sɾ˙ɔǝdssʞɔoɯ-ɹɐlnƃuɐ/ʞɔoɯƃu/ʇsǝʇ/sɾ˙ɹɐlnƃuɐ/qnɥʇıƃ/ɯɔs/ʇʇǝɹq/sɹǝsn/) <snoɯʎuouɐ>˙llnu ʇɐ        
˙3 ǝq oʇ 4 pǝʇɔǝdxǝ :ɹoɹɹǝ    
˙3 ǝq oʇ 4 pǝʇɔǝdxǝ    
pǝlıɐɟ poɥʇǝɯ sɹnoɥʇǝƃ ǝʞɐɟ plnoɥs ǝʇɐpzʇ ʞɔoɯƃu (0˙9˙01 x so ɔɐɯ) 0561˙0˙13 ǝɯoɹɥɔ

(in all seriousness, i assigned the issue to myself and will take a look at it)

@matsko

This comment has been minimized.

Member

matsko commented Dec 31, 2013

I'm willing to handle this in person if a trip to Australia can be expensed for me :)

@heymatthew

This comment has been minimized.

heymatthew commented Mar 14, 2014

Same issue in NZ.

@IgorMinar

This comment has been minimized.

Member

IgorMinar commented Mar 18, 2014

I don't quite understand why is this the case. The TzDate mock is specifically built to be time-zone agnostic.

We compute the offset based on your local settings and then adjust the date to be the same in all timezones. See:

var localOffset = new Date(timestamp).getTimezoneOffset();
self.offsetDiff = localOffset*60*1000 - offset*1000*60*60;
self.date = new Date(timestamp + self.offsetDiff);

I don't understand why this works fine in all timezones except for Australia and New Zealand. Any ideas?

@brettporter

This comment has been minimized.

Contributor

brettporter commented Mar 18, 2014

@IgorMinar hello from the future! Given the off by one, this just appears to be a DST issue (the tests pass if I were to move to Brisbane where they don't practice DST). Thanks for the pointer, that was a helpful clue. I'll put together a pull request.

@brettporter

This comment has been minimized.

Contributor

brettporter commented Mar 18, 2014

Actually, on further investigation, this doesn't look like a bug in the angular tests - perhaps a native problem. DST started in Australia in 1971, so the UNIX epoch was at 10:00 UTC+10, 1/1/70.

However, in the tests using a negative offset as these ones do gives a timezone as +11 instead of +10. This can be seen in node as well:

$ node
> new Date(0)
Thu Jan 01 1970 10:00:00 GMT+1000 (EST)
> new Date(-36000000)
Thu Jan 01 1970 01:00:00 GMT+1100 (EST)
> new Date(1970, 0, 1)
Thu Jan 01 1970 00:00:00 GMT+1100 (EST)
> new Date(1970, 0, 1, 1, 0, 0)
Thu Jan 01 1970 01:00:00 GMT+1100 (EST)

The results seem to vary wildly under conditions I can't control - Safari gets it wrong for even new Date(0), and some conditions I can't reproduce have some Chrome tabs (but not new ones as in karma) doing it right.

There's a workaround: to change the tests to convert dates that won't become negative. It's a bit of a kludge, but doesn't seem to invalidate the tests - hopefully that's acceptable?

brettporter added a commit to brettporter/angular.js that referenced this issue Mar 18, 2014

test(ngMock): workaround issue with negative timestamps
In some specific timezones and operating systems, it seems that
getTimezoneOffset() can return an incorrect value for negative timestamps, as
described in angular#5017. While this isn't something easily fixed in the mock code,
the tests can avoid that particular timeframe by using a positive timestamp.

Closes angular#5017

brettporter added a commit to brettporter/angular.js that referenced this issue Mar 18, 2014

test(ngMock): workaround issue with negative timestamps
In some specific timezones and operating systems, it seems that
getTimezoneOffset() can return an incorrect value for negative timestamps, as
described in angular#5017. While this isn't something easily fixed in the mock code,
the tests can avoid that particular timeframe by using a positive timestamp.

Closes angular#5017

@IgorMinar IgorMinar closed this in 0c65f1a Mar 19, 2014

IgorMinar added a commit that referenced this issue Mar 19, 2014

test(ngMock): workaround issue with negative timestamps
In some specific timezones and operating systems, it seems that
getTimezoneOffset() can return an incorrect value for negative timestamps, as
described in #5017. While this isn't something easily fixed in the mock code,
the tests can avoid that particular timeframe by using a positive timestamp.

Closes #5017
Closes #6730
@apaprocki

This comment has been minimized.

apaprocki commented Apr 8, 2015

I believe this is due to ES5 15.9.1.8 (https://es5.github.io/#x15.9.1.8).

"The implementation of ECMAScript should not try to determine whether the exact time was subject to daylight saving time, but just whether daylight saving time would have been in effect if the current daylight saving time algorithm had been used at the time. This avoids complications such as taking into account the years that the locale observed daylight saving time year round.

If the host environment provides functionality for determining daylight saving time, the implementation of ECMAScript is free to map the year in question to an equivalent year (same leap-year-ness and same starting week day for the year) for which the host environment provides daylight saving time information. The only restriction is that all equivalent years should produce the same result."

Different JS engines handle dates < 1970 differently when it comes to local timezone adjustment. Some engines map < 1970 to more modern years, which would explain why you are seeing the date have DST applied even though Australia didn't observe DST then.

See: https://bugzilla.mozilla.org/show_bug.cgi?id=351066#c20 and https://bugzilla.mozilla.org/show_bug.cgi?id=1029923

@hasokeric

This comment has been minimized.

hasokeric commented Apr 8, 2015

+1

@detaybey

This comment has been minimized.

detaybey commented Apr 8, 2015

Lol (sorry)

@miglen

This comment has been minimized.

miglen commented Apr 8, 2015

Came here from HN.YC. This is funny! :D lol

@InfoSec812

This comment has been minimized.

InfoSec812 commented Apr 8, 2015

+1 - Best bug report ever!

@jspavlick

This comment has been minimized.

jspavlick commented Apr 8, 2015

+1 Australia got digest looped.

@justjanne

This comment has been minimized.

justjanne commented Apr 9, 2015

It seems to be an issue with the specification, obviously, the question is, how it should be fixed – as the ECMAscript specification states todays DST should be applied to past years, while other programming languages, and especially users, expect a different behaviour.

Either someone would have to write a wrapper around Date that correctly implements previous DST data and fixes therefore the issue, or a different solution needs to be found. I don’t know which would be preferable, but to a user, the current solution is definitely confusing.

gkalpak added a commit to gkalpak/angular.js that referenced this issue Mar 21, 2016

test(TzDate): fix test in Australia
Probably due to implementation differences in browsers for pre-DST period (see
angular#5017 and especially
angular#5017 (comment) for context), some
`TzDate` tests had different behavior on different Timezones/Regions (e.g. failed in Australia,
which started to observe DST in 1971).
Since the used year (`1970`) didn't have any particular significance, this commit fixes the issue
by using a year that is more consistently handled by browsers (`2000`).

Fixes angular#14272

gkalpak added a commit that referenced this issue Mar 21, 2016

test(TzDate): fix test in Australia
Probably due to implementation differences in browsers for pre-DST period (see
#5017 and especially
#5017 (comment) for context), some
`TzDate` tests had different behavior on different Timezones/Regions (e.g. failed in Australia,
which started to observe DST in 1971).
Since the used year (`1970`) didn't have any particular significance, this commit fixes the issue
by using a year that is more consistently handled by browsers (`2000`).

Fixes #14272

Closes #14285

gkalpak added a commit that referenced this issue Mar 21, 2016

test(TzDate): fix test in Australia
Probably due to implementation differences in browsers for pre-DST period (see
#5017 and especially
#5017 (comment) for context), some
`TzDate` tests had different behavior on different Timezones/Regions (e.g. failed in Australia,
which started to observe DST in 1971).
Since the used year (`1970`) didn't have any particular significance, this commit fixes the issue
by using a year that is more consistently handled by browsers (`2000`).

Fixes #14272

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