code review 6218048: doc/progs: use test/run.go for testing

doc/progs: use test/run.go for testing on Windows
        cgo[1-4].go, go1.go couldn't be tested now
(cgo[1-4].go can only be tested when cgo is enabled, go1.go
contain a list of filenames in the current directory)

[brainman, 2012-05-22 06:19:59]

I do not see how you are going to hook it all to the builder. Lets wait for CL
6220049 to submit, then we can come back here.

Alex

[minux1, 2012-05-22 18:08:43]

On 2012/05/22 06:19:59, brainman wrote:
> I do not see how you are going to hook it all to the builder. Lets wait for CL
> 6220049 to submit, then we can come back here.
Due to constrain of the codereview plugin, I can't change the same file in two
CL.

My plan is to just enable test/run.go based doc/progs test on Windows,
because it can't test cgo*.go properly (no way to skip if cgo is disabled),
so on FreeBSD/Linux/Mac OS X, we still need to resort the bash script.

[brainman, 2012-08-10 01:32:41]

On 2012/05/22 18:08:43, minux wrote:
> 
> My plan is ...

If you want to finish with your plan, I will review it again.

Alex

[minux1, 2012-08-10 01:56:46]

On Fri, Aug 10, 2012 at 9:32 AM, <alex.brainman@gmail.com> wrote:

> On 2012/05/22 18:08:43, minux wrote:
>
>  My plan is ...
>>
>
> If you want to finish with your plan, I will review it again.
>
thanks. the problem is doc/progs/go1.go lists files under the current
directory,
it think this can't be tested with strict byte-by-byte comparing.

should we just ignore it on Windows?

[minux1, 2012-08-10 03:33:00]

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

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

[brainman, 2012-08-10 04:20:05]

You didn't change run.bash. It is still calling $GOROOT/doc/progs/run. Shouldn't
both run,bash and run.bat do the same things?

Alex

http://codereview.appspot.com/6218048/diff/16001/doc/progs/go1.go
File doc/progs/go1.go (right):

http://codereview.appspot.com/6218048/diff/16001/doc/progs/go1.go#newcode2
doc/progs/go1.go:2: // this file will output a list of filenames in cwd, not
suitable for cmpout
I am not saying lets do it, but maybe we could allow for regex matches.

http://codereview.appspot.com/6218048/diff/16001/doc/progs/go1.go#newcode2
doc/progs/go1.go:2: // this file will output a list of filenames in cwd, not
suitable for cmpout
Your previous version (with cmpout) did work, if doc/progs is current directory.
Could you remind me, please, why we could not change directory before we run
run.go?

http://codereview.appspot.com/6218048/diff/16001/src/run.bat
File src/run.bat (right):

http://codereview.appspot.com/6218048/diff/16001/src/run.bat#newcode78
src/run.bat:78: echo.
Perhaps you could also build programs in ..\doc\codewalk like run.bash does.

[minux1, 2012-08-10 04:40:49]

On Fri, Aug 10, 2012 at 12:20 PM, <alex.brainman@gmail.com> wrote:

> You didn't change run.bash. It is still calling $GOROOT/doc/progs/run.
> Shouldn't both run,bash and run.bat do the same things?
>
go1.go and cgo[1-4].go still need to be tested by run.bash

>
> http://codereview.appspot.com/**6218048/diff/16001/doc/progs/**
>
go1.go#newcode2<http://codereview.appspot.com/6218048/diff/16001/doc/progs/go1.go#newcode2>
> doc/progs/go1.go:2: // this file will output a list of filenames in cwd,
> not suitable for cmpout
> Your previous version (with cmpout) did work, if doc/progs is current
> directory. Could you remind me, please, why we could not change
> directory before we run run.go?
>
the reason is it depends on filepath.Walk traversal order, and what's more,
there could be other files (like *.orig) in doc/progs.

>
>
http://codereview.appspot.com/**6218048/diff/16001/src/run.**bat#newcode78<ht...
> src/run.bat:78: echo.
> Perhaps you could also build programs in ..\doc\codewalk like run.bash
>
also in this CL? i think this CL is only for doc/progs.

will add that in another CL.

[minux1, 2012-08-10 04:48:16]

On Fri, Aug 10, 2012 at 12:40 PM, minux <minux.ma@gmail.com> wrote:

> http://codereview.appspot.com/**6218048/diff/16001/doc/progs/**
>>
go1.go#newcode2<http://codereview.appspot.com/6218048/diff/16001/doc/progs/go1.go#newcode2>
>> doc/progs/go1.go:2: // this file will output a list of filenames in cwd,
>> not suitable for cmpout
>> Your previous version (with cmpout) did work, if doc/progs is current
>> directory. Could you remind me, please, why we could not change
>> directory before we run run.go?
>>
> the reason is it depends on filepath.Walk traversal order, and what's more,
> there could be other files (like *.orig) in doc/progs.
>
update: filepath.Walk uses lexical ordering during traversal. but the other
reason
still applies.

[brainman, 2012-08-10 04:59:45]

LGTM

But wait for a day or two before submitting.

Please, put a TODO in run.bat to remember to resolve issue with go1.go and
cgo?.* and change run.bash to use same "go run run.go ..." command.

Thank you.

Alex

[rsc, 2012-08-30 14:43:17]

LGTM but you need a change to run.bash

[minux1, 2012-09-01 08:16:02]

On 2012/08/30 14:43:17, rsc wrote:
> ... but you need a change to run.bash
run.go can't test cgo*.go and go1.go now
can we just add
go run %GOROOT%/test/run.go - ../doc/progs
to run.bash without deleting the existing call to
../doc/prog.run?

[rsc, 2012-09-01 13:56:00]

On Sat, Sep 1, 2012 at 4:16 AM,  <minux.ma@gmail.com> wrote:
> On 2012/08/30 14:43:17, rsc wrote:
>>
>> ... but you need a change to run.bash
>
> run.go can't test cgo*.go and go1.go now
> can we just add
> go run %GOROOT%/test/run.go - ../doc/progs
> to run.bash without deleting the existing call to
> ../doc/prog.run?

I guess so, but I would rather keep run.bat and run.bash in sync. It
sounds like you are saying that even after this CL the Windows build
is missing some tests that happen in the Unix build?

Russ

[minux1, 2012-09-01 14:04:14]

On Sat, Sep 1, 2012 at 9:55 PM, Russ Cox <rsc@golang.org> wrote:

> I guess so, but I would rather keep run.bat and run.bash in sync. It
> sounds like you are saying that even after this CL the Windows build
> is missing some tests that happen in the Unix build?
>
cgo[1-4].go can only be tested on cgo enabled (and not FreeBSD)
builds, our run.go doesn't support conditional testing.
go1.go will output filenames in the current directory, so this can'be
tested by "cmpout"/"run", so i can only "compile" this file (unless run.go
supports regexp matching for cmpout)

[rsc, 2012-09-01 14:22:44]

ok

[minux1, 2012-09-02 19:50:12]

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

doc/progs: use test/run.go for testing on Windows
        cgo[1-4].go, go1.go couldn't be tested now
(cgo[1-4].go can only be tested when cgo is enabled, go1.go
contain a list of filenames in the current directory)

R=golang-dev, alex.brainman, rsc
CC=golang-dev
http://codereview.appspot.com/6218048