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

cmd/cover: provide a mechanism to flush out coverage report #31007

Open
matloob opened this issue Mar 22, 2019 · 5 comments

Comments

@matloob
Copy link
Contributor

commented Mar 22, 2019

In some cases, such as golang.org/x/tools/go/analysis/singlechecker.Main, a function might need to call os.Exit/log.Fatal, but want to flush any coverage reports in progress before doing so.

This bug proposes providing a mechanism to flush out the coverage report, so that if there's one in progress, it can be flushed out.

@mvdan

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

/cc @alandonovan @rogpeppe @ianlancetaylor @jayconrod

This seems like a clearly good thing to me - are there any disadvantages? I do wonder how this flushing mechanism would work, though, unless cmd/cover gets special treatment from the os or runtime packages.

If we can more or less agree that this should happen, it would be great to get it into 1.13 and close all the related os.Exit issues for good :)

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2019

I like the idea, but what's the mechanism?

@mvdan

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

Quoting Alan's comment which started this discussion:

What if there were a "cover" package in the standard library that exposed a function to flush out the coverage report, if applicable? Applications could call it prior to their final exit. It's still imperfect because you have to remember to call flush before each exit, but at least the API would then resemble the existing ones for cpu/memory/trace.

Then Roger's comment:

The companion to that would be for a running test binary to be able to request that additional coverage information be added.

So, I think a few clarifications should be made, if I'm understanding the original thread correctly:

  • This would be in a new, importable package, not cmd/cover
  • This wouldn't automatically work with os.Exit, unlike how deferred function works with returns
  • This might require some change in the testing package, as per Roger's comment

Perhaps Alan can clarify, particularly since he didn't reply to Roger's comment.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

See also #30306.

@jayconrod

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

The function that does this currently is coverReport in testing. So I guess we'd basically move coverage to a new package and export that? We'd need to add -coverpkg to go build, too.

What happens if coverage data is still being recorded while that function is writing data out? Is the inconsistency okay? I don't think we currently have a global lock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.