Navigation Menu

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

Jump to bottom

cmd/go: add test -coverhtml #22964

Open
broady opened this issue Dec 1, 2017 · 8 comments
Open

cmd/go: add test -coverhtml #22964

broady opened this issue Dec 1, 2017 · 8 comments
Labels
DevExp anything around developer experience help wanted Proposal Proposal-Accepted
Milestone
Backlog

Comments

@broady
Copy link
Member

broady commented Dec 1, 2017 •
edited

(maybe this shouldn't be a proposal, it seems trivial enough that we should just do it?)

Inspired by rakyll's suggestion in #16768.

I propose a small change to go test's "coverprofile" flag to output both the binary coverage format as well as HTML. The file extension passed in by the user would switch the output format (".html" for HTML, anything else for the existing behaviour).

This change is simple and highly unlikely to break existing users, unlike #16768, which suggests to add additional flags that overlap with "go tool cover" and can quickly descend into supporting more and more flags.

Before:

go test -coverprofile foo.out && go tool cover -html foo.out -o foo.html && rm foo.out

After:

go test -coverprofile foo.html
@gopherbot gopherbot added this to the Proposal milestone Dec 1, 2017
@broady broady added the DevExp anything around developer experience label Dec 1, 2017
@dlsniper
Copy link
Contributor

dlsniper commented Dec 1, 2017

I propose a to allow -coverprofile to support multiple file names / types. For example -coverprofile p.bin p.html p.json should output binary, html, and json. That way it allows for future extension of formats out of the box and it allows the user / tool to control exactly what gets returned by the tool. What do you think?

@slrz
Copy link

slrz commented Dec 1, 2017

Please don't tie behavioural changes to magic file name interpretation.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2017

Perhaps instead:

go test -coverhtml x.html

Like -json, this flag would only exist in the go command, not in the actual test binaries.

@slrz
Copy link

slrz commented Dec 5, 2017

Better, but this is still the prototypical kind of thing a small shell script (or function) can solve in a more efficient (and probably even more convenient) way.

@rsc rsc modified the milestones: Proposal, Go1.11 Feb 5, 2018
@rsc
Copy link
Contributor

rsc commented Feb 5, 2018

Let's just do this for Go 1.11.

@rsc rsc changed the title proposal: cmd/test: directly output coverage HTML cmd/test: directly output coverage HTML Feb 5, 2018
@rsc rsc changed the title cmd/test: directly output coverage HTML cmd/go: add test -coverhtml Feb 5, 2018
@rsc
Copy link
Contributor

rsc commented Apr 18, 2018

/cc @bcmills

@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Go1.12 Jul 3, 2018
@bcmills bcmills modified the milestones: Go1.12, Go1.13 Jan 18, 2019
@zegl
Copy link
Contributor

zegl commented Feb 20, 2019

I have started a implementation of this feature in zegl@605724f. Before I open a PR I'd like to discuss one thing:

go tool cover -html profile creates a temporary file and opens it in the browser when the -o option is not set.

go/src/cmd/cover/html.go

Lines 84 to 88 in 34fb585

if outfile == "" {
if !browser.Open("file://" + out.Name()) {
fmt.Fprintf(os.Stderr, "HTML output written to %s\n", out.Name())
}
}

Should a "naked" go test -coverhtml be allowed (no value set) and use the same behaviour? Or should -coverhtml always be set to a filename?

@bcmills
Copy link
Contributor

bcmills commented Apr 13, 2019

Should a "naked" go test -coverhtml be allowed (no value set) and use the same behaviour?

Probably yes.

CC @jayconrod

@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DevExp anything around developer experience help wanted Proposal Proposal-Accepted
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
9 participants
@broady @zegl @rsc @andybons @dlsniper @ianlancetaylor @bcmills @slrz @gopherbot