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/go: merge coverage profiles from subprocesses #28235

Closed
bcmills opened this issue Oct 16, 2018 · 4 comments
Closed

cmd/go: merge coverage profiles from subprocesses #28235

bcmills opened this issue Oct 16, 2018 · 4 comments
Labels
FrozenDueToAge GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Oct 16, 2018

According to this snippet, initCoverProfile clears the coverage profile on every run:

// Using this function clears the profile in case it existed from a previous run,
// or in case it doesn't exist and the test is going to fail to create it (or not run).

That seems counterproductive when we're collecting coverage from a test that runs a binary multiple times (such as the tests for cmd/go itself), or from a test that invokes itself as a subprocess.

Instead, I suspect that we should use OS-level file locking so that we can merge coverage from multiple invocations.

If we do that, I'm not entirely sure when we should remove the previous profile, though.

(CC: @ianlancetaylor @rsc)

@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Oct 16, 2018
@bcmills bcmills added this to the Unplanned milestone Oct 16, 2018
@bcmills
Copy link
Contributor Author

bcmills commented Jan 17, 2019

CC @julieqiu

@fgm
Copy link

fgm commented Jan 25, 2022

This is also annoying when testing exiting code using a subprocess, as in this technique by @adg https://talks.golang.org/2014/testing.slide#23

@bcmills
Copy link
Contributor Author

bcmills commented Dec 7, 2022

@thanm, is this done automatically with the new coverage support enabled?

@thanm
Copy link
Contributor

thanm commented Dec 8, 2022

At least in the cmd/go case, yes, this is already taken care of by the new implementation, yes.

@bcmills bcmills closed this as completed Dec 8, 2022
@bcmills bcmills modified the milestones: Unplanned, Go1.20 Dec 8, 2022
@golang golang locked and limited conversation to collaborators Dec 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants