code review 10587043: net: fix some test bug

net: fix shadowing in test cleanup code

Fixes issue 5785.

[chai2010, 2013-06-26 03:30:16]

Hello golang-dev@googlegroups.com,

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

[dfc, 2013-06-26 04:31:59]

Thank you for your enthusiasm, but I'm not sure what problem this CL fixes. Can
you please explain a little, possibly in issue 5785 what the problem that this
CL fixes.

[chai2010, 2013-06-26 04:47:13]

On 2013/06/26 04:31:59, dfc wrote:
> Thank you for your enthusiasm, but I'm not sure what problem this CL fixes.
Can
> you please explain a little, possibly in issue 5785 what the problem that this
> CL fixes.

sorry for this issue, i have made a mistake.
the timeout_test.go seems good.

but unix_test.go maybe has a hidden issue [1]:

	for _, laddr := range []string{"", testUnixAddr()} {
		...
		defer func() {
				c.Close()
				if la != nil {
					defer os.Remove(laddr)
				}
		}()
		...
	}

in the defer closures, the os.Remove argument `laddr` use the same variable.
just like FAQ[2] said: they are same instance of the variable `laddr`.

---

[1]
http://golang.org/src/pkg/net/unix_test.go?s=3935:4020#L142

[2]
http://golang.org/doc/faq#closures_and_goroutines

[dfc, 2013-08-04 23:07:34]

Yes, that is a potiential problem, please update this cl to add a laddr := laddr
just after the for.

[chai2010, 2013-08-05 00:14:57]

On 2013/08/04 23:07:34, dfc wrote:
> Yes, that is a potiential problem, please update this cl to add a laddr :=
laddr
> just after the for.

Done.

[dfc, 2013-08-05 01:24:50]

LGTM. Please fix the issue title

net: fix shadowing in test cleanup code

and I will submit this. Once this is submitted, issue 5875 will be closed.
Please open a new issue if you find more test problems.

[chai2010, 2013-08-05 01:42:11]

On 2013/08/05 01:24:50, dfc wrote:
> LGTM. Please fix the issue title
> 
> net: fix shadowing in test cleanup code
Done.
> 
> and I will submit this. Once this is submitted, issue 5875 will be closed.
> Please open a new issue if you find more test problems.

[dfc, 2013-08-05 02:00:10]

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

net: fix some test bug

Fixes issue 5785.

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

Committer: Dave Cheney <dave@cheney.net>

[r, 2013-08-05 03:44:25]

"fix some test bug" is just about the worst CL description I have ever
seen. You cited the bug (5785), which is good, but please provide more
context in the CL description itself in future.

[dfc, 2013-08-05 03:52:32]

Hi Rob,

Sorry, this was my fault. I verified that the Author had updated the
title in the CL, but the title of the issue was not updated. I should
have asked them to hg mail the issue again which should have fixed the
issue title.

On Mon, Aug 5, 2013 at 1:44 PM, Rob Pike <r@golang.org> wrote:
> "fix some test bug" is just about the worst CL description I have ever seen.
> You cited the bug (5785), which is good, but please provide more context in
> the CL description itself in future.
>