code review 163071: time: another bug in SecondsToUTC.

time: another bug in SecondsToUTC.
added random test to look for more.

Fixes issue 363.

[rsc, 2009-12-02 08:20:39]

Hello r (cc: cw),

I'd like you to review the following change.

[cw, 2009-12-02 08:24:00]

LGTM

[cw, 2009-12-02 08:24:46]

On 2009/12/02 08:24:00, cw wrote:
> LGTM

Actually, there is another failure case still:
    978220860 e: Sun Dec 31 00:01:00 UTC 2000   g: Sun Jan  1 00:01:00 UTC 2001

[rsc, 2009-12-02 09:00:29]

Thanks.  Fixed and reuploaded.  Same bug on a different line.
Added your test case and also verified that
running 100000 quickchecks would have caught it.
Now running 10M quickchecks (will take an hour).

[r, 2009-12-02 16:08:51]

http://codereview.appspot.com/163071/diff/1008/1010
File src/pkg/time/time_test.go (right):

http://codereview.appspot.com/163071/diff/1008/1010#newcode93
src/pkg/time/time_test.go:93: cfg := &quick.Config{MaxCount: 10000};
how long does this run? if very long, i'd make it shorter by default and make
the count a flag so you can create 6.out and then run it with a flag.

[rsc, 2009-12-02 16:21:38]

> http://codereview.appspot.com/163071/diff/1008/1010#newcode93
> src/pkg/time/time_test.go:93: cfg := &quick.Config{MaxCount: 10000};
> how long does this run? if very long, i'd make it shorter by default and
> make the count a flag so you can create 6.out and then run it with a
> flag.

It's 200ms.  I agree there should be a way to make this bigger.
There is a flag in testing/quick already but it doesn't quite work.
Will address in a different CL.

Russ

[cw, 2009-12-02 16:22:13]

http://codereview.appspot.com/163071/diff/1008/1010
File src/pkg/time/time_test.go (right):

http://codereview.appspot.com/163071/diff/1008/1010#newcode93
src/pkg/time/time_test.go:93: cfg := &quick.Config{MaxCount: 10000};
On 2009/12/02 16:08:51, r wrote:
> how long does this run? if very long, i'd make it shorter by default and make
> the count a flag so you can create 6.out and then run it with a flag.

FWIW, the way I spotted these originally was to invoke 'data' using exec.Run for
each day counting up and down from epoch.

It's pretty nasty and unix (assumes /bin/date) specific so I never pushed the
tests for that, though I can if you wish (make it bail/slip where not
applicable).

[r2, 2009-12-02 16:25:39]

LGTM

On Dec 2, 2009, at 8:21 AM, Russ Cox wrote:

>> http://codereview.appspot.com/163071/diff/1008/1010#newcode93
>> src/pkg/time/time_test.go:93: cfg := &quick.Config{MaxCount: 10000};
>> how long does this run? if very long, i'd make it shorter by default and
>> make the count a flag so you can create 6.out and then run it with a
>> flag.
> 
> It's 200ms.  I agree there should be a way to make this bigger.
> There is a flag in testing/quick already but it doesn't quite work.
> Will address in a different CL.
> 
> Russ

[rsc, 2009-12-02 16:25:58]

> FWIW, the way I spotted these originally was to invoke 'data' using
> exec.Run for each day counting up and down from epoch.
>
> It's pretty nasty and unix (assumes /bin/date) specific so I never
> pushed the tests for that, though I can if you wish (make it bail/slip
> where not applicable).

Nah, don't worry about it.  I started with that too but
it also assumes GNU date (not BSD date) so it doesn't
belong in a test.  I verified that quick-check would have
found all these bugs, because Date->Seconds is much
easier than Seconds->Date, so the round trip is good.

Russ

[rsc, 2009-12-02 16:28:01]

*** Submitted as http://code.google.com/p/go/source/detail?r=20bb7a7c6765 ***

time: another bug in SecondsToUTC.
added random test to look for more.

Fixes issue 363.

R=r, cw
http://codereview.appspot.com/163071

[cw, 2009-12-02 16:36:56]

LGTM, works here.