Skip to content
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

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

Open
argusdusty opened this issue Jun 11, 2024 · 1 comment
Open

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

argusdusty opened this issue Jun 11, 2024 · 1 comment

Comments

@argusdusty
Copy link

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
Copy link
Contributor

hyangah commented Jun 14, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants