Skip to content

Test coverage on PR #41

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

Closed
masch opened this issue Jun 28, 2020 · 10 comments
Closed

Test coverage on PR #41

masch opened this issue Jun 28, 2020 · 10 comments
Labels
enhancement New feature or request

Comments

@masch
Copy link

masch commented Jun 28, 2020

Hi!

Have you ever considered to add a Github action on PR to messure the test coverage of it?

I love your golang template project!
Thanks!

@masch masch changed the title ] [Quesion] Test coverage on PR Jun 28, 2020
@masch masch changed the title [Quesion] Test coverage on PR [Question] Test coverage on PR Jun 28, 2020
@pellared pellared added enhancement New feature or request question Further information is requested labels Jun 29, 2020
@pellared pellared changed the title [Question] Test coverage on PR Test coverage on PR Jun 29, 2020
@pellared
Copy link
Member

pellared commented Jun 29, 2020

@masch
First of all, thanks a lot for both the question and feedback.

Could you please try to specify how it should behave to avoid misunderstanding.
Alternatively (or even additionally) you can also send some example project where it is done.

Side notes regarding code coverage in general.

Some time ago I was thinking of creating to add a coverage badge, but I found that it would probably require to add a dependency + manual configuration with a service like https://coveralls.io/ or https://codecov.io/ which I am not sure if it is a good idea to couple this template to such services. Moreover, I was unable to find which one should I use.

Additionally, the "default" test coverage shows only coverage for the package being tested. It does not show coverage of dependant packages. For some most developers, it should be OK, however, this is only my assumption. It depends on the test strategy and application/library design as well. We could use -coverpkg but there is one annoying issue with it: golang/go#23883.

@masch
Copy link
Author

masch commented Jul 1, 2020

You are totally right, measuring test coverage is a little tricky on Golang. As a internal convention, we do unit test on every package and measure them package by package. Also we are integration/functional test in order to increase the changes to catch a e2e bug. After the tests run fine, we have an internal tool that takes the output from the go test and then convert it to some internal coverage report. Too much fixed for the company that I work.

In the past, I used to configure my personal projects with Codedov but also it has the down side of multiple configuration by project.

So I was asking my self, What is a good and generic solution to use on this templates project?
I don't have a good answer, I was asking to know what is your opinion on this kind of tool and how could I help in order to work on it?

@pellared
Copy link
Member

pellared commented Jul 1, 2020

Analysis

I think that there are two problematic things that can be distinguished here.

Executing go test with test coverage

I think that we can update the test target to something like:

go test -race -coverpkg=./... -covermode=atomic -coverprofile=coverage.out ./...

And a new make target to view the coverage it in browser:

go tool cover -html=coverage.out`

What do you think?

Where to output the test coverage result

Right now I see the following options:

HTML artifact

Create HTML output created using something like go tool cover -html=coverage.out -o coverage.html and put coverage.html as build artifact so that it can be downloaded and analyzed locally.

Benefits:

  1. Can be used even locally (new make target) to analyze coverage during development
  2. No initialization steps required after "using" the template

Drawbacks:

  1. https://github.com/actions/upload-artifact always zips the artifacts. Lots of steps if someone wants to check the coverage when e.g. reviewing a PR ❓ TO BE VERFIED
  2. Lacks historical analysis, comparisons, graphs, PR reports etc.

Codecov

An example project using Codecov: https://github.com/goreleaser/goreleaser.
Example PR report thanks to codecov-action: goreleaser/goreleaser#1503 (comment).

Benefits:

  1. Historical analysis, comparisons, graphs etc
  2. PR reports
  3. Free not only for public repos but also private repos with up to 5 users

Drawbacks:

  1. Requires setup after "using" the template
  2. Not suited for local development ❓ TO BE VERFIED

Coveralls

An example project using Coveralls: https://github.com/mattn/goveralls.
Example PR report: mattn/goveralls#169 (comment). ❓ No idea how it was configured

Similar benefits and drawbacks to Codecov.

Next steps

What I propose for now is that I can try making a PR for "Executing go test with test coverage".

In the meantime, you could focus on "Where to output the test coverage result". You can compare HTML report in artifacts vs Codecov and Coveralls in practice on some fork(s).

I think that there is nothing against having both the coverage in HTML file for local development and Codecov or Coveralls at the same time. My first impression (but without configuring it myself) is that Codecov seems to be the way to better and also has a better pricing plan than Coveralls and is also more robust than plain HTML output in artifacts. Also, I feel that the setup should be easy. However I suspect that most of the steps are still needed to be done manually. I think we will possibility end up with a FAQ how to setup Codecov for a new project created using this template. But these are only my guesses and I would prefer if we have some real experience here.

Please feel free to comment, agree and disagree. Your feedback, help and experience are more than welcome.

@masch
Copy link
Author

masch commented Jul 2, 2020

I think it a greet start from you deeply research, nice work. Here are my comments:

Executing go test with test coverage

Yeah, it a great use go tool cover with the html result for local environment, I use it a lot before I commit a PR.
I think the parameters options are quite enough to get a grate report and especially the -race flag, It catches a lot of tricky 🪲.

Also as I'm a big fun of the use of terminal, I also use goverreport tool to get an easy test measure. Do you know it?
I would love to have that kind of report with a Github action on PR's comments.

HTML artifact

I didn't know it, so from what I red it could be a good output of test result, but personally I prefer a PR's comment like Codecov does it. But if a user doesn't want to use an external tool, it will be very useful.

Codecov vs Coveralls

Both tools and test coverage mechanism are great. I know Coverall but I never use it on my projects, but I use a lot Codecov. So in my personal experience, I would prefer the use of Codecov. The heat map on PR's comments and the UI on their web are better than Coveralls.
As you mentioned, I think the use of this tools should be optional on the project template with a FAQ how to setup it on a new project.

Next steps

I think it's a great start the one that you have proposed it, If you want to I could do a POC with it.

@pellared
Copy link
Member

pellared commented Jul 2, 2020

Can you please review: #43

Doing a PoC with Codecov would help me 😉

Thanks for sharing goverreport with me. However, I do not know how I could integrate it right now.

Currently I think of following improvements for this repo:

  1. Integration with Codecov (at least FAQ section how to do it if it would not make sense to automate it - maybe provide some example repository where it is configured as reference) - DONE
  2. Integraton with Dependabot. Now it is nicley integrated with GitHub. - DONE
  3. Maybe also integrate Code Scanning. - POSTPONED: THIS FEATURE IS BETA
  4. Experiment with reviewdog. I think it can be used to add comments to PRs on build, lint and test errors.
  5. I want to make something to help with dependency licensing (validation and reporting), but have not found any good free solution that would support Go Modules.
  6. Create something like https://github.com/marketplace/actions/github-tag-bump as a native Go app so to decrease the coupling to GitHub Actions. - REMOVED IN FAVOR OF FAQ

@masch
Copy link
Author

masch commented Jul 2, 2020

Great! All the improvements are wonderful I can't wait to see on the road! 🎉

@pellared
Copy link
Member

pellared commented Jul 4, 2020

@masch
I had some time today to play with Codecov. Could you please test and review this PR: #47 ?
I have already merged it, but I can still change anything. I want to keep momentum going.

@masch
Copy link
Author

masch commented Jul 7, 2020

Hey! @pellared
I'm sorry for the delay, I've used your PR and I've integrated Codecov with GH actions on my project.
Great momentum with a lot of feature!

@masch
Copy link
Author

masch commented Jul 7, 2020

Thank you for the invitation!! 🎉

@pellared
Copy link
Member

pellared commented Jul 8, 2020

@masch I am happy that I could help you.

Please feel free to create a new issue if you have some idea how to improve this repository template.

@pellared pellared closed this as completed Jul 8, 2020
@pellared pellared removed the question Further information is requested label Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants