golang / vscode-go

Go extension for Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=golang.Go
Other
3.78k stars 727 forks source link

Add option(s) to disable coverage on benchmarks #3422

Open argusdusty opened 2 weeks ago

argusdusty commented 2 weeks ago

Is your feature request related to a problem? Please describe. When coverage is enabled for tests (coverOnSingleTest, coverOnSingleTestFile, coverOnTestPackage), it's also enabled for the corresponding benchmark commands. Cover adds overhead to local code that can bias benchmarks based on how much local vs external code they contain. This can potentially cause confusion or mistakes over which version of a function is faster (which it just did in my case).

While it makes sense to be able to get coverage from benchmark runs if needed, it's annoying to have to disable the test cover settings to get accurate benchmark results, only to toggle them back on to get coverage results from tests.

Describe the solution you'd like There should be an additional setting(s) to enable cover on benchmarks. Preferably default off.

Describe alternatives you've considered One alternative could be to just add a note that "cover on test" includes benchmarks (to somewhat mitigate user confusion about benchmark results) - currently settings documentation only indicates that it runs on Test (Function at cursor/Single File/Package) commands, and doesn't mention the Benchmark commands they also run on. This doesn't resolve the difficulty of having to toggle the coverage settings on/off to get both accurate benchmarks and test coverage, though.

Another alternative could be to disable coverage on benchmarks entirely. This would be more reflective of the current documentation, but leaves users with no way to get coverage of benchmarks (if for some reason someone wants that), and may break existing workflows.

hyangah commented 2 weeks ago

Personally, I think the current behavior is a bug, and I prefer to disable coverage on benchmarks entirely. Other than when optimizing the coverage computation logic itself (in that case, they can always run it from command line), it's hard for me to think about a case where people want to benchmark with coverage on. I may be missing something.