-
Notifications
You must be signed in to change notification settings - Fork 18.1k
cmd/go: -coverpkg=all is not useful #10271
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
Comments
Working as intended. The word "all" is known to the go command and it's the go command that's interpreting it, not cover. It would be confusing to change its meaning here, I believe. |
What is the intention here? What does it mean and how is it useful to build 10 packages but cover all packages? |
The current behavior makes absolutely no sense to me. What am I missing? |
Perhaps it's time to introduce -coverpkg=deps option, which means
all packages depended by the packages on the command line.
|
A new mode would work for me. But I still don't see the intention behind current -coverpkg=all behavior and how it can be useful. |
Your complaint is like complaining that "cc *" fails because some of the files aren't .c files. "all" is a wildcard and it means the same thing everywhere. I'm reluctant to add more wildcards to the cover tool's support in the go command, which is too complex already. And even if I weren't, I wouldn't know which ones to add. "deps" sounds ok but means we will cover things like runtime and reflect, which would be pointless. Solution: List the packages you care about. They're easy to find and easy to specify. Don't use "all". |
No, I am complaining about a different thing. I am complaining how the packages in -coverpkg are used. The documentation says: So I would assume that whatever -coverpkg value is or is expanded to, only an intersection of the list and the packages that would be built otherwise should matter. |
@robpike So, |
I agree with the disposition of this issue at the time given the go command at the time, but the answer is different today. We just introduced using the package list syntax as a filter language (for example, go build -gcflags=net/...=-N myprog to build myprog with optimization disabled only in the net packages). Now that package lists as matchers is an established part of the go command, it makes sense for -coverpkg to be interpreted as filters as well, and then -coverpkg=all means "all the packages being built anyway" and not "all the packages in the world". I will send a CL doing this. |
If you run go test -coverpkg=all fmt one possible interpretation is that you want coverage for all the packages involved in the fmt test, not all the packages in the world. Because coverpkg was previously defined as a list of packages to be loaded, however, it meant all packages in the world. Now that the go command has a concept of package notation being used as a matching filter instead of a direct enumeration, apply that to -coverpkg, so that -coverpkg=all now has the more useful filter interpretation. Fixes golang#10271. Fixes golang#21283. Change-Id: Iddb77b21ba286d3dd65b62507af27e244865072d Reviewed-on: https://go-review.googlesource.com/76876 Run-TryBot: Russ Cox <[email protected]> Reviewed-by: David Crawshaw <[email protected]>
Fixed by CL 76876 (e33794f). Not sure why it was not closed. |
tries to build ALL packages that you have in GOROOT and GOPATH. I have tons of stuff there, significant portion of which does not build for various reasons. So the build either fails or takes enormous amount of time and produces tons of "warning: no packages being tested depend on something".
-coverpkg=all should mean "all packages used by the test" rather than "all packages". Note that -coverpkg=all is useful for any kind of coverage-guided fuzzing, and in this case you want to instrument all packages used by the program.
Below is a temporal patch that I use as a workaround. Note that I also exclude "runtime" as it is not useful for coverage-based fuzzing -- e.g. a suddenly triggered GC increases coverage; or receive from empty/non-empty chan gives different coverage; or creation of a goroutine with empty/non-empty local goroutine header cache gives different coverage, etc.
I don't know what is the right solution here: on one hand excluding runtime from all looks illogical; on the other hand if runtime is included into all, -coverpkg=all again becomes useless for coverage-guided fuzzing.
The text was updated successfully, but these errors were encountered: