Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(491)

Issue 11219043: code review 11219043: cmd/go, testing: streamline direct use of test binaries (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 11 months ago by rsc
Modified:
11 years, 11 months ago
Reviewers:
r
CC:
r, golang-dev
Visibility:
Public.

Description

cmd/go, testing: streamline direct use of test binaries Before: $ go test -c -cover fmt $ ./fmt.test -test.covermode=set PASS coverage: 65.1% of statements in strconv $ After: $ go test -c -cover fmt $ ./fmt.test PASS coverage: 65.1% of statements in strconv $ In addition to being cumbersome, the old flag didn't make sense: the cover mode cannot be changed after the binary has been built. Another useful effect of this CL is that if you happen to do $ go test -c -covermode=atomic fmt and then forget you did that and run benchmarks, the final line of the output (the coverage summary) reminds you that you are benchmarking with coverage enabled, which might not be what you want. $ ./fmt.test -test.bench . PASS BenchmarkSprintfEmpty 10000000 217 ns/op BenchmarkSprintfString 2000000 755 ns/op BenchmarkSprintfInt 2000000 774 ns/op BenchmarkSprintfIntInt 1000000 1363 ns/op BenchmarkSprintfPrefixedInt 1000000 1501 ns/op BenchmarkSprintfFloat 1000000 1257 ns/op BenchmarkManyArgs 500000 5346 ns/op BenchmarkScanInts 1000 2562402 ns/op BenchmarkScanRecursiveInt 500 3189457 ns/op coverage: 91.4% of statements $ As part of passing the new mode setting in via _testmain.go, merge the two registration mechanisms into one extensible mechanism (a struct).

Patch Set 1 #

Patch Set 2 : diff -r fd8dcd53e0ec https://code.google.com/p/go/ #

Total comments: 2

Patch Set 3 : diff -r 93e8422d5a38 https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+43 lines, -44 lines) Patch
M src/cmd/go/doc.go View 1 2 2 chunks +5 lines, -6 lines 0 comments Download
M src/cmd/go/test.go View 1 2 3 chunks +13 lines, -5 lines 0 comments Download
M src/cmd/go/testflag.go View 1 3 chunks +3 lines, -7 lines 0 comments Download
M src/pkg/testing/cover.go View 1 2 3 chunks +16 lines, -23 lines 0 comments Download
M src/pkg/testing/testing.go View 1 3 chunks +6 lines, -3 lines 0 comments Download

Messages

Total messages: 3
rsc
Hello r (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/
11 years, 11 months ago (2013-07-12 16:00:38 UTC) #1
r
LGTM https://codereview.appspot.com/11219043/diff/3001/src/cmd/go/test.go File src/cmd/go/test.go (right): https://codereview.appspot.com/11219043/diff/3001/src/cmd/go/test.go#newcode142 src/cmd/go/test.go:142: Implies -cover. while you're here, please change these ...
11 years, 11 months ago (2013-07-12 23:22:10 UTC) #2
rsc
11 years, 11 months ago (2013-07-13 00:40:33 UTC) #3
*** Submitted as https://code.google.com/p/go/source/detail?r=074696c98950 ***

cmd/go, testing: streamline direct use of test binaries

Before:

        $ go test -c -cover fmt
        $ ./fmt.test -test.covermode=set
        PASS
        coverage: 65.1% of statements in strconv
        $

After:

        $ go test -c -cover fmt
        $ ./fmt.test
        PASS
        coverage: 65.1% of statements in strconv
        $

In addition to being cumbersome, the old flag didn't make sense:
the cover mode cannot be changed after the binary has been built.

Another useful effect of this CL is that if you happen to do

        $ go test -c -covermode=atomic fmt

and then forget you did that and run benchmarks,
the final line of the output (the coverage summary) reminds you
that you are benchmarking with coverage enabled, which might
not be what you want.

        $ ./fmt.test -test.bench .
        PASS
        BenchmarkSprintfEmpty	10000000	       217 ns/op
        BenchmarkSprintfString	 2000000	       755 ns/op
        BenchmarkSprintfInt	 2000000	       774 ns/op
        BenchmarkSprintfIntInt	 1000000	      1363 ns/op
        BenchmarkSprintfPrefixedInt	 1000000	      1501 ns/op
        BenchmarkSprintfFloat	 1000000	      1257 ns/op
        BenchmarkManyArgs	  500000	      5346 ns/op
        BenchmarkScanInts	    1000	   2562402 ns/op
        BenchmarkScanRecursiveInt	     500	   3189457 ns/op
        coverage: 91.4% of statements
        $

As part of passing the new mode setting in via _testmain.go, merge
the two registration mechanisms into one extensible mechanism
(a struct).

R=r
CC=golang-dev
https://codereview.appspot.com/11219043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b