-
Notifications
You must be signed in to change notification settings - Fork 17.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
proposal: cmd/go: -benchformat option in go test #12826
Comments
One may be concerned that adding Currently there is no struct for a test result template. It would need to be added. I don't see a strong reason to add |
Considering that /cc @adg |
See also #2981.
I think having a -json flag is enough to solve this problem
once and for all.
|
Thanks for mentioning #2981. Assuming implementation would use Even if we concatenate json chunks, parsing them with Also from reading #2981 it seems that adding |
With -json it can still stream the result out as a single
json object for each test/benchmark. Actually, I think
that's the only way to go, because we don't want to
lose progress even if the test binary panics in midway.
Even if adding -json is hard, if we intended to add -json
eventually, I don't think we should add more redundant
mechanisms now. This is different from the go list case
because go list are normally used by shell scripts so
having a -f flag in addition to -json makes sense.
|
Let's continue the discussion about |
During discussion in golang/go#12826 adg@, minux@ and nodir@ decided that always streaming would simplify -json flag design. Also this change makes `testing.Result` internal, which decreases modifications to the public api. Change-Id: I40444bb9b34db53ecf2d6ad606578a874c4bd473 Reviewed-on: https://go-review.googlesource.com/15603 Reviewed-by: Andrew Gerrand <adg@golang.org>
OK. Counter-proposal: let's guarantee that this format will not change. Specifically, I propose that the output format is exactly a sequence of lines containing space-separated fields:
The number of on a particular line is not specified, nor is the space of possible s. This describes all the current output and blesses (instead of forcing rewrites of) tools that parse the output already, like benchcmp and benchstat. /cc @aclements |
I don't have a strong opinion about this, but I'm inclined to think it's unnecessary as long as we simply bless the current format as suggested by @rsc and especially unnecessary if we add -json (and if I had to choose between the two, I would definitely choose -json). Suppose we were to simply bless the current format. What future changes to go test -bench might that cause friction with? I have a few things on my list. I'd like an external tool (e.g., benchstat) to be able to do linearity testing. We could support that by outputting lines for each run-up step. I'd like go test -bench to be able to output many more instances of shorter runs so external tools can compute more robust statistics. Again, go test -bench can just output more instances of the same benchmark line. I've thought about adding custom outputs (# of GCs, max pause, etc). We could support that by adding more (value unit) pairs. |
Formal counter-proposal: #14313. |
Background
Currently
go test -bench .
prints out benchmark results in a certain format, but there is no guarantee that this format will not change. Thus a tool that parses go test output may break if an incompatible change to the output format is made.Also currently there is no good way to distinguish benchmark results from the rest of
go test
output, other than checking that a line starts with"Benchmark"
.Proposal
-benchformat
flag togo test
. Its value is a text/template template withBenchmarkResult
input, similar to-f
flag ingo list
.Proc
toBenchmarkResult
. It will have the value ofruntime.GOMAXPROCS
at the beginning ofB.launch
call.Example:
The text was updated successfully, but these errors were encountered: