code review 10050045: cmd/go: add coverage analysis

cmd/go: add coverage analysis
This feature is not yet ready for real use. The CL marks a bite-sized
piece that is ready for review. TODOs that remain:
        provide control over output
        produce output without setting -v
        make work on reflect, sync and time packages
                (fail now due to link errors caused by inlining)
        better documentation
Almost all packages work now, though, if clumsily; try:
        go test -v -cover=count encoding/binary

[r, 2013-06-10 21:49:26]

Hello golang-dev@googlegroups.com,

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

[remyoudompheng, 2013-06-10 22:25:30]

On 2013/06/10 21:49:26, r wrote:
> Hello mailto:golang-dev@googlegroups.com,
> 
> I'd like you to review this change to
> https://code.google.com/p/go



What are these link errors? are they in the tool or are they toolchain bugs?

[r, 2013-06-10 23:00:03]

Long story. Short version: dependencies of the tested code are
compiled with a function whose inline state changes when coverage
analysis is turned on, and there is an inconsistency that the linker
rejects.

Let's not worry about this yet; it's distracting and rare.

-rob

[gobot, 2013-06-10 23:27:49]

R=rsc (assigned by r)

[rsc, 2013-06-11 05:37:52]

Only doc.go is here. Missing some files?

[r, 2013-06-11 05:53:38]

oh for heaven's sake.

PTAL

[rsc, 2013-06-11 06:05:57]

LGTM

https://codereview.appspot.com/10050045/diff/5001/src/cmd/go/build.go
File src/cmd/go/build.go (right):

https://codereview.appspot.com/10050045/diff/5001/src/cmd/go/build.go#newcode...
src/cmd/go/build.go:1117: return err
The easiest way to make the output handling work is to give cover a -o option
and then use it here and change b.runOut to b.run. The latter knows that any
output is bad output, and so it displays it appropriately.

https://codereview.appspot.com/10050045/diff/5001/src/cmd/go/test.go
File src/cmd/go/test.go (right):

https://codereview.appspot.com/10050045/diff/5001/src/cmd/go/test.go#newcode562
src/cmd/go/test.go:562: if err := writeTestmain(filepath.Join(testDir,
"_testmain.go"), p, ptest.coverVars); err != nil {
I guess this is fine, but it seems like this could go back up to 508 or so where
it was before, and s/ptest./p./

https://codereview.appspot.com/10050045/diff/5001/src/cmd/go/test.go#newcode1008
src/cmd/go/test.go:1008: coverDump()
Fine for now but eventually I think you'll want this in a library that can be
more easily modified, like maybe testing.PrintCover.

[r, 2013-06-11 16:32:51]

will add -o and fix that in a separate CL

[r, 2013-06-11 16:35:13]

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

cmd/go: add coverage analysis
This feature is not yet ready for real use. The CL marks a bite-sized
piece that is ready for review. TODOs that remain:
        provide control over output
        produce output without setting -v
        make work on reflect, sync and time packages
                (fail now due to link errors caused by inlining)
        better documentation
Almost all packages work now, though, if clumsily; try:
        go test -v -cover=count encoding/binary

R=rsc
CC=gobot, golang-dev, remyoudompheng
https://codereview.appspot.com/10050045