opt: suppress logs in benchmarks#58974
Conversation
|
This change is |
2157c56 to
f6cc69b
Compare
rytaft
left a comment
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, 1 of 1 files at r2.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @mgartner and @RaduBerinde)
pkg/sql/opt/bench/bench_test.go, line 281 at r2 (raw file):
skip.IgnoreLint(t, "Remove this when profiling. Use profile flags above to configure. Sample command line: \n"+ "GOMAXPROCS=1 go test -run TestCPUProfile --logtostderr NONE && go tool pprof bench.test cpu.out",
Just curious, why did you need to get rid of the bench.test param?
As of cockroachdb#57134 passing `-logtostderr=false` as a `TESTFLAG` in benchmarks errs: `flag provided but not defined: -logtostderr`. The preferred method for suppressing logs in tests and benchmarks to is add `defer log.Scope(t).Close(t)` to the top of the test/benchmark (see cockroachdb#57979). This commit uses this new method to suppress logs in optimizer benchmarks. Release note: None
f6cc69b to
a628983
Compare
mgartner
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/bench/bench_test.go, line 281 at r2 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
Just curious, why did you need to get rid of the
bench.testparam?
It's treated by go tool pprof as a file, and that file is not generated by the go test command, only cpu.out is. So go tool pprof errors with: bench.test: open bench.test: no such file or directory. I'm not sure why it was included in this comment in the first place.
rytaft
left a comment
There was a problem hiding this comment.
Reviewed 3 of 3 files at r3.
Reviewable status:
pkg/sql/opt/bench/bench_test.go, line 281 at r2 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
It's treated by
go tool pprofas a file, and that file is not generated by thego testcommand, onlycpu.outis. Sogo tool pproferrors with:bench.test: open bench.test: no such file or directory. I'm not sure why it was included in this comment in the first place.
got it - thanks!
|
bors r=rytaft |
|
Build failed (retrying...): |
|
Build succeeded: |
As of #57134 passing
-logtostderr=falseas aTESTFLAGin benchmarkserrs:
flag provided but not defined: -logtostderr. The preferredmethod for suppressing logs in tests and benchmarks to is add
defer log.Scope(t).Close(t)to the top of the test/benchmark(see #57979).
This commit uses this new method to suppress logs in optimizer
benchmarks.
Release note: None