code review 90930044: testing: write profiles on failure

testing: write profiles on failure

Fixes issue 7901.

[r, 2014-04-30 11:08:41]

https://codereview.appspot.com/90930044/diff/20001/src/pkg/testing/testing.go
File src/pkg/testing/testing.go (right):

https://codereview.appspot.com/90930044/diff/20001/src/pkg/testing/testing.go...
src/pkg/testing/testing.go:440: after()
Is the premise here correct? We don't run benchmarks if the tests fails, either,
and that seems right.

[rsc, 2014-05-02 16:02:02]

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

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

[rsc, 2014-05-02 16:03:50]

That's a fair point about not running benchmarks, but I think that's a
little different. Maybe the test is failing because there are unexpected
allocations - writing the profile lets you see where they are. Maybe the
test is failing because it takes too long - writing the cpu profile lets
you see where the time goes. Profiles are debugging information; benchmark
results are (usually) not.

Also, without the after call here, we write a partial CPU profile, which is
much more confusing than a complete one.

Russ

[r, 2014-05-02 20:20:32]

LGTM
please add a clause to go1.3.html

[rsc, 2014-05-09 16:18:39]

added 1 line to go1.3.html:

<li>
The <a href="/pkg/testing/"><code>testing</code></a> package now
diagnoses tests that call <code>panic(nil)</code>, which are almost always
erroneous.
Also, tests now write profiles (if invoked with profiling flags) even on
failure. <<<< THIS ONE
</li>

[rsc, 2014-05-09 16:18:55]

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

testing: write profiles on failure

Fixes issue 7901.

LGTM=r
R=r
CC=golang-codereviews
https://codereview.appspot.com/90930044