code review 7323051: net: change server_test.go so we could see failure messages

net: change server_test.go so we could see failure messages

[brainman, 2013-02-12 06:18:35]

Hello golang-dev@googlegroups.com,

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

[brainman, 2013-02-12 06:19:25]

Found this while investigating user report
https://groups.google.com/d/msg/golang-nuts/AcFOgsqpq48/rx7aS9FQN64J

...
ok  	mime/multipart	0.927s
--- FAIL: TestShutdown (0.11 seconds)
net_test.go:54: 	client Read: 0, EOF
panic: test timed out

goroutine 77 [running]:
testing.alarm()
	D:/src/Go/gotip/go/src/pkg/testing/testing.go:522 +0x43
created by time.goFunc
	D:/src/Go/gotip/go/src/pkg/time/sleep.go:107 +0x44

goroutine 1 [chan receive]:
testing.RunTests(0x401000, 0x666f20, 0x5a, 0x5a, 0x0, ...)
	D:/src/Go/gotip/go/src/pkg/testing/testing.go:423 +0x6c5
testing.Main(0x401000, 0x666f20, 0x5a, 0x5a, 0x6620f8, ...)
	D:/src/Go/gotip/go/src/pkg/testing/testing.go:322 +0x68
main.main()
	C:/Users/Arie/AppData/Local/Temp/go-build498798071/net/_test/_testmain.go:231
+0x80

goroutine 2 [syscall]:
created by runtime.main
	D:/src/Go/gotip/go/src/pkg/runtime/proc.c:232

goroutine 3 [timer goroutine (idle)]:
created by addtimer
	D:/src/Go/gotip/go/src/pkg/runtime/ztime_windows_386.c:73

goroutine 5 [syscall]:
syscall.Syscall6(0x74cd19ee, 0x5, 0x140, 0x30dc5000, 0x30dac508, ...)
	D:/src/Go/gotip/go/src/pkg/runtime/zsyscall_windows_windows_386.c:97 +0x49
syscall.GetQueuedCompletionStatus(0x140, 0x30dc5000, 0x30dac508, 0x30dac500,
0xffffffff, ...)
	D:/src/Go/gotip/go/src/pkg/syscall/zsyscall_windows_386.go:506 +0x7e
net.(*resultSrv).Run(0x30dac410, 0x0)
	D:/src/Go/gotip/go/src/pkg/net/fd_windows.go:145 +0x86
created by net.startServer
	D:/src/Go/gotip/go/src/pkg/net/fd_windows.go:279 +0xde

goroutine 65 [finalizer wait]:
created by runtime.gc
	D:/src/Go/gotip/go/src/pkg/runtime/mgc0.c:1423

goroutine 71 [chan receive]:
net.TestStreamConnServer(0x30df9f00, 0xe)
	D:/src/Go/gotip/go/src/pkg/net/server_test.go:123 +0x45f
testing.tRunner(0x30df9f00, 0x667190, 0x0)
	D:/src/Go/gotip/go/src/pkg/testing/testing.go:310 +0x6e
created by testing.RunTests
	D:/src/Go/gotip/go/src/pkg/testing/testing.go:422 +0x6aa

goroutine 76 [select]:
net.(*ioSrv).ExecIO(0x30dac418, 0x30dfd400, 0x30db3120, 0x0, 0x0, ...)
	D:/src/Go/gotip/go/src/pkg/net/fd_windows.go:230 +0x6b4
net.(*netFD).accept(0x30d60280, 0x441d40, 0x0, 0x0)
	D:/src/Go/gotip/go/src/pkg/net/fd_windows.go:642 +0x357
net.(*TCPListener).AcceptTCP(0x30dd9408, 0x0, 0x0, 0x0)
	D:/src/Go/gotip/go/src/pkg/net/tcpsock_posix.go:232 +0x53
net.(*TCPListener).Accept(0x30dd9408, 0x0, 0x0, 0x0, 0x0, ...)
	D:/src/Go/gotip/go/src/pkg/net/tcpsock_posix.go:242 +0x48
net.runStreamConnServer(0x30df9f00, 0x55ab38, 0x3, 0x30dd92e8, 0x7, ...)
	D:/src/Go/gotip/go/src/pkg/net/server_test.go:196 +0x214
created by net.TestStreamConnServer
	D:/src/Go/gotip/go/src/pkg/net/server_test.go:109 +0x22e
FAIL	net	120.916s
ok  	net/http	12.028s
...

TestStreamConnServer test should not hang indefinitely. And it doesn't print any
errors.

Alex

[dave_cheney.net, 2013-02-12 08:03:34]

Thank you, these suggestions may make it easier to reason about.

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go
File src/pkg/net/server_test.go (left):

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go#o...
src/pkg/net/server_test.go:171: func runStreamConnServer(t *testing.T, net,
laddr string, listening chan<- string, done chan<- int) {
defer close(done)

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go#o...
src/pkg/net/server_test.go:176: done <- 1
delete

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go#o...
src/pkg/net/server_test.go:191: done <- 1
close(done)

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go#o...
src/pkg/net/server_test.go:206: done <- 1
delete

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go
File src/pkg/net/server_test.go (right):

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go#n...
src/pkg/net/server_test.go:100: done := make(chan int, 1)
I'm not sure you need to make this buffered. See below.

[brainman, 2013-02-13 01:40:24]

Hello golang-dev@googlegroups.com, dave@cheney.net (cc:
golang-dev@googlegroups.com),

Please take another look.

[brainman, 2013-02-13 01:40:31]

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go
File src/pkg/net/server_test.go (left):

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go#o...
src/pkg/net/server_test.go:171: func runStreamConnServer(t *testing.T, net,
laddr string, listening chan<- string, done chan<- int) {
On 2013/02/12 08:03:34, dfc wrote:
> defer close(done)

Done.

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go#o...
src/pkg/net/server_test.go:176: done <- 1
On 2013/02/12 08:03:34, dfc wrote:
> delete

Done.

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go#o...
src/pkg/net/server_test.go:191: done <- 1
On 2013/02/12 08:03:34, dfc wrote:
> close(done)

Done.

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go#o...
src/pkg/net/server_test.go:206: done <- 1
On 2013/02/12 08:03:34, dfc wrote:
> delete

Done.

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go
File src/pkg/net/server_test.go (right):

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go#n...
src/pkg/net/server_test.go:100: done := make(chan int, 1)
Agree. Reverted.

[dave_cheney.net, 2013-02-13 03:23:36]

One final comment.

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go
File src/pkg/net/server_test.go (right):

https://codereview.appspot.com/7323051/diff/5001/src/pkg/net/server_test.go#n...
src/pkg/net/server_test.go:317: done := make(chan int, 1)
probably doesn't need to buffered anymore.

[brainman, 2013-02-13 05:10:40]

Hello golang-dev@googlegroups.com, dave@cheney.net (cc:
golang-dev@googlegroups.com),

Please take another look.

[dave_cheney.net, 2013-02-13 05:16:33]

LGTM. Thanks.

[brainman, 2013-02-13 05:17:56]

*** Submitted as https://code.google.com/p/go/source/detail?r=27fc858ccd7b ***

net: change server_test.go so we could see failure messages

R=golang-dev, dave
CC=golang-dev
https://codereview.appspot.com/7323051