code review 8671049: cmd/vet: clean up the test

cmd/vet: clean up the test
- clean up the notypes version of the test so it's quiet
- change the package in the buildtag test to avoid confusing godoc

[r, 2013-04-24 16:39:59]

Hello golang-dev@googlegroups.com,

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

[minux1, 2013-04-24 16:52:52]

Given that the Makefile does nothing more than run a
series of commands, could we use a shell script for
the job?

We don't reqauire make(1) for all.bash before this CL.

What do you think?

[bradfitz, 2013-04-24 16:55:15]

In addition to what Minux said, could we move the test input files to a
"testdata" subdirectory of src/cmd/vet ?



On Wed, Apr 24, 2013 at 9:39 AM, <r@golang.org> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> cmd/vet: test it in the build
> - clean up the notypes version of the test so it's quiet
> - change the package in the buildtag test to avoid confusing godoc
> - add test_notypes to run.bash
>
> Please review this at
https://codereview.appspot.**com/8671049/<https://codereview.appspot.com/8671...
>
> Affected files:
>   M src/cmd/vet/Makefile
>   M src/cmd/vet/test_buildtag_bad.**go
>   M src/run.bash
>
>
> Index: src/cmd/vet/Makefile
> ==============================**==============================**=======
> --- a/src/cmd/vet/Makefile
> +++ b/src/cmd/vet/Makefile
> @@ -5,8 +5,10 @@
>  # Assumes go/types is installed
>  test testshort:
>         go build -tags 'vet_test gotypes'
> -       ../../../test/errchk ./vet -printfuncs='Warn:1,Warnf:1' *.go *.s
> +       ../../../test/errchk ./vet -printfuncs='Warn:1,Warnf:1' test_*.go
> test_*.s
>
>  test_notypes:
>         go build -tags 'vet_test'
> -       ../../../test/errchk ./vet -printfuncs='Warn:1,Warnf:1' *.go *.s
> +       # Only those tests that do not depend on types.
> +       # Excluded: test_print.go
> +       ../../../test/errchk ./vet -printfuncs='Warn:1,Warnf:1'
> test_asm.go test_assign.go test_atomic.go test_buildtag.go
> test_buildtag_bad.go test_deadcode.go test_method.go test_rangeloop.go
> test_structtag.go test_taglit.go test_*.s
> Index: src/cmd/vet/test_buildtag_bad.**go
> ==============================**==============================**=======
> rename from src/cmd/vet/buildtag_bad.go
> rename to src/cmd/vet/test_buildtag_bad.**go
> --- a/src/cmd/vet/buildtag_bad.go
> +++ b/src/cmd/vet/test_buildtag_**bad.go
> @@ -8,4 +8,8 @@
>  // +build @#$ // ERROR "invalid non-alphanumeric build constraint"
>
>  // +build toolate // ERROR "build comment appears too late in file"
> -package main
> +package pkg
> +
> +// This is package 'pkg' rather than 'main' so the erroneous build
> +// tag doesn't end up looking like a package doc for the vet command
> +// when examined by godoc.
> Index: src/run.bash
> ==============================**==============================**=======
> --- a/src/run.bash
> +++ b/src/run.bash
> @@ -143,6 +143,11 @@
>  go test ../test/bench/go1
>  ) || exit $?
>
> +(xcd cmd/vet
> +echo
> +make test_notypes
> +) || exit $?
> +
>  (xcd ../test
>  unset GOMAXPROCS
>  time go run run.go
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[r, 2013-04-24 17:06:11]

You're turning this into a major production. I'd rather not
restructure the build just now.

-rob

[r, 2013-04-24 17:07:30]

Hello golang-dev@googlegroups.com, minux.ma@gmail.com, bradfitz@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[r, 2013-04-24 17:07:54]

Dropped the change to run.bash. PTAL.

-rob


On Wed, Apr 24, 2013 at 10:05 AM, Rob Pike <r@golang.org> wrote:
> You're turning this into a major production. I'd rather not
> restructure the build just now.
>
> -rob

[r, 2013-04-24 17:08:17]

Hello golang-dev@googlegroups.com, minux.ma@gmail.com, bradfitz@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[r, 2013-04-24 17:10:12]

By the way, the test was not broken; there was no bitrot. All this
will get better when go/types is in the main tree and this doublestep
can be removed.

-rob

[bradfitz, 2013-04-24 17:17:35]

Not broken? Why doesn't it work for me?

bradfitz@barbon:~/go/src/cmd/vet$ rm vet
bradfitz@barbon:~/go/src/cmd/vet$ make test_notypes
go build -tags 'vet_test'
../../../test/errchk ./vet -printfuncs='Warn:1,Warnf:1' *.go *.s
BUG: errchk: test_print.go:80: missing expected error: '"arg .hi. for
printf verb %b of wrong type"'
errchk: test_print.go:81: missing expected error: '"arg 2.3 for printf verb
%c of wrong type"'
errchk: test_print.go:82: missing expected error: '"arg 2.3 for printf verb
%d of wrong type"'
errchk: test_print.go:83: missing expected error: '"arg .hi. for printf
verb %e of wrong type"'
errchk: test_print.go:84: missing expected error: '"arg true for printf
verb %E of wrong type"'
errchk: test_print.go:85: missing expected error: '"arg .hi. for printf
verb %f of wrong type"'
errchk: test_print.go:86: missing expected error: '"arg 'x' for printf verb
%F of wrong type"'
errchk: test_print.go:87: missing expected error: '"arg .hi. for printf
verb %g of wrong type"'
errchk: test_print.go:88: missing expected error: '"arg i for printf verb
%G of wrong type"'
errchk: test_print.go:89: missing expected error: '"arg x for printf verb
%o of wrong type"'
errchk: test_print.go:90: missing expected error: '"arg 23 for printf verb
%p of wrong type"'
errchk: test_print.go:91: missing expected error: '"arg x for printf verb
%q of wrong type"'
errchk: test_print.go:92: missing expected error: '"arg b for printf verb
%s of wrong type"'
errchk: test_print.go:93: missing expected error: '"arg 23 for printf verb
%t of wrong type"'
errchk: test_print.go:94: missing expected error: '"arg x for printf verb
%U of wrong type"'
errchk: test_print.go:95: missing expected error: '"arg nil for printf verb
%x of wrong type"'
errchk: test_print.go:96: missing expected error: '"arg 2.3 for printf verb
%X of wrong type"'
errchk: test_print.go:97: missing expected error: '"arg 'x' for printf verb
%g of wrong type"'
errchk: test_print.go:108: missing expected error: '"arg .hi. for \* in
printf format not of type int"'
errchk: test_print.go:110: missing expected error: '"arg s for \* in printf
format not of type int"'
errchk: test_print.go:130: missing expected error: '"no args in Error call"'
errchk: test_print.go:134: missing expected error: '"no args in Error call"'




On Wed, Apr 24, 2013 at 10:09 AM, Rob Pike <r@golang.org> wrote:

> By the way, the test was not broken; there was no bitrot. All this
> will get better when go/types is in the main tree and this doublestep
> can be removed.
>
> -rob
>

[bradfitz, 2013-04-24 17:18:28]

LGTM

I see.

[minux1, 2013-04-24 17:23:06]

LGTM.

[r, 2013-04-24 17:40:36]

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

cmd/vet: clean up the test
- clean up the notypes version of the test so it's quiet
- change the package in the buildtag test to avoid confusing godoc

R=golang-dev, minux.ma, bradfitz
CC=golang-dev
https://codereview.appspot.com/8671049

[r, 2013-04-24 17:41:55]

I'll tidy this up and attach it to the build after Go 1.1, probably by
writing a _test.go file that simulates the shell script  I would use
if we only cared about Unix.

-rob