Date proto implementation

[LinusU]

This fixes #669

Implemented the rest of the Date prototype.

• Methods to set time (setSeconds, setHours, etc)
• Methods to set date (setFullYear, setMonth, etc)
• Longer constructor (new Date(year, month[, ...]))
• More formats (toLocaleString, toTimeString, etc)
• UTC setters (setUTCHours, setUTCMinutes, etc)
• Timezone support (getTimezoneOffset)
• UTC constructor (Date.UTC(year, month[, ...]))
• Tested in multiple timezones

When running the tests, be sure to try with some different timezones (TZ=Europe/Stockholm make test).

• America/Los_Angelesi (UTC-08:00)
• America/St_Johns (UTC-03:30)
• Asia/Kabul (UTC+04:30)
• Europe/Stockholm (UTC+01:00)
• Hongkong (UTC+08:00)

[rwaldron]

I'm getting errors:

ot ok test/issues/issue-runtime-669.js ............... 31/36
    Command: "node issue-runtime-669.js"
    TAP version 13
    ok 1 undefined: "40" == "40"
    ok 2 undefined: "40" == "40"
    ok 3 undefined: "8" == "8"
    ok 4 undefined: "40" == "40"
    ok 5 undefined: "20" == "20"
    ok 6 undefined: "8" == "8"
    not ok 7 undefined: "1970" == "1969"
    ok 8 undefined: "35" == "35"
    ok 9 undefined: "2" == "2"
    ok 10 undefined: "11" == "11"
    ok 11 undefined: "92" == "92"
    ok 12 undefined: "1992" == "1992"
    ok 13 undefined: "92" == "92"
    ok 14 undefined: "1992" == "1992"
    ok 15 undefined: "2" == "2"
    ok 16 undefined: "11" == "11"
    ok 17 undefined: "92" == "92"
    ok 18 undefined: "1992" == "1992"
    ok 19 undefined: "36" == "36"
    ok 20 undefined: "4" == "4"
    ok 21 undefined: "1992" == "1992"
    ok 22 undefined: "92" == "92"
    ok 23 undefined: "11" == "11"
    ok 24 undefined: "2" == "2"
    ok 25 undefined: "16" == "16"
    ok 26 undefined: "17" == "17"
    ok 27 undefined: "18" == "18"
    ok 28 undefined: "19" == "19"
    ok 29 undefined: "Wed Dec 02 1992" == "Wed Dec 02 1992"
    ok 30 undefined: "Wednesday, December 02, 1992" == "Wednesday, December 02, 1992"
    not ok 31 undefined: "Wed Dec 02 1992 16:00:00 GMT-0500 (EST)" == "Wed Dec 02 1992 16:00:00 GMT+0000 (GMT)"
    ok 32 undefined: "16:00:00" == "16:00:00"
    not ok 33 undefined: "Wed, 02 Dec 1992 21:00:00 GMT" == "Wed, 02 Dec 1992 16:00:00 GMT"
    not ok 34 undefined: "16:00:00 GMT-0500 (EST)" == "16:00:00 GMT+0000 (GMT)"
    not ok 35 undefined: "Wed, 02 Dec 1992 21:00:00 GMT" == "Wed, 02 Dec 1992 16:00:00 GMT"
    ok 36 test/issues/issue-runtime-669.js

    1..36
    # tests 36
    # pass  31
    # fail  5

Run directly:

$ colony test/issues/issue-runtime-669.js
1..35
ok 1 - undefined: "40" == "40"
ok 2 - undefined: "40" == "40"
not ok 3 - undefined: "8" == "3"
ok 4 - undefined: "40" == "40"
ok 5 - undefined: "20" == "20"
not ok 6 - undefined: "8" == "3"
not ok 7 - undefined: "1970" == "1969"
ok 8 - undefined: "35" == "35"
ok 9 - undefined: "2" == "2"
ok 10 - undefined: "11" == "11"
ok 11 - undefined: "92" == "92"
ok 12 - undefined: "1992" == "1992"
ok 13 - undefined: "92" == "92"
ok 14 - undefined: "1992" == "1992"
ok 15 - undefined: "2" == "2"
ok 16 - undefined: "11" == "11"
ok 17 - undefined: "92" == "92"
ok 18 - undefined: "1992" == "1992"
ok 19 - undefined: "36" == "36"
ok 20 - undefined: "4" == "4"
ok 21 - undefined: "1992" == "1992"
ok 22 - undefined: "92" == "92"
ok 23 - undefined: "11" == "11"
ok 24 - undefined: "2" == "2"
ok 25 - undefined: "16" == "16"
ok 26 - undefined: "17" == "17"
ok 27 - undefined: "18" == "18"
ok 28 - undefined: "19" == "19"
ok 29 - undefined: "Wed Dec 02 1992" == "Wed Dec 02 1992"
ok 30 - undefined: "Wednesday, December 02, 1992" == "Wednesday, December 02, 1992"
ok 31 - undefined: "Wed Dec 02 1992 16:00:00 GMT+0000 (GMT)" == "Wed Dec 02 1992 16:00:00 GMT+0000 (GMT)"
ok 32 - undefined: "16:00:00" == "16:00:00"
not ok 33 - undefined: "Wed, 02 Dec 1992 21:00:00 GMT" == "Wed, 02 Dec 1992 16:00:00 GMT"
not ok 34 - undefined: "21:00:00 GMT+0000 (GMT)" == "16:00:00 GMT+0000 (GMT)"
not ok 35 - undefined: "Wed, 02 Dec 1992 21:00:00 GMT" == "Wed, 02 Dec 1992 16:00:00 GMT"

[LinusU]

Funny, I would normally would have caught this since I live in Sweden. Currently on vacation in London thought...

[rwaldron]

Well, the good news is that I'm pretty sure this patch is perfect—so awesome work! I'm double checking a few bits in the spec, but this looks great.

[LinusU]

Okay, I actually did a small misstake in my code. I hard coded GMT+0000 (GMT) at two places, fixing that now.

However, node returns GMT+0000 (GMT) where colony (which uses Lua, which uses C++'s strftime) returns GMT+0000 (UTC). I guess that both are technically correct although I UTC being the preferred one.

I'll update the test to allow both...

[LinusU]

@rwaldron I just pushed a new one, it shouldn't solve all your problems thought... Could you test it and post result? thanks

[rwaldron]

However, node returns GMT+0000 (GMT) where colony (which uses Lua, which uses C++'s strftime) returns GMT+0000 (UTC)

May need to modify the string to match node (in the name of compatibility)

[LinusU]

Just pushed e13b8ab with this fix:

@@ -82,10 +82,15 @@ tap.eq(19, d.getMilliseconds());

 d = new Date(1992, 11, 2, 16, 0, 0);

+function testEqFixTimezoneName (a, b) {
+  tap.eq(a.replace(' (UTC)', ' (GMT)'), b);
+}
+
 tap.eq(d.toDateString(), 'Wed Dec 02 1992');
 tap.eq(d.toLocaleDateString(), 'Wednesday, December 02, 1992');
-tap.eq(d.toLocaleString(), 'Wed Dec 02 1992 16:00:00 GMT+0000 (GMT)');
 tap.eq(d.toLocaleTimeString(), '16:00:00');
 tap.eq(d.toUTCString(), 'Wed, 02 Dec 1992 16:00:00 GMT');
-tap.ok(/16\:00\:00 GMT\+0000 \((GMT|UTC)\)/.test(d.toTimeString()));
 tap.eq(d.toGMTString(), 'Wed, 02 Dec 1992 16:00:00 GMT');
+
+testEqFixTimezoneName(d.toLocaleString(), 'Wed Dec 02 1992 16:00:00 GMT+0000 (GMT)');
+testEqFixTimezoneName(d.toTimeString(), '16:00:00 GMT+0000 (GMT)');

[LinusU]

I think that the problem is that there is no time support added yet, getTimezoneOffset is just hard coded to 0.

When I worked on the patch I thought we simply didn't support time zones and that it might be supported at a later date. I also know that the clock on my tessel in Sweden runs 1 hour wrong (UTC instead of Swedish time).

[rwaldron]

That's not going to work, the code needs produce the same expected output as it would when run from node.

[LinusU]

Okay I think that this is the problem with GMT vs. UTC. Travis CI have there servers timezone set to UTC since it dosen't matter where in the world they are but they always want to use GMT+0000 (no DST as well). Makes sense. On their machines both node and colony returns UTC.

On my machine, currently located in London, OS X has used the location service to determine that I'm in the UK. It has thus set my timezone to GMT. I'm affected by DST.

My tests works on both of these setups since the only difference is the name. But since I don't know where the tests will be run I cannot hard code any of them. My quick function fixed it for my machine and the Travis server but it dosen't really matter. We need a way to ask the system which timezone it's in and then expect that output. That would be a pain though...

A realistic test might be to test that it outputs something between the parentheses.

Further more I don't really know how we can get the other tests to realise that correct output can be any of 16:00:00 GMT+0000, 17:00:00 GMT+0100, 17:21:00 GMT+0121, etc...

One solution would be to test for /[0-9]{2}\:[0-9]{2}\:00 GMT+[0-9]{4} (.+)/.

I think I've lost myself with all this thinking :D any ideas?

[rwaldron]

I think I've lost myself with all this thinking :D any ideas?

Really? I thought that was a pretty good summary of the problem :D

As for ideas, I'm going to ping a few people that know more than I do about it and see what they say.

[LinusU]

Yeah I think that the implementation is alright, it's just the test that are really hard to get right :)

I like the setTime(0) getFullYear() === 1969, but that is correct behaviour...

[LinusU]

Nailed it! Tests now works in Node.js and Colony, in Los Angeles, Hongkong and London!

I now implemented timezone support for the Date class so I need to add some more tests to make sure that it's working all right.

It means that .getTimezoneOffset() no longer always return 0 and the setUTCHours() isn't the same thing as setHours().

[LinusU]

Finally! This Date implementation should be up to par with the v8 one at this point.

I fixed the Date.UTC method and did some bug fixes to the rest of the code.

Everything should work now :beers:

I'll squash and rebase in a minute...

[LinusU]

Okay, everything is squashed into one nice commit and all the tests are working in many timezones. I updated the first post with all the info.

[johnnyman727]

This should fix https://github.com/tessel/runtime/issues/300, https://github.com/tessel/runtime/issues/389, and https://github.com/tessel/runtime/issues/669.

[tm-rampart]

Approved by @johnnyman727. Running tests.

[tm-rampart]

Merge or tests failed.

[rwaldron]

:clap:

[johnnyman727]

It looks like Date() should return a localized string but is instead returning one w/o a timezone.

[rwaldron]

@johnnyman727 not sure what you mean by localized?

When Date is called as a function rather than as a constructor, it returns a String representing the current time (UTC).

The spec defines that as toString, not toLocaleString. The UTC definition:

This function returns a String value. The contents of the String are implementation-dependent, but are intended to represent the Date in the current time zone in a convenient, human-readable form

Is this what you mean? The current time zone?

[johnnyman727]

Yeah, sorry for the confusing language. I meant that Date() should return the date as a string in the current time zone but Colony returns the date as a string in the UTC timezone.

[rwaldron]

Got it. This stuff is not easy to keep straight 0_o

[LinusU]

Correct, can't believe I missed this :D The fix is really easy

--- a/src/colony/lua/colony-js.lua
+++ b/src/colony/lua/colony-js.lua
@@ -1544,7 +1544,7 @@ global.Date = function (this, ...)
     getmetatable(this).date = dateValue
     return this
   else
-    return os.date('!%a %h %d %Y %H:%M:%S GMT%z (%Z)', dateValue / 1e6)
+    return os.date('%a %h %d %Y %H:%M:%S GMT%z (%Z)', dateValue / 1e6)
   end
 end

Rebasing on master now...

[tm-rampart]

Approved by @johnnyman727. Running tests.

[tm-rampart]

Merge or tests failed.

[tm-rampart]

Approved by @johnnyman727. Running tests.

[tm-rampart]

Merge or tests failed.

[johnnyman727]

@LinusU, this PR is failing on a seemingly unrelated hardware test. I'll have to check it out when I get into the office this morning.

[LinusU]

Yeah I was just checking the code for the firmware and couldn't find any link to the Date class... I'll wait :)

[tm-rampart]

Approved by @johnnyman727. Running tests.

[tm-rampart]

Merge or tests failed.

[tm-rampart]

Approved by @johnnyman727. Running tests.

[tm-rampart]

Merge or tests failed.

[tm-rampart]

Approved by @kevinmehall. Running tests.

[johnnyman727]

@LinusU,

That was the most heinous bug I think I've ever seen. For whatever reason, on our test of reading pulses, one pulse was held high a little bit too long. There was something that seemed to be blocking the event queue for 6 milliseconds longer than it should have. I went through your addition and tracked it down to a single function which, when removed, caused the test to pass. But that function was never actually called by the test. The weirder part was, changing the name of the function didn't make the test pass, but changing the contents of the function (which, again, was never called) did!

@kevinmehall had a bunch of theories like a flash sector being worn out or some high demand piece of data being split between two page tables, causing a slighter longer read time.

Ultimately, we resolved the issue by updating the CLI on our testing machine. It was using a modified version of CLI that was tuned to allow better USB performance on Linux. We're still not exactly sure how that fixes the issue (or what exactly it was) but it's fixed now. So :boom:.

[LinusU]

Hehe, that must have been very fun to debug :) :fireworks: :beers: