util/log: Avoid logging output on stderr under go test -bench#59054
util/log: Avoid logging output on stderr under go test -bench#59054craig[bot] merged 1 commit intocockroachdb:masterfrom
go test -bench#59054Conversation
|
This change is |
yuzefovich
left a comment
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @knz and @mgartner)
pkg/testutils/skip/skip.go, line 104 at r1 (raw file):
// UnderBench returns true iff a test is currently running under `go // test -bench`. When true, tests should avoid writing data on
super nit: seems like a double space before When.
mgartner
left a comment
There was a problem hiding this comment.
LGTM. If you want you can remove what looks like an unnecessary bit of commented code in test_log_scope.go while you're modifying that file:
cockroach/pkg/util/log/test_log_scope.go
Line 271 in 6a364c2
6a364c2 to
20cd305
Compare
knz
left a comment
There was a problem hiding this comment.
If you want you can remove what looks like an unnecessary bit of commented code in test_log_scope.go
Done
Reviewable status:
pkg/testutils/skip/skip.go, line 104 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
super nit: seems like a double space before
When.
https://www.theatlantic.com/science/archive/2018/05/two-spaces-after-a-period/559304/
|
Thanks to both! bors r=mgartner,yuzefovich |
yuzefovich
left a comment
There was a problem hiding this comment.
The workspace is dirty:
[18:29:30][Step 6/6] --- a/pkg/util/log/BUILD.bazel
[18:29:30][Step 6/6] +++ b/pkg/util/log/BUILD.bazel
[18:29:30][Step 6/6] @@ -44,6 +44,7 @@ go_library(
[18:29:30][Step 6/6] deps = [
[18:29:30][Step 6/6] "//pkg/build",
[18:29:30][Step 6/6] "//pkg/cli/exit",
[18:29:30][Step 6/6] + "//pkg/testutils/skip",
[18:29:30][Step 6/6] "//pkg/util",
[18:29:30][Step 6/6] "//pkg/util/caller",
[18:29:30][Step 6/6] "//pkg/util/encoding/encodingtype",
bors r-
Reviewable status:
pkg/testutils/skip/skip.go, line 104 at r1 (raw file):
Previously, knz (kena) wrote…
https://www.theatlantic.com/science/archive/2018/05/two-spaces-after-a-period/559304/
Oh, interesting, I'll take a look, thanks.
|
Canceled. |
We have found that logging output can interleave with benchmark results, which makes it awkward to run `benchstat` and the like on the output. This change ensures that log.Scope disables stderr logging for benchmarks. Release note: None
20cd305 to
f14e506
Compare
|
ah good catch. Fixed. I'll wait for this to be green before merging. |
|
it's green now bors r=mgartner,yuzefovich |
|
Build succeeded: |
Fixes #57979
We have found that logging output can interleave with benchmark
results, which makes it awkward to run
benchstatand the like on theoutput.
This change ensures that log.Scope disables stderr logging for
benchmarks.
Release note: None