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

matloob · 2019-03-22T21:23:44Z

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 · 2019-03-26T22:27:15Z

/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 · 2019-03-26T22:33:26Z

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

mvdan · 2019-03-26T22:40:50Z

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 · 2019-03-27T18:11:02Z

See also #30306.

jayconrod · 2019-03-27T18:38:31Z

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.

albertito · 2020-05-23T00:48:38Z

Hi! I'm also interested on having a mechanism like this, for the same reasons (collecting coverage of binaries during integration tests, which sometimes have to exit with os.Exit(1)/log.Fatal).

Are there any news on this? What's blocking the decision?

Thanks!

gopherbot · 2022-06-14T05:51:24Z

Change https://go.dev/cl/411907 mentions this issue: go/analysis/checker: a go/packages-based driver library

The {single,multi}checker packages provide the main function for a complete application, as a black box. Many users want the ability to customize the analyzer behavior with additional logic, as described in the attached issues. This change creates a new package, go/analysis/checker, that exposes an Analyze pure function---one that avoids global flags, os.Exit, logging, profiling, and other side effects---that runs a set of analyzers on a set of packages loaded (by the client) using go/packages, and presents the graph of results in a form that allows postprocessing. This is just a sketch. API feedback welcome. DO NOT SUBMIT Updates golang/go#30231 Updates golang/go#30219 Updates golang/go#31007 Updates golang/go#31897 Updates golang/go#50265 Updates golang/go#53215 Updates golang/go#53336 Change-Id: I745d319a587dca506564a4624b52a7f1eb5f4751

The {single,multi}checker packages provide the main function for a complete application, as a black box. Many users want the ability to customize the analyzer behavior with additional logic, as described in the attached issues. This change creates a new package, go/analysis/checker, that exposes an Analyze pure function---one that avoids global flags, os.Exit, logging, profiling, and other side effects---that runs a set of analyzers on a set of packages loaded (by the client) using go/packages, and presents the graph of results in a form that allows postprocessing. package checker func Analyze([]*analysis.Analyzer, []*packages.Package, *Options) (*Graph, error) type Graph struct { Roots []*Action } type Action struct { ... } // information about each step func (*Graph) WriteJSONDiagnostics(io.Writer) error func (*Graph) WriteTextDiagnostics(io.Writer, contextLines int) error func (*Graph) All() iter.Seq[*Action] // (was Visit in the proposal) See the example_test.go file for typical API usage. API feedback welcome. This change should have no effect on the behavior of existing programs. Logic changes have been kept to a minimum, so the large diffs are mostly code motion. Auxiliary functions for printing, flags, fixes, and so on has been kept to a minimum so that we can make progress on this change. They will be dealt with in follow-up changes. Detailed list of changes to ease review: analysistest: - Run: we report Load errors to t.Log up front (unless RunDespiteErrors); this was previously in loadPackages. - Run: the Validate call is now done by checker.Analyze. - internal/checker.TestAnalyzer has melted away - the logic to construct the "legacy" Result slice is new. - Result: this type used to be an alias for internal/checker.checker.TestAnalyzerResult (regrettably exposing more than intended); now it is a plain public struct. - check: the logic to check fact expectations is new. - The buildtag and directive analyzers both used a similar hack w.r.t. IgnoredFiles; this hack is now in analysistest. Better ideas welcome. checker: - checker.go is mostly moved from internal/checker/checker.go. - print.go is mostly split from old printDiagnostics in internal/checker/checker.go. - Action.execOnce: the pass.Result/Err logic was simplified using an immediately-applied lambda. - inheritFacts: the comments on package-fact handling are new. - Graph.Visit is "born deprecated". Clients using go1.23 should use Graph.All (iter.Seq). - Example: this is new code. internal/checker: - applyFixes: now accepts only the actions to apply, without calling visitAll over the action graph. The previous code was wrong, a latent bug. Some files outside go/analysis were updated, not out of necessity (it's not a breaking change) but to modernize them (e.g. avoiding analysistest.Result.Pass). Updates golang/go#61324 (new API proposal) Fixes golang/go#53215 (feature proposal) Fixes golang/go#31897 Fixes golang/go#50265 Fixes golang/go#53336 Fixes golang/go#66745 Updates golang/go#30231 Updates golang/go#30219 Updates golang/go#31007 Updates golang/go#66745 Change-Id: I745d319a587dca506564a4624b52a7f1eb5f4751 Reviewed-on: https://go-review.googlesource.com/c/tools/+/411907 Reviewed-by: Robert Findley <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>

adonovan · 2024-11-21T17:56:20Z

Is this issue still relevant since go1.20 added support for coverage of integration tests?
https://go.dev/doc/build-cover

matloob · 2024-12-02T19:46:56Z

I don't think it is. I'll close this issue. I'm guessing that if we needed more functionality from getting coverage of a non-test program the solution would look different in light of the build -cover support and we'd want a different proposal anyways.

matloob mentioned this issue Mar 22, 2019

x/tools/go/analysis/singlechecker: don't use os.Exit in Main #30219

Closed

ALTree added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 24, 2019

seankhliao mentioned this issue Aug 3, 2021

go test coverage rate not work for exec.Command #47515

Closed

bcmills mentioned this issue Mar 2, 2022

cmd/cover: extend coverage testing to include applications #51430

Closed

hyangah mentioned this issue Jun 6, 2022

proposal: x/tools/go/analysis: change singlechecker/multichecker to allow customization of package loading #53215

Closed

This comment was marked as duplicate.

Sign in to view

gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022

mknyszek added this to Go Compiler / Runtime Jul 13, 2022

mknyszek moved this to Triage Backlog in Go Compiler / Runtime Jul 15, 2022

seankhliao added this to the Unplanned milestone Aug 20, 2022

adonovan mentioned this issue Jul 12, 2023

x/tools/go/analysis/checker: an importable analysis driver based on go/packages #61324

Closed

matloob closed this as not planned Won't fix, can't repro, duplicate, stale Dec 2, 2024

github-project-automation bot moved this from Triage Backlog to Done in Go Compiler / Runtime Dec 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

matloob commented Mar 22, 2019

mvdan commented Mar 26, 2019

ianlancetaylor commented Mar 26, 2019

mvdan commented Mar 26, 2019 •

edited

Loading

ianlancetaylor commented Mar 27, 2019

jayconrod commented Mar 27, 2019

albertito commented May 23, 2020

This comment was marked as duplicate.

gopherbot commented Jun 14, 2022

adonovan commented Nov 21, 2024

matloob commented Dec 2, 2024

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

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

Comments

matloob commented Mar 22, 2019

mvdan commented Mar 26, 2019

ianlancetaylor commented Mar 26, 2019

mvdan commented Mar 26, 2019 • edited Loading

ianlancetaylor commented Mar 27, 2019

jayconrod commented Mar 27, 2019

albertito commented May 23, 2020

This comment was marked as duplicate.

gopherbot commented Jun 14, 2022

adonovan commented Nov 21, 2024

matloob commented Dec 2, 2024

mvdan commented Mar 26, 2019 •

edited

Loading