code review 7323044: src: use internal tests if possible

src: use internal tests if possible

If a test can be placed in the same package ("internal"), it is placed
there. This facilitates testing of package-private details. Because of
dependency cycles some packages cannot be tested by internal tests.

[u, 2013-02-11 09:07:11]

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

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

[u, 2013-02-11 09:12:02]

On 2013/02/11 09:07:11, u wrote:
> Hello mailto:golang-dev@googlegroups.com (cc: mailto:r@golang.org),
> 
> I'd like you to review this change to
> https://code.google.com/p/go

Discussion reference:
https://groups.google.com/d/msg/golang-dev/6tD734W7Ofo/WJUspezs4wAJ

[rsc, 2013-02-11 12:44:59]

https://codereview.appspot.com/7323044/diff/5001/src/pkg/runtime/race/race_te...
File src/pkg/runtime/race/race_test.go (left):

https://codereview.appspot.com/7323044/diff/5001/src/pkg/runtime/race/race_te...
src/pkg/runtime/race/race_test.go:12: package race_test
Please revert this change. The race package is an implicit dependency of every
build when using the race detector.

https://codereview.appspot.com/7323044/diff/5001/src/pkg/unicode/utf16/utf16_...
File src/pkg/unicode/utf16/utf16_test.go (left):

https://codereview.appspot.com/7323044/diff/5001/src/pkg/unicode/utf16/utf16_...
src/pkg/unicode/utf16/utf16_test.go:5: package utf16_test
Please revert this change. On Windows, syscall (and therefore testing) depends
on utf16.

[u, 2013-02-11 16:05:06]

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

Please take another look.

[u, 2013-02-16 06:20:12]

Updated to reflect changes in rev 3836bcbafa69.

[mikio, 2013-02-16 07:11:29]

i'm fine with changes for net package. leave for others.

please change the CL description to "src: use internal tests if possible";
I mean, src might be good prefix and starts lowercase after prefix.

https://codereview.appspot.com/7323044/diff/4017/src/pkg/net/conn_test.go
File src/pkg/net/conn_test.go (left):

https://codereview.appspot.com/7323044/diff/4017/src/pkg/net/conn_test.go#old...
src/pkg/net/conn_test.go:4: 
please add a comment something like {conn,packetconn,protoconn}_test.go
are just for API tests across over platforms and will never have a build tag.
fwiw that's the (weak) reason why those threes stay external.

https://codereview.appspot.com/7323044/diff/4017/src/pkg/net/packetconn_test.go
File src/pkg/net/packetconn_test.go (left):

https://codereview.appspot.com/7323044/diff/4017/src/pkg/net/packetconn_test....
src/pkg/net/packetconn_test.go:4: 
ditto

https://codereview.appspot.com/7323044/diff/4017/src/pkg/net/protoconn_test.go
File src/pkg/net/protoconn_test.go (left):

https://codereview.appspot.com/7323044/diff/4017/src/pkg/net/protoconn_test.g...
src/pkg/net/protoconn_test.go:4: 
ditto

[u, 2013-02-16 07:44:28]

On 2013/02/16 07:11:29, mikio wrote:
> i'm fine with changes for net package. leave for others.
> 
> please change the CL description to "src: use internal tests if possible";
> I mean, src might be good prefix and starts lowercase after prefix.
> 
> https://codereview.appspot.com/7323044/diff/4017/src/pkg/net/conn_test.go
> File src/pkg/net/conn_test.go (left):
> 
>
https://codereview.appspot.com/7323044/diff/4017/src/pkg/net/conn_test.go#old...
> src/pkg/net/conn_test.go:4: 
> please add a comment something like {conn,packetconn,protoconn}_test.go
> are just for API tests across over platforms and will never have a build tag.
> fwiw that's the (weak) reason why those threes stay external.
> 
>
https://codereview.appspot.com/7323044/diff/4017/src/pkg/net/packetconn_test.go
> File src/pkg/net/packetconn_test.go (left):
> 
>
https://codereview.appspot.com/7323044/diff/4017/src/pkg/net/packetconn_test....
> src/pkg/net/packetconn_test.go:4: 
> ditto
> 
> https://codereview.appspot.com/7323044/diff/4017/src/pkg/net/protoconn_test.go
> File src/pkg/net/protoconn_test.go (left):
> 
>
https://codereview.appspot.com/7323044/diff/4017/src/pkg/net/protoconn_test.g...
> src/pkg/net/protoconn_test.go:4: 
> ditto

I've updated the CL description and added the comments. Just to clarify should
these three files be internal or external? (i.e. should I revert the changes
made to them).

Thank you

/robin

[mikio, 2013-02-16 08:21:00]

On Sat, Feb 16, 2013 at 4:44 PM,  <r.eklind.87@gmail.com> wrote:

> I've updated the CL description and added the comments.
> Just to clarify

thanks but comments should be;

// This file implements blah blah ...
// ... blah.

> should these three files be internal or external? (i.e. should I revert
> the changes made to them).

i'm fine with either.

[u, 2013-02-16 08:27:21]

Hello golang-dev@googlegroups.com, rsc@golang.org, mikioh.mikioh@gmail.com (cc:
golang-dev@googlegroups.com, r@golang.org),

Please take another look.

[rsc, 2013-02-19 14:52:12]

LGTM

[rsc, 2013-02-19 15:02:09]

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

src: use internal tests if possible

If a test can be placed in the same package ("internal"), it is placed
there. This facilitates testing of package-private details. Because of
dependency cycles some packages cannot be tested by internal tests.

R=golang-dev, rsc, mikioh.mikioh
CC=golang-dev, r
https://codereview.appspot.com/7323044

Committer: Russ Cox <rsc@golang.org>