code review 4968041: time: fix zone during windows test

time: fix zone during windows test
Factor out sleep interrupt.

Fixes issue 1109.

[rsc, 2011-08-25 16:03:30]

Hello brainman (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg

[rsc, 2011-08-25 16:05:33]

(Cross-posting from the other discussion.)

If someone with a Windows machine in California
could run

package main

import (
   "fmt"
   "syscall"
)

func main() {
   var i syscall.Timezoneinformation
   syscall.GetTimeZoneInformation(&i)
   fmt.Printf("%#v\n", i)
}

and send me the output, I will paste it into
zoneinfo_windows.go.  Thanks.

[peterGo, 2011-08-25 16:29:59]

For (UTC-08:00) Pacific Time (US & Canada):

syscall.Timezoneinformation{Bias:480, StandardName:[32]uint16{0x50, 0x61, 0x63,
0x69, 0x66, 0x69, 0x63, 0x20, 0x53, 0x74, 0x61, 0x6e, 0x64, 0x61, 0x72, 0x64, 0x
20, 0x54, 0x69, 0x6d, 0x65, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x
0}, StandardDate:syscall.Systemtime{Year:0x0, Month:0xb, DayOfWeek:0x0, Day:0x1,
 Hour:0x2, Minute:0x0, Second:0x0, Milliseconds:0x0}, StandardBias:0, DaylightNa
me:[32]uint16{0x50, 0x61, 0x63, 0x69, 0x66, 0x69, 0x63, 0x20, 0x44, 0x61, 0x79,
0x6c, 0x69, 0x67, 0x68, 0x74, 0x20, 0x54, 0x69, 0x6d, 0x65, 0x0, 0x0, 0x0, 0x0,
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, DaylightDate:syscall.Systemtime{Year:0x0, Mo
nth:0x3, DayOfWeek:0x0, Day:0x2, Hour:0x2, Minute:0x0, Second:0x0, Milliseconds:
0x0}, DaylightBias:-60}

[rsc, 2011-08-25 16:41:19]

Hello alex.brainman@gmail.com, go.peter.90@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[peterGo, 2011-08-25 17:04:40]

8g  -o _go_.8 format.go sleep.go sys.go tick.go time.go  sys_posix.go
zoneinfo_windows.go
sys_posix.go:22: undefined: syscall.Kill
sys_posix.go:22: undefined: syscall.SIGCHLD

[rsc, 2011-08-25 17:12:39]

Hello alex.brainman@gmail.com, go.peter.90@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[rsc, 2011-08-25 17:12:44]

> sys_posix.go:22: undefined: syscall.Kill
> sys_posix.go:22: undefined: syscall.SIGCHLD

aha!
please try again

[peterGo, 2011-08-25 17:29:17]

INSTALL FAIL time
make[1]: Entering directory `c:/tzwin/src/pkg/time'
make[1]: Leaving directory `c:/tzwin/src/pkg/time'
make[1]: *** No rule to make target `sys_windows.go', needed by `_go_.8'.  Stop.

[rsc, 2011-08-25 17:36:42]

here's sys_windows.go.
i think this CL is too complicated
for hg clpatch to get right.


// Copyright 2011 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package time

import (
	"os"
	"syscall"
)

func sysSleep(t int64) os.Error {
	errno := syscall.Sleep(t)
	if errno != 0 && errno != syscall.EINTR {
		return os.NewSyscallError("sleep", errno)
	}
	return nil
}

// for testing: whatever interrupts a sleep
func interrupt() {
}

[peterGo, 2011-08-25 17:59:26]

$ gotest -v
rm -f _test/time.a
rm -f _test/time.a
gopack grc _test/time.a _gotest_.8
=== RUN  time_test.TestSleep
--- PASS: time_test.TestSleep (0.11 seconds)
=== RUN  time_test.TestAfterFunc
--- PASS: time_test.TestAfterFunc (0.00 seconds)
=== RUN  time_test.TestAfter
--- PASS: time_test.TestAfter (0.10 seconds)
=== RUN  time_test.TestAfterTick
--- PASS: time_test.TestAfterTick (1.00 seconds)
=== RUN  time_test.TestAfterStop
--- PASS: time_test.TestAfterStop (0.20 seconds)
=== RUN  time_test.TestAfterQueuing
--- PASS: time_test.TestAfterQueuing (0.90 seconds)
=== RUN  time_test.TestTicker
--- PASS: time_test.TestTicker (1.20 seconds)
=== RUN  time_test.TestTeardown
--- PASS: time_test.TestTeardown (0.30 seconds)
=== RUN  time_test.TestZoneData
--- PASS: time_test.TestZoneData (0.00 seconds)
=== RUN  time_test.TestSecondsToUTC
--- PASS: time_test.TestSecondsToUTC (0.00 seconds)
=== RUN  time_test.TestNanosecondsToUTC
--- PASS: time_test.TestNanosecondsToUTC (0.00 seconds)
=== RUN  time_test.TestSecondsToLocalTime
--- FAIL: time_test.TestSecondsToLocalTime (0.00 seconds)
        SecondsToLocalTime(0):
          want={Year:1969 Month:12 Day:31 Hour:16 Minute:0 Second:0 Nanosecond:0
 Weekday:3 ZoneOffset:-28800 Zone:PST}
          have={Year:1969 Month:12 Day:31 Hour:16 Minute:0 Second:0 Nanosecond:0
 Weekday:3 ZoneOffset:-28800 Zone:Pacific Standard Time}
        SecondsToLocalTime(1221681866):
          want={Year:2008 Month:9 Day:17 Hour:13 Minute:4 Second:26 Nanosecond:0
 Weekday:3 ZoneOffset:-25200 Zone:PDT}
          have={Year:2008 Month:9 Day:17 Hour:13 Minute:4 Second:26 Nanosecond:0
 Weekday:3 ZoneOffset:-25200 Zone:Pacific Daylight Time}
=== RUN  time_test.TestNanoecondsToLocalTime
--- FAIL: time_test.TestNanoecondsToLocalTime (0.00 seconds)
        NanosecondsToLocalTime(100000000):
          want={Year:1969 Month:12 Day:31 Hour:16 Minute:0 Second:0 Nanosecond:1
00000000 Weekday:3 ZoneOffset:-28800 Zone:PST}
          have={Year:1969 Month:12 Day:31 Hour:16 Minute:0 Second:0 Nanosecond:1
00000000 Weekday:3 ZoneOffset:-28800 Zone:Pacific Standard Time}
        NanosecondsToLocalTime(1221681866300000000):
          want={Year:2008 Month:9 Day:17 Hour:13 Minute:4 Second:26 Nanosecond:3
00000000 Weekday:3 ZoneOffset:-25200 Zone:PDT}
          have={Year:2008 Month:9 Day:17 Hour:13 Minute:4 Second:26 Nanosecond:3
00000000 Weekday:3 ZoneOffset:-25200 Zone:Pacific Daylight Time}
=== RUN  time_test.TestSecondsToUTCAndBack
--- PASS: time_test.TestSecondsToUTCAndBack (0.07 seconds)
=== RUN  time_test.TestNanosecondsToUTCAndBack
--- PASS: time_test.TestNanosecondsToUTCAndBack (0.07 seconds)
=== RUN  time_test.TestRFC3339Conversion
--- PASS: time_test.TestRFC3339Conversion (0.00 seconds)
=== RUN  time_test.TestFormat
--- FAIL: time_test.TestFormat (0.00 seconds)
        UnixDate expected "Wed Feb  4 21:00:57 PST 2009" got "Wed Feb  4 21:00:5
7 Pacific Standard Time 2009"
        RFC822 expected "04 Feb 09 2100 PST" got "04 Feb 09 2100 Pacific Standar
d Time"
        RFC850 expected "Wednesday, 04-Feb-09 21:00:57 PST" got "Wednesday, 04-F
eb-09 21:00:57 Pacific Standard Time"
        RFC1123 expected "Wed, 04 Feb 2009 21:00:57 PST" got "Wed, 04 Feb 2009 2
1:00:57 Pacific Standard Time"
=== RUN  time_test.TestParse
--- FAIL: time_test.TestParse (0.00 seconds)
        UnixDate: bad tz offset: 0 not -28800
        RFC850: bad tz offset: 0 not -28800
        RFC1123: bad tz offset: 0 not -28800
        UnixDate: bad tz offset: 0 not -28800
        RFC850: bad tz offset: 0 not -28800
        RFC1123: bad tz offset: 0 not -28800
=== RUN  time_test.TestRubyParse
--- PASS: time_test.TestRubyParse (0.00 seconds)
=== RUN  time_test.TestFormatAndParse
--- FAIL: time_test.TestFormatAndParse (0.00 seconds)
        error: parsing time "Sun Pacific Daylight Time 1937-07-25T20:18:25-07:00
" as "Mon MST 2006-01-02T15:04:05Z07:00": cannot parse "Pacific Daylight Time 19
37-07-25T20:18:25-07:00" as "MST"
        #1: failed on input -1023568895
=== RUN  time_test.TestParseErrors
--- PASS: time_test.TestParseErrors (0.00 seconds)
=== RUN  time_test.TestNoonIs12PM
--- PASS: time_test.TestNoonIs12PM (0.00 seconds)
=== RUN  time_test.TestMidnightIs12AM
--- PASS: time_test.TestMidnightIs12AM (0.00 seconds)
=== RUN  time_test.Test12PMIsNoon
--- PASS: time_test.Test12PMIsNoon (0.00 seconds)
=== RUN  time_test.Test12AMIsMidnight
--- PASS: time_test.Test12AMIsMidnight (0.00 seconds)
=== RUN  time_test.TestMissingZone
--- PASS: time_test.TestMissingZone (0.00 seconds)
=== RUN  time_test.TestMinutesInTimeZone
--- PASS: time_test.TestMinutesInTimeZone (0.00 seconds)
FAIL

[rsc, 2011-08-25 18:09:32]

Cool, thanks.  This is progress.
It looks like maybe the Windows time zone
code has always done this?
The zone name you sent me was definitely
'Pacific Standard Time' etc but the code
wants 'PST'.  I wonder if we can get the
short version out of the Windows system call?

Russ

[peterGo, 2011-08-25 18:22:58]

Right. If you use the abbreviations, everything works. For example,

var usPacific = syscall.Timezoneinformation{
	Bias: 8 * 60,
	StandardName: [32]uint16{
		'P', 'S', 'T',
//	'P', 'a', 'c', 'i', 'f', 'i', 'c', ' ', 'S', 't', 'a', 'n', 'd', 'a', 'r',
'd', ' ', 'T', 'i', 'm', 'e',
	},
	StandardDate: syscall.Systemtime{Month: 11, Day: 1, Hour: 2},
	DaylightName: [32]uint16{
		'P', 'D', 'T',
//		'P', 'a', 'c', 'i', 'f', 'i', 'c', ' ', 'D', 'a', 'y', 'l', 'i', 'g', 'h',
't', ' ', 'T', 'i', 'm', 'e',
	},
	DaylightDate: syscall.Systemtime{Month: 3, Day: 2, Hour: 2},
	DaylightBias: -60,
}

Then,

$ gotest -v
rm -f _test/time.a
8g  -o _gotest_.8 format.go sleep.go sys.go tick.go time.go  sys_windows.go zone
info_windows.go  internal_test.go
rm -f _test/time.a
gopack grc _test/time.a _gotest_.8
=== RUN  time_test.TestSleep
--- PASS: time_test.TestSleep (0.10 seconds)
=== RUN  time_test.TestAfterFunc
--- PASS: time_test.TestAfterFunc (0.00 seconds)
=== RUN  time_test.TestAfter
--- PASS: time_test.TestAfter (0.10 seconds)
=== RUN  time_test.TestAfterTick
--- PASS: time_test.TestAfterTick (1.00 seconds)
=== RUN  time_test.TestAfterStop
--- PASS: time_test.TestAfterStop (0.20 seconds)
=== RUN  time_test.TestAfterQueuing
--- PASS: time_test.TestAfterQueuing (0.90 seconds)
=== RUN  time_test.TestTicker
--- PASS: time_test.TestTicker (1.20 seconds)
=== RUN  time_test.TestTeardown
--- PASS: time_test.TestTeardown (0.30 seconds)
=== RUN  time_test.TestZoneData
--- PASS: time_test.TestZoneData (0.00 seconds)
=== RUN  time_test.TestSecondsToUTC
--- PASS: time_test.TestSecondsToUTC (0.00 seconds)
=== RUN  time_test.TestNanosecondsToUTC
--- PASS: time_test.TestNanosecondsToUTC (0.00 seconds)
=== RUN  time_test.TestSecondsToLocalTime
--- PASS: time_test.TestSecondsToLocalTime (0.00 seconds)
=== RUN  time_test.TestNanoecondsToLocalTime
--- PASS: time_test.TestNanoecondsToLocalTime (0.00 seconds)
=== RUN  time_test.TestSecondsToUTCAndBack
--- PASS: time_test.TestSecondsToUTCAndBack (0.06 seconds)
=== RUN  time_test.TestNanosecondsToUTCAndBack
--- PASS: time_test.TestNanosecondsToUTCAndBack (0.07 seconds)
=== RUN  time_test.TestRFC3339Conversion
--- PASS: time_test.TestRFC3339Conversion (0.00 seconds)
=== RUN  time_test.TestFormat
--- PASS: time_test.TestFormat (0.00 seconds)
=== RUN  time_test.TestParse
--- PASS: time_test.TestParse (0.00 seconds)
=== RUN  time_test.TestRubyParse
--- PASS: time_test.TestRubyParse (0.00 seconds)
=== RUN  time_test.TestFormatAndParse
--- PASS: time_test.TestFormatAndParse (0.32 seconds)
=== RUN  time_test.TestParseErrors
--- PASS: time_test.TestParseErrors (0.00 seconds)
=== RUN  time_test.TestNoonIs12PM
--- PASS: time_test.TestNoonIs12PM (0.00 seconds)
=== RUN  time_test.TestMidnightIs12AM
--- PASS: time_test.TestMidnightIs12AM (0.00 seconds)
=== RUN  time_test.Test12PMIsNoon
--- PASS: time_test.Test12PMIsNoon (0.00 seconds)
=== RUN  time_test.Test12AMIsMidnight
--- PASS: time_test.Test12AMIsMidnight (0.00 seconds)
=== RUN  time_test.TestMissingZone
--- PASS: time_test.TestMissingZone (0.00 seconds)
=== RUN  time_test.TestMinutesInTimeZone
--- PASS: time_test.TestMinutesInTimeZone (0.00 seconds)
PASS

[rsc, 2011-08-25 18:24:48]

On Thu, Aug 25, 2011 at 14:22,  <go.peter.90@gmail.com> wrote:
> Right. If you use the abbreviations, everything works.

Cool.  The part I'm worried about is that if you're
actually running real code (not gotest) then you
get what the system gives you, and if that is
"Pacific Standard Time", then things trying to
print "PST" are going to be unhappy.

Russ

[peterGo, 2011-08-25 18:49:31]

Submit this CL using the abbreviation and leaving the full name as a comment. It
represents substantial forward progress.

There is no official standard for time zone abbreviations. For example, they are
not unique. For EST. are you in New York or Sidney? For CST are you in Chicago
or China? Therefore Windows, doesn't use them; it uses 32-character string time
zone identifiers and names. Unhappy folks should complain to Microsoft, but they
won't get anywhere.

[rsc, 2011-08-25 18:56:17]

On Thu, Aug 25, 2011 at 14:49,  <go.peter.90@gmail.com> wrote:
> Submit this CL using the abbreviation and leaving the full name as a
> comment. It represents substantial forward progress.
>
> There is no official standard for time zone abbreviations. For example,
> they are not unique. For EST. are you in New York or Sidney? For CST are
> you in Chicago or China? Therefore Windows, doesn't use them; it uses
> 32-character string time zone identifiers and names. Unhappy folks
> should complain to Microsoft, but they won't get anywhere.

It's a bit weird to submit something that only works
in testing but not in real life.  Any tools that call
time.Format and expect an abbreviation back
(even if an ambiguous one) are going to be surprised.

Maybe the right fix for now is to just take the
capital letters?

Russ

[peterGo, 2011-08-25 19:22:58]

On 2011/08/25 18:56:17, rsc wrote:
> Maybe the right fix for now is to just take the
> capital letters?

You could do that, and document it. The tzdata key and the abbreviated Windows
name may differ. Windows programmers won't get what they expect. There is no
right answer.

Dates, Times, and Time Zones
http://msdn.microsoft.com/en-us/library/bb384268.aspx

[rsc, 2011-08-25 19:38:37]

> You could do that, and document it. The tzdata key and the abbreviated
> Windows name may differ. Windows programmers won't get what they expect.
> There is no right answer.

Thanks.  I've changed zoneinfo_windows.go and
uploaded a new copy.

Russ

[peterGo, 2011-08-25 20:47:47]

I'm still having the problem with sys_windows.go. Is sys_posix.Go being renamed
before it's copied?


Index: src/pkg/time/sys_unix.go
===================================================================
rename from src/pkg/time/sys_posix.go
rename to src/pkg/time/sys_unix.go
--- a/src/pkg/time/sys_posix.go
+++ b/src/pkg/time/sys_unix.go
@@ -16,3 +16,8 @@
 	}
 	return nil
 }
+
+// for testing: whatever interrupts a sleep
+func interrupt() {
+	syscall.Kill(os.Getpid(), syscall.SIGCHLD)
+}
Index: src/pkg/time/sys_windows.go
===================================================================
copy from src/pkg/time/sys_posix.go
copy to src/pkg/time/sys_windows.go
--- a/src/pkg/time/sys_posix.go
+++ b/src/pkg/time/sys_windows.go
@@ -16,3 +16,7 @@
 	}
 	return nil
 }
+
+// for testing: whatever interrupts a sleep
+func interrupt() {
+}

[rsc, 2011-08-25 20:50:01]

On Thu, Aug 25, 2011 at 16:47,  <go.peter.90@gmail.com> wrote:
> I'm still having the problem with sys_windows.go. Is sys_posix.Go being
> renamed before it's copied?

probably.  if you want to dig into
hgpatch you're welcome to try to fix it,
but i am happy to leave things be.
the actual submit will use hg directly
so it will not have this problem.

[peterGo, 2011-08-25 21:28:42]

LGTM

C:\tzwin\src\pkg\time>gotest -v
rm -f _test/time.a
rm -f _test/time.a
gopack grc _test/time.a _gotest_.8
=== RUN  time_test.TestSleep
--- PASS: time_test.TestSleep (0.10 seconds)
=== RUN  time_test.TestAfterFunc
--- PASS: time_test.TestAfterFunc (0.01 seconds)
=== RUN  time_test.TestAfter
--- PASS: time_test.TestAfter (0.10 seconds)
=== RUN  time_test.TestAfterTick
--- PASS: time_test.TestAfterTick (1.00 seconds)
=== RUN  time_test.TestAfterStop
--- PASS: time_test.TestAfterStop (0.20 seconds)
=== RUN  time_test.TestAfterQueuing
--- PASS: time_test.TestAfterQueuing (0.90 seconds)
=== RUN  time_test.TestTicker
--- PASS: time_test.TestTicker (1.20 seconds)
=== RUN  time_test.TestTeardown
--- PASS: time_test.TestTeardown (0.30 seconds)
=== RUN  time_test.TestZoneData
--- PASS: time_test.TestZoneData (0.00 seconds)
=== RUN  time_test.TestSecondsToUTC
--- PASS: time_test.TestSecondsToUTC (0.00 seconds)
=== RUN  time_test.TestNanosecondsToUTC
--- PASS: time_test.TestNanosecondsToUTC (0.00 seconds)
=== RUN  time_test.TestSecondsToLocalTime
--- PASS: time_test.TestSecondsToLocalTime (0.00 seconds)
=== RUN  time_test.TestNanoecondsToLocalTime
--- PASS: time_test.TestNanoecondsToLocalTime (0.00 seconds)
=== RUN  time_test.TestSecondsToUTCAndBack
--- PASS: time_test.TestSecondsToUTCAndBack (0.06 seconds)
=== RUN  time_test.TestNanosecondsToUTCAndBack
--- PASS: time_test.TestNanosecondsToUTCAndBack (0.07 seconds)
=== RUN  time_test.TestRFC3339Conversion
--- PASS: time_test.TestRFC3339Conversion (0.00 seconds)
=== RUN  time_test.TestFormat
--- PASS: time_test.TestFormat (0.00 seconds)
=== RUN  time_test.TestParse
--- PASS: time_test.TestParse (0.00 seconds)
=== RUN  time_test.TestRubyParse
--- PASS: time_test.TestRubyParse (0.00 seconds)
=== RUN  time_test.TestFormatAndParse
--- PASS: time_test.TestFormatAndParse (0.32 seconds)
=== RUN  time_test.TestParseErrors
--- PASS: time_test.TestParseErrors (0.00 seconds)
=== RUN  time_test.TestNoonIs12PM
--- PASS: time_test.TestNoonIs12PM (0.00 seconds)
=== RUN  time_test.TestMidnightIs12AM
--- PASS: time_test.TestMidnightIs12AM (0.00 seconds)
=== RUN  time_test.Test12PMIsNoon
--- PASS: time_test.Test12PMIsNoon (0.00 seconds)
=== RUN  time_test.Test12AMIsMidnight
--- PASS: time_test.Test12AMIsMidnight (0.00 seconds)
=== RUN  time_test.TestMissingZone
--- PASS: time_test.TestMissingZone (0.00 seconds)
=== RUN  time_test.TestMinutesInTimeZone
--- PASS: time_test.TestMinutesInTimeZone (0.00 seconds)
PASS

[rsc, 2011-08-25 21:31:43]

yay, thanks

On Thu, Aug 25, 2011 at 17:28,  <go.peter.90@gmail.com> wrote:
> LGTM
>
> C:\tzwin\src\pkg\time>gotest -v
> rm -f _test/time.a
> rm -f _test/time.a
> gopack grc _test/time.a _gotest_.8
> === RUN  time_test.TestSleep
> --- PASS: time_test.TestSleep (0.10 seconds)
> === RUN  time_test.TestAfterFunc
> --- PASS: time_test.TestAfterFunc (0.01 seconds)
> === RUN  time_test.TestAfter
> --- PASS: time_test.TestAfter (0.10 seconds)
> === RUN  time_test.TestAfterTick
> --- PASS: time_test.TestAfterTick (1.00 seconds)
> === RUN  time_test.TestAfterStop
> --- PASS: time_test.TestAfterStop (0.20 seconds)
> === RUN  time_test.TestAfterQueuing
> --- PASS: time_test.TestAfterQueuing (0.90 seconds)
> === RUN  time_test.TestTicker
> --- PASS: time_test.TestTicker (1.20 seconds)
> === RUN  time_test.TestTeardown
> --- PASS: time_test.TestTeardown (0.30 seconds)
> === RUN  time_test.TestZoneData
> --- PASS: time_test.TestZoneData (0.00 seconds)
> === RUN  time_test.TestSecondsToUTC
> --- PASS: time_test.TestSecondsToUTC (0.00 seconds)
> === RUN  time_test.TestNanosecondsToUTC
> --- PASS: time_test.TestNanosecondsToUTC (0.00 seconds)
> === RUN  time_test.TestSecondsToLocalTime
> --- PASS: time_test.TestSecondsToLocalTime (0.00 seconds)
> === RUN  time_test.TestNanoecondsToLocalTime
> --- PASS: time_test.TestNanoecondsToLocalTime (0.00 seconds)
> === RUN  time_test.TestSecondsToUTCAndBack
> --- PASS: time_test.TestSecondsToUTCAndBack (0.06 seconds)
> === RUN  time_test.TestNanosecondsToUTCAndBack
> --- PASS: time_test.TestNanosecondsToUTCAndBack (0.07 seconds)
> === RUN  time_test.TestRFC3339Conversion
> --- PASS: time_test.TestRFC3339Conversion (0.00 seconds)
> === RUN  time_test.TestFormat
> --- PASS: time_test.TestFormat (0.00 seconds)
> === RUN  time_test.TestParse
> --- PASS: time_test.TestParse (0.00 seconds)
> === RUN  time_test.TestRubyParse
> --- PASS: time_test.TestRubyParse (0.00 seconds)
> === RUN  time_test.TestFormatAndParse
> --- PASS: time_test.TestFormatAndParse (0.32 seconds)
> === RUN  time_test.TestParseErrors
> --- PASS: time_test.TestParseErrors (0.00 seconds)
> === RUN  time_test.TestNoonIs12PM
> --- PASS: time_test.TestNoonIs12PM (0.00 seconds)
> === RUN  time_test.TestMidnightIs12AM
> --- PASS: time_test.TestMidnightIs12AM (0.00 seconds)
> === RUN  time_test.Test12PMIsNoon
> --- PASS: time_test.Test12PMIsNoon (0.00 seconds)
> === RUN  time_test.Test12AMIsMidnight
> --- PASS: time_test.Test12AMIsMidnight (0.00 seconds)
> === RUN  time_test.TestMissingZone
> --- PASS: time_test.TestMissingZone (0.00 seconds)
> === RUN  time_test.TestMinutesInTimeZone
> --- PASS: time_test.TestMinutesInTimeZone (0.00 seconds)
> PASS
>
> http://codereview.appspot.com/4968041/
>

[mattn, 2011-08-26 00:22:20]

rsc, thanks to your CL.

http://codereview.appspot.com/4968041/diff/8010/src/pkg/time/sleep_test.go
File src/pkg/time/sleep_test.go (right):

http://codereview.appspot.com/4968041/diff/8010/src/pkg/time/sleep_test.go#ne...
src/pkg/time/sleep_test.go:16: const delay = int64(100e6)
1e9 to be enough

http://codereview.appspot.com/4968041/diff/8010/src/pkg/time/sleep_test.go#ne...
src/pkg/time/sleep_test.go:24: if duration < delay {
probably, inverse.

[brainman, 2011-08-26 00:25:55]

Thank you for fixing this.

Add "Fixes issue 1109." to CL description.

Also remove "NOTEST+=time" from pkg\Makefile.

http://codereview.appspot.com/4968041/diff/8010/src/pkg/time/sys_windows.go
File src/pkg/time/sys_windows.go (right):

http://codereview.appspot.com/4968041/diff/8010/src/pkg/time/sys_windows.go#n...
src/pkg/time/sys_windows.go:21: func interrupt() {
TestSleep passes on windows now. But interrupt function does nothing on windows.
So I wander what are we testing, and what is the point of calling of Interrupt()
during TestSleep? I commented out syscall.Kill inside of linux version of
interrupt and it passes too. It can't be right.

http://codereview.appspot.com/4968041/diff/8010/src/pkg/time/zoneinfo_unix.go
File src/pkg/time/zoneinfo_unix.go (right):

http://codereview.appspot.com/4968041/diff/8010/src/pkg/time/zoneinfo_unix.go...
src/pkg/time/zoneinfo_unix.go:188: func setupTestingZone() {
Shouldn't this live in a zoneinfo_linux_test.go instead? Why did we create this
facility, if we are not going to use ourselves?

http://codereview.appspot.com/4968041/diff/8010/src/pkg/time/zoneinfo_windows.go
File src/pkg/time/zoneinfo_windows.go (right):

http://codereview.appspot.com/4968041/diff/8010/src/pkg/time/zoneinfo_windows...
src/pkg/time/zoneinfo_windows.go:52: z.name = syscall.UTF16ToString(short[:w])
fmt.Println(time.LocalTime()) displays

Fri Aug 26 10:20:10 AUSEST 2011

on Windows now, while on linux

# date
Fri Aug 26 10:21:35 EST 2011
#

But I have no better suggestions. Perhaps, we could hard code little table to
map "Windows zone names" to "unix zone names".

[mattn, 2011-08-26 00:34:58]

>
http://codereview.appspot.com/4968041/diff/8010/src/pkg/time/sys_windows.go#n...
> src/pkg/time/sys_windows.go:21: func interrupt() {
> TestSleep passes on windows now. But interrupt function does nothing on
windows.
> So I wander what are we testing, and what is the point of calling of
Interrupt()
> during TestSleep? I commented out syscall.Kill inside of linux version of
> interrupt and it passes too. It can't be right.

if duration < delay {

is inverse.(probably)
And I'll add the code of interrupt() in another CL.

[mattn, 2011-08-26 02:50:32]

I'm finding way to implement to interrupt sleep via interrupt on windows.

The candidates for implement are using SleepEx/QueueUserAPC.
But QueueUserAPC require thread handle to notify the completion. So the code
will be following. 

----------------------------------------
var mu sync.Mutex
var th []syscall.Handle

func sysSleep(t int64) os.Error {
	mu.Lock()
	th[syscall.GetCurrentThread()] = true
	mu.Unlock()
	defer func() {
		mu.Lock()
		th[syscall.GetCurrent] = false, false
		mu.Unlock()
	}()
	errno := syscall.SleepEx(t, true)
	if errno != 0 && errno != syscall.EINTR {
		return os.NewSyscallError("sleep", errno)
	}
	return nil
}

// for testing: whatever interrupts a sleep
func interrupt() {
	mu.Lock()
	// call QueueUserAPC() to each threads in th.
	defer mu.Unlock()
}
----------------------------------------

This seems very adhoc and redundancy to me. it must use SleepEx not Sleep.
Also define syscall.GetCurrentThread(). And it must handle all thread
handles while calling SleepEx().

There is easy way to do same using chan or WaitGroup (i'm not sure).

I suspect to implement pseudo sysSleep/interrupt without using syscall.XXX.

[mattn, 2011-08-26 11:26:42]

rsc

I noticed that SleepEx/QueueUserAPC should be called from each other threads.
If calling from goroutine(meaning same thread), QueueUserAPC succeeded but not
succeeded to interrupt sleep.

Then I've just gave up to use SleepEx/QueueUserAPC. So I changed implement
of that with using CreateEvent/SetEvent/WaitForSingleObject.

----------------------
package time

import (
	"os"
	"syscall"
	"sync"
)


var mu sync.Mutex
var event = make(map[syscall.Handle]bool)

func sysSleep(t int64) os.Error {
	ev, _ := syscall.CreateEvent(nil, false, false, nil)
	mu.Lock()
	event[ev] = true
	mu.Unlock()
	defer func() {
		mu.Lock()
		event[ev] = false, false
		mu.Unlock()
	}()
	er, errno := syscall.WaitForSingleObject(ev, uint32((t + 1e6 - 1) / 1e6))
	if errno != 0 {
		return os.NewSyscallError("sleep", errno)
	}
	if er == syscall.WAIT_OBJECT_0 {
		return os.NewSyscallError("sleep", syscall.EINTR)
	}
	return nil
}

// for testing: whatever interrupts a sleep
func interrupt() {
	for h, _ := range event {
		syscall.SetEvent(h)
	}
}
----------------------

This may NOT be real Sleep implement, however if you accept,
I'll post this to new CL after the submit of 4968041.

Thanks.

[rsc, 2011-08-26 19:11:48]

On Thu, Aug 25, 2011 at 20:25,  <alex.brainman@gmail.com> wrote:
> TestSleep passes on windows now. But interrupt function does nothing on
> windows. So I wander what are we testing, and what is the point of
> calling of Interrupt() during TestSleep? I commented out syscall.Kill
> inside of linux version of interrupt and it passes too. It can't be
> right.

TestSleep is checking that (1) sleep sleeps for long enough
and (2) it does so even if the process gets interrupted in the
middle (it should sleep again).

If you take out the interrupt, you're no longer checking (2),
so you've essentially turned off that part of the test.
That's why it still passes.

Russ

[rsc, 2011-08-26 19:15:28]

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

time: fix zone during windows test
Factor out sleep interrupt.

Fixes issue 1109.

R=alex.brainman, go.peter.90, mattn.jp
CC=golang-dev
http://codereview.appspot.com/4968041