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

testing: coverprofile can't profile coverage of test cases themselves #53508

Open
seebs opened this issue Jun 22, 2022 · 6 comments
Open

testing: coverprofile can't profile coverage of test cases themselves #53508

seebs opened this issue Jun 22, 2022 · 6 comments
Labels
NeedsInvestigation
Milestone

Comments

@seebs
Copy link
Contributor

@seebs seebs commented Jun 22, 2022

What version of Go are you using (go version)?

1.18

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

N/A, but Mac or Linux both.

What did you do?

Wrote test cases with non-trivial infrastructure and used -coverprofile.

What did you expect to see?

Coverage information for my test code and test infrastructure.

What did you see instead?

Coverage information only for the package being tested.

The issue here is that in some cases, tests may reasonably want some non-trivial infrastructure or setup code, which isn't logically relevant to the package outside of its unit tests, but which is complex enough that I want to be carefully verifying that code too, not just the overall results of unit tests using it. Being able to confirm/deny that the test code is running as expected, and not skipping things early or happening to hit only happy paths in the test logic, would be useful for diagnosing this.

@robpike
Copy link
Contributor

@robpike robpike commented Jun 23, 2022

Can't you put that information into the package, in the manner of "export_test.go" files, and cover it that way? Accessing this data should not require changes to go test, and it might be unwise to go down that route.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 23, 2022

CC @thanm

@thanm
Copy link
Contributor

@thanm thanm commented Jun 23, 2022

I would have to agree with @robpike here, it seems problematic to add a knob/option that would expose *_test.go code to coverage instrumentation-- potentially very confusing to users, and it would complicate reporting.

tests may reasonably want some non-trivial infrastructure or setup code

As previously suggested, the best option would be to take this non-trivial infrastructure / setup code and add it to a separate package. You can then write unit tests against it and get your coverage that way.

Here's an example of a similar change in the Go repo:

https://go-review.googlesource.com/c/go/+/364036

which takes a chunk of test infrastructure code and moves it out into a separate package (doing this also enables reuse of the infrastructure in other package tests).

@seebs
Copy link
Contributor Author

@seebs seebs commented Jun 23, 2022

So, I had thought of the export_test.go strategy, and I've done it in the past, but I would rather not have the extra code in the package at all outside of test builds. I guess that's not necessarily worse than reworking some part of the test code, but my thought is, changes to how testing works only affect testing, actually adding this to the package code, even unexported, makes the package itself more complex and makes it easier for it to have holes. I like the assurance that non-test usage can't accidentally hit this code.

In this particular case, the setup code would not normally be exportable, or capable of being in another package -- it wants to interact with package internals.

@thanm
Copy link
Contributor

@thanm thanm commented Jun 23, 2022

Would it help to use an "internal" directory?

@seebs
Copy link
Contributor Author

@seebs seebs commented Jun 23, 2022

I don't think "internal" helps much? I want something that is actually inside this package and can freely interact with package internals. But also, the infrastructure I want is inherently test-only. Like, it takes testing.TB as a parameter, uses tb.Cleanup(...), and so on. So I actually have a pretty strong preference for not having it even exist in non-test cases, because I don't want to suck in all of testing for non-test builds.

(I also sometimes, but not always, want exported symbols in foo_test.go to be accessible to other packages in their _test.go files, but not outside of test builds.)

@cagedmantis cagedmantis added the NeedsInvestigation label Jun 24, 2022
@cagedmantis cagedmantis added this to the Backlog milestone Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation
Projects
None yet
Development

No branches or pull requests

5 participants