code review 7314047: go.net/proxy: make proxy tests able to run with host pr...

go.net/proxy: make proxy tests able to run with host protocol stack

[mikio, 2013-02-06 10:31:39]

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

I'd like you to review this change to
https://code.google.com/p/go.net

[dave_cheney.net, 2013-02-08 12:31:47]

Thank you for this proposal. I don't really know this code, so another round of
reviews is probably in order.

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go
File proxy/proxy_test.go (left):

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#oldcode69
proxy/proxy_test.go:69: wg.Add(1)
oops

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go
File proxy/proxy_test.go (right):

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode28
proxy/proxy_test.go:28: wg := &sync.WaitGroup{}
var wg sync.WaitGroup

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode30
proxy/proxy_test.go:30: go socks5Gateway(t, gateway, endSystem, socks5Domain,
wg)
&wg

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode65
proxy/proxy_test.go:65: wg := &sync.WaitGroup{}
var wg sync.WaitGroup

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode67
proxy/proxy_test.go:67: go socks5Gateway(t, gateway, endSystem, socks5IP4, wg)
&wg

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode99
proxy/proxy_test.go:99: t.Fatalf("io.ReadFull failed: %v", err)
cannot use Fatalf here, this is running in a goroutine.

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode110
proxy/proxy_test.go:110: t.Fatalf("io.ReadFull failed: %v", err)
ditto

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode123
proxy/proxy_test.go:123: t.Fatalf("net.SplitHostPort failed: %v", err)
etc

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode128
proxy/proxy_test.go:128: t.Fatalf("strconv.Atoi failed: %v", err)
etc

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode132
proxy/proxy_test.go:132: t.Fatalf("net.Conn.Write failed: %v", err)
etc

[mikio, 2013-02-09 01:00:32]

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go
File proxy/proxy_test.go (left):

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#oldcode69
proxy/proxy_test.go:69: wg.Add(1)
On 2013/02/08 12:31:47, dfc wrote:
> oops

my bad

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go
File proxy/proxy_test.go (right):

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode28
proxy/proxy_test.go:28: wg := &sync.WaitGroup{}
On 2013/02/08 12:31:47, dfc wrote:
> var wg sync.WaitGroup

Done.

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode30
proxy/proxy_test.go:30: go socks5Gateway(t, gateway, endSystem, socks5Domain,
wg)
On 2013/02/08 12:31:47, dfc wrote:
> &wg

Done.

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode65
proxy/proxy_test.go:65: wg := &sync.WaitGroup{}
On 2013/02/08 12:31:47, dfc wrote:
> var wg sync.WaitGroup

Done.

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode67
proxy/proxy_test.go:67: go socks5Gateway(t, gateway, endSystem, socks5IP4, wg)
On 2013/02/08 12:31:47, dfc wrote:
> &wg

Done.

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode99
proxy/proxy_test.go:99: t.Fatalf("io.ReadFull failed: %v", err)
On 2013/02/08 12:31:47, dfc wrote:
> cannot use Fatalf here, this is running in a goroutine.

Done.

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode110
proxy/proxy_test.go:110: t.Fatalf("io.ReadFull failed: %v", err)
On 2013/02/08 12:31:47, dfc wrote:
> ditto

Done.

https://codereview.appspot.com/7314047/diff/5001/proxy/proxy_test.go#newcode123
proxy/proxy_test.go:123: t.Fatalf("net.SplitHostPort failed: %v", err)
On 2013/02/08 12:31:47, dfc wrote:
> etc

Done.

[mikio, 2013-02-09 01:00:52]

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

Please take another look.

[agl1, 2013-02-09 17:42:52]

LGTM

[mikio, 2013-02-10 01:45:22]

*** Submitted as
https://code.google.com/p/go/source/detail?r=ba1b5eb4f677&repo=net ***

go.net/proxy: make proxy tests able to run with host protocol stack

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