exercism / common-lisp

Exercism exercises in Common Lisp.
https://exercism.org/tracks/common-lisp
MIT License
82 stars 77 forks source link

Unexpected gigasecond results, some tests failing. #39

Closed wobh closed 9 years ago

wobh commented 9 years ago

Reported here: exercism.io/submissions/ff41c29d66b648b49d2cdcffa608293a

Output of tests on example solution as of a few minutes ago in SBCL 1.2.2 for OSX:

To load "lisp-unit":
  Load 1 ASDF system:
    lisp-unit
; Loading "lisp-unit"

FROM-LISP-EPOCH: 1 assertions passed, 0 failed.

FROM-UNIX-EPOCH: 1 assertions passed, 0 failed.

FROM-20110425T120000Z: 1 assertions passed, 0 failed.

FROM-19770613T235959Z: 1 assertions passed, 0 failed.

 | Failed Form: (GIGASECOND:FROM 1959 7 19 12 30 30)
 | Expected (1991 3 27 14 17 10) but saw (1991 3 27 13 17 10)
 |
FROM-19590719T123030Z: 0 assertions passed, 1 failed.

Unit Test Summary
 | 5 assertions total
 | 4 passed
 | 1 failed
 | 0 execution errors
 | 0 missing tests

T

Might as well get this out of the way: it's probably a DST or TZ thing.

verdammelt commented 9 years ago

If I've asked the might Oracle, Wolfram Alpha correctly it seems that it agrees with the test data:

http://www.wolframalpha.com/input/?i=what+is+one+billion+seconds+from+00%3A00%3A00+GMT+Jan+01+1900+

and

http://www.wolframalpha.com/input/?i=what+is+one+billion+seconds+from+12%3A30%3A30+GMT+July+19+1959

verdammelt commented 9 years ago

The tests seem to intend for everything to be in UTC. The example code does not specify time zone on either encode or decode so local timezone is used in both. BUT encode doesn't take DST into account but decode does. (ref: http://www.lispworks.com/documentation/HyperSpec/Body/f_encode.htm#encode-universal-time and http://www.lispworks.com/documentation/HyperSpec/Body/f_dec_un.htm#decode-universal-time respectively).

I hate dealing with time.

verdammelt commented 9 years ago

The reason that those two tests fail is because the example implementation is wrong. The example implementation does everything in "local timezone" not UTC as the tests seem to imply.

Of course when you do that then those tests pass but the other 3 fail. It seems those tests are wrong (again checking with Wolfram Alpha).

verdammelt commented 9 years ago

Made the pull request #40.

However I am assuming it must still be faulty about DST and it only working right now for EDT.

kytrinyx commented 9 years ago

Should I close this or do you want to figure out DST?

verdammelt commented 9 years ago

Please do not close yet. I still worry about the DST (possible) issue.

verdammelt commented 9 years ago

@wobh how did you come up with the expected answers in the tests. I think that will be important for us to figure out if there are still problems.

wobh commented 9 years ago

Ugh. I don't remember. I looked it up in one place or another and did some math to verify my verification. It doesn't surprise me that I could have gotten it wrong on both sides of it. At one time I got all passing tests, though.

verdammelt commented 9 years ago

I don't doubt you had them all passing at one point. What do you think of my 'fix'. Do you think there is still a DST problem lurking? (I only debugged and fixed after our recent DST switch over so I am suspicious). But on the other hand the failure case after DST switchover was the same as reported before so maybe that means that DST is (for once) irrelevant?

wobh commented 9 years ago

Well, my brain is still pretty mucous bound and sleep depped. I'm astonished how fast this you turned this around. All I can say now is that the tests pass for me which was the metric I used originally. I'm comfortable with it until November 1.

verdammelt commented 9 years ago

Sorry to hear that you are en-colded. I am just getting over mine.

I am amused and bemused that I didn't look into this until you created an issue for it... I have been well trained by too many projects using JIRA - can't work on anything if there isn't a ticket for it :)

That being said - I'll close this.

@wobh hope you feel better soon.